Skip to content

Commit

Permalink
Fix mypy errors (open-telemetry#229)
Browse files Browse the repository at this point in the history
In particular, the following errors are fixed in this commit:

* Don't return False in __exit__

Returning a literal causes a mypy error when combined with the
`typing.Optional[bool]` type hint. Furthermore, exception handling is
the same when returning `False` and when returning `None` (the
exception is re-raised). Therefore, it's simpler to remove the return
statement and change the type hint to `None`.

* Correctly initialize nested tuple

Tuples of length 1 should be initialized with a trailing comma to be
properly interpreted.

* Pass correct type to use_context() in test

* Add type annotations for test helper functions

Since we have `disallow_untyped_calls = True` in our mypy config for
tests, we must add type annotations to any function that is called
from a test.


Addditionally, bump minimal mypy version to 0.740 to consistently reproduce these errors.
  • Loading branch information
johananl authored and c24t committed Oct 21, 2019
1 parent 235d74f commit 3594e32
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 11 deletions.
8 changes: 2 additions & 6 deletions opentelemetry-api/src/opentelemetry/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,13 +245,9 @@ def __exit__(
exc_type: typing.Optional[typing.Type[BaseException]],
exc_val: typing.Optional[BaseException],
exc_tb: typing.Optional[python_types.TracebackType],
) -> typing.Optional[bool]:
"""Ends context manager and calls `end` on the `Span`.
Returns False.
"""
) -> None:
"""Ends context manager and calls `end` on the `Span`."""
self.end()
return False


class TraceOptions(int):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,14 @@ def test_get_current_context(self):
self.assertIsNone(self.manager.get_current_context())

def test_use_context(self):
expected = object()
expected = distributedcontext.DistributedContext(
(
distributedcontext.Entry(
distributedcontext.EntryMetadata(0),
distributedcontext.EntryKey("0"),
distributedcontext.EntryValue(""),
),
)
)
with self.manager.use_context(expected) as output:
self.assertIs(output, expected)
2 changes: 1 addition & 1 deletion opentelemetry-api/tests/metrics/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def setUp(self):

def test_record_batch(self):
counter = metrics.Counter()
self.meter.record_batch(("values"), ((counter, 1)))
self.meter.record_batch(("values"), ((counter, 1),))

def test_create_metric(self):
metric = self.meter.create_metric("", "", "", float, metrics.Counter)
Expand Down
5 changes: 3 additions & 2 deletions opentelemetry-api/tests/test_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import sys
import unittest
from importlib import reload
from typing import Any, Callable

from opentelemetry import trace
from opentelemetry.util import loader
Expand Down Expand Up @@ -59,7 +60,7 @@ def test_preferred_impl(self):

# NOTE: We use do_* + *_<arg> methods because subtest wouldn't run setUp,
# which we require here.
def do_test_preferred_impl(self, setter):
def do_test_preferred_impl(self, setter: Callable[[Any], Any]) -> None:
setter(get_opentelemetry_implementation)
tracer = trace.tracer()
self.assertIs(tracer, DUMMY_TRACER)
Expand All @@ -81,7 +82,7 @@ def test_try_set_again(self):
)
self.assertIn("already loaded", str(einfo.exception))

def do_test_get_envvar(self, envvar_suffix):
def do_test_get_envvar(self, envvar_suffix: str) -> None:
global DUMMY_TRACER # pylint:disable=global-statement

# Test is not runnable with this!
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ python =

[testenv]
deps =
mypy,mypyinstalled: mypy~=0.711
mypy,mypyinstalled: mypy~=0.740

setenv =
mypy: MYPYPATH={toxinidir}/opentelemetry-api/src/
Expand Down

0 comments on commit 3594e32

Please sign in to comment.