From a2b35fced5efb3a46cc63b053be78936b26c749d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Mon, 8 Jul 2019 15:33:14 +0200 Subject: [PATCH 1/6] Add unittest based unit-tests. --- opentelemetry-api/tests/__init__.py | 13 ++++ opentelemetry-api/tests/test_loader.py | 95 ++++++++++++++++++++++++++ tox.ini | 17 ++++- 3 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 opentelemetry-api/tests/__init__.py create mode 100644 opentelemetry-api/tests/test_loader.py diff --git a/opentelemetry-api/tests/__init__.py b/opentelemetry-api/tests/__init__.py new file mode 100644 index 00000000000..d853a7bcf65 --- /dev/null +++ b/opentelemetry-api/tests/__init__.py @@ -0,0 +1,13 @@ +# Copyright 2019, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/opentelemetry-api/tests/test_loader.py b/opentelemetry-api/tests/test_loader.py new file mode 100644 index 00000000000..72d008499f1 --- /dev/null +++ b/opentelemetry-api/tests/test_loader.py @@ -0,0 +1,95 @@ +# Copyright 2019, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from importlib import reload +import sys +import unittest +import os + +from opentelemetry import loader +from opentelemetry import trace + +DUMMY_TRACER = None + +class DummyTracer(trace.Tracer): + pass + +def get_opentelemetry_implementation(type_): + global DUMMY_TRACER #pylint:disable=global-statement + assert type_ is trace.Tracer + DUMMY_TRACER = DummyTracer() + return DUMMY_TRACER + +#pylint:disable=redefined-outer-name,protected-access,unidiomatic-typecheck + +class TestLoader(unittest.TestCase): + + def setUp(self): + reload(loader) + reload(trace) + + # Need to reload self, otherwise DummyTracer will have the wrong base + # class after reloading `trace`. + reload(sys.modules[__name__]) + + + def test_get_default(self): + tracer = trace.tracer() + self.assertIs(type(tracer), trace.Tracer) + + def test_preferred_impl(self): + trace.set_preferred_tracer_implementation( + get_opentelemetry_implementation) + tracer = trace.tracer() + self.assertIs(tracer, DUMMY_TRACER) + + # NOTE: We use do_* + *_ methods because subtest wouldn't run setUp, + # which we require here. + def do_test_preferred_impl(self, setter): + setter(get_opentelemetry_implementation) + tracer = trace.tracer() + self.assertIs(tracer, DUMMY_TRACER) + def test_preferred_impl_with_tracer(self): + self.do_test_preferred_impl(trace.set_preferred_tracer_implementation) + def test_preferred_impl_with_default(self): + self.do_test_preferred_impl( + loader.set_preferred_default_implementation) + + def test_try_set_again(self): + self.assertTrue(trace.tracer()) + # Try setting after the tracer has already been created: + with self.assertRaises(RuntimeError) as einfo: + trace.set_preferred_tracer_implementation( + get_opentelemetry_implementation) + self.assertIn('already loaded', str(einfo.exception)) + + def do_test_get_envvar(self, envvar_suffix): + global DUMMY_TRACER #pylint:disable=global-statement + + # Test is not runnable with this! + self.assertFalse(sys.flags.ignore_environment) + + envname = 'OPENTELEMETRY_PYTHON_IMPLEMENTATION_' + envvar_suffix + os.environ[envname] = __name__ + try: + tracer = trace.tracer() + self.assertIs(tracer, DUMMY_TRACER) + finally: + DUMMY_TRACER = None + del os.environ[envname] + self.assertIs(type(tracer), DummyTracer) + def test_get_envvar_tracer(self): + return self.do_test_get_envvar('TRACER') + def test_get_envvar_default(self): + return self.do_test_get_envvar('DEFAULT') diff --git a/tox.ini b/tox.ini index 4997e45f350..20eb439414c 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] skipsdist = True -envlist = py37-{lint,mypy}, docs +envlist = py37-{lint,mypy,test}, docs [travis] python = @@ -11,10 +11,23 @@ deps = lint: pylint~=2.3.1 mypy: mypy~=0.711 +setenv = + PYTHONPATH={toxinidir}/opentelemetry-api/src/ + mypy: MYPYPATH={env:PYTHONPATH} + +changedir = + test: opentelemetry-api/tests + commands = - py37-lint: pylint opentelemetry-api/src/opentelemetry/ +; Prefer putting everything in one pylint command to profit from duplication +; warnings. + py37-lint: pylint opentelemetry-api/src/opentelemetry/ opentelemetry-api/tests/ py37-mypy: mypy opentelemetry-api/src/opentelemetry/ +; For test code, we don't want to use the full mypy strictness, so we use its +; default flags instead. + py37-mypy: mypy opentelemetry-api/tests/ --config-file= + test: python -m unittest discover [testenv:docs] deps = From d3d74fde204d31e6bcfc90937bc9080e85df36a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Mon, 8 Jul 2019 17:55:21 +0200 Subject: [PATCH 2/6] Also lint/mypy code outside main package. Why would we ignore our line length limits in setup.py? Also use more mypy checks for test code (without check_untyped_defs it's really useless). --- docs/conf.py | 2 +- mypy-relaxed.ini | 22 ++++++++++++++++++++++ mypy.ini | 1 + opentelemetry-api/setup.py | 15 ++++++++------- tox.ini | 8 ++++---- 5 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 mypy-relaxed.ini diff --git a/docs/conf.py b/docs/conf.py index c4fee51f31a..8f2ac1d5e6b 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -18,7 +18,7 @@ # -- Project information ----------------------------------------------------- project = 'OpenTelemetry' -copyright = '2019, OpenTelemetry Authors' +copyright = '2019, OpenTelemetry Authors' #pylint:disable=redefined-builtin author = 'OpenTelemetry Authors' diff --git a/mypy-relaxed.ini b/mypy-relaxed.ini new file mode 100644 index 00000000000..8f2be85affc --- /dev/null +++ b/mypy-relaxed.ini @@ -0,0 +1,22 @@ +; This is mainly intended for unit tests and such. So proably going forward, we +; will disable even more warnings here. +[mypy] + disallow_any_unimported = True +; disallow_any_expr = True + disallow_any_decorated = True + disallow_any_explicit = True + disallow_any_generics = True + disallow_subclassing_any = True + disallow_untyped_calls = True +; disallow_untyped_defs = True + disallow_incomplete_defs = True + check_untyped_defs = True + disallow_untyped_decorators = True + allow_untyped_globals = True +; Due to disabling some other warnings, unused ignores may occur. +; warn_unused_ignores = True + warn_return_any = True + strict_equality = True + +[mypy-setuptools] + ignore_missing_imports = True diff --git a/mypy.ini b/mypy.ini index 5899d1954e0..5b46777838d 100644 --- a/mypy.ini +++ b/mypy.ini @@ -12,3 +12,4 @@ disallow_untyped_decorators = True warn_unused_ignores = True warn_return_any = True + strict_equality = True diff --git a/opentelemetry-api/setup.py b/opentelemetry-api/setup.py index de97ec88c14..3c83a77fb3a 100644 --- a/opentelemetry-api/setup.py +++ b/opentelemetry-api/setup.py @@ -15,15 +15,16 @@ import os import setuptools -base_dir = os.path.dirname(__file__) - -package_info = {} -with open(os.path.join(base_dir, "src", "opentelemetry", "internal", "version.py")) as f: - exec(f.read(), package_info) +BASE_DIR = os.path.dirname(__file__) +VERSION_FILENAME = os.path.join( + BASE_DIR, "src", "opentelemetry", "internal", "version.py") +PACKAGE_INFO = {} +with open(VERSION_FILENAME) as f: + exec(f.read(), PACKAGE_INFO) #pylint:disable=exec-used setuptools.setup( name="opentelemetry-api", - version=package_info["__version__"], # noqa + version=PACKAGE_INFO["__version__"], # noqa author="OpenTelemetry Authors", author_email="cncf-opentelemetry-contributors@lists.cncf.io", classifiers=[ @@ -47,6 +48,6 @@ license="Apache-2.0", package_dir={"": "src"}, packages=setuptools.find_namespace_packages(where="src"), - url="https://github.com/open-telemetry/opentelemetry-python/tree/master/opentelemetry-api", + url="https://github.com/open-telemetry/opentelemetry-python/tree/master/opentelemetry-api", #pylint:disable=line-too-long zip_safe=False, ) diff --git a/tox.ini b/tox.ini index 20eb439414c..03fdab389dc 100644 --- a/tox.ini +++ b/tox.ini @@ -22,11 +22,11 @@ changedir = commands = ; Prefer putting everything in one pylint command to profit from duplication ; warnings. - py37-lint: pylint opentelemetry-api/src/opentelemetry/ opentelemetry-api/tests/ + py37-lint: pylint opentelemetry-api/src/opentelemetry/ opentelemetry-api/tests/ opentelemetry-api/setup.py + py37-lint: pylint --disable=invalid-name docs/conf.py py37-mypy: mypy opentelemetry-api/src/opentelemetry/ -; For test code, we don't want to use the full mypy strictness, so we use its -; default flags instead. - py37-mypy: mypy opentelemetry-api/tests/ --config-file= +; For test code, we don't want to enforce the full mypy strictness + py37-mypy: mypy --config-file=mypy-relaxed.ini opentelemetry-api/tests/ opentelemetry-api/setup.py docs/conf.py test: python -m unittest discover [testenv:docs] From 99daa643a4747e37f9aa947717eabee76b06f822 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 15 Jul 2019 11:41:52 -0700 Subject: [PATCH 3/6] Don't lint docs --- docs/conf.py | 2 +- tox.ini | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 8f2ac1d5e6b..c4fee51f31a 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -18,7 +18,7 @@ # -- Project information ----------------------------------------------------- project = 'OpenTelemetry' -copyright = '2019, OpenTelemetry Authors' #pylint:disable=redefined-builtin +copyright = '2019, OpenTelemetry Authors' author = 'OpenTelemetry Authors' diff --git a/tox.ini b/tox.ini index 6b88f049558..f40352c16d5 100644 --- a/tox.ini +++ b/tox.ini @@ -23,7 +23,6 @@ commands = ; Prefer putting everything in one pylint command to profit from duplication ; warnings. py37-lint: pylint opentelemetry-api/src/opentelemetry/ opentelemetry-api/tests/ opentelemetry-api/setup.py - py37-lint: pylint --disable=invalid-name docs/conf.py py37-mypy: mypy opentelemetry-api/src/opentelemetry/ ; For test code, we don't want to use the full mypy strictness, so we use its ; default flags instead. From 1463de538b5230dbfcc77ade4b8444bcdb374051 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 15 Jul 2019 12:01:50 -0700 Subject: [PATCH 4/6] Split long URL line --- opentelemetry-api/setup.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/setup.py b/opentelemetry-api/setup.py index 3c83a77fb3a..d2a27a0d442 100644 --- a/opentelemetry-api/setup.py +++ b/opentelemetry-api/setup.py @@ -48,6 +48,7 @@ license="Apache-2.0", package_dir={"": "src"}, packages=setuptools.find_namespace_packages(where="src"), - url="https://github.com/open-telemetry/opentelemetry-python/tree/master/opentelemetry-api", #pylint:disable=line-too-long + url=("https://github.com/open-telemetry/opentelemetry-python" + "/tree/master/opentelemetry-api"), zip_safe=False, ) From e3ef0a04779d74d2184530bcd2efbaa2e7f45ac4 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 15 Jul 2019 15:31:40 -0700 Subject: [PATCH 5/6] Deal with missing type parameters in loader --- opentelemetry-api/src/opentelemetry/loader.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/loader.py b/opentelemetry-api/src/opentelemetry/loader.py index dc3a4e078e9..3c2fdcab6ac 100644 --- a/opentelemetry-api/src/opentelemetry/loader.py +++ b/opentelemetry-api/src/opentelemetry/loader.py @@ -80,7 +80,7 @@ def my_factory_for_t(api_type: typing.Type[T]) -> typing.Optional[T]: # code. #ImplementationFactory = Callable[[Type[_T]], Optional[_T]] -_DEFAULT_FACTORY: Optional[_UntrustedImplFactory] = None +_DEFAULT_FACTORY: Optional[_UntrustedImplFactory[object]] = None def _try_load_impl_from_modname( implementation_modname: str, api_type: Type[_T]) -> Optional[_T]: @@ -101,7 +101,8 @@ def _try_load_impl_from_mod( implementation_fn = getattr( implementation_mod, - 'get_opentelemetry_implementation') # type: _UntrustedImplFactory + 'get_opentelemetry_implementation' + ) # type: _UntrustedImplFactory[_T] except AttributeError: # TODO Log/warn return None @@ -109,7 +110,7 @@ def _try_load_impl_from_mod( return _try_load_impl_from_callback(implementation_fn, api_type) def _try_load_impl_from_callback( - implementation_fn: _UntrustedImplFactory, + implementation_fn: _UntrustedImplFactory[_T], api_type: Type[_T] ) -> Optional[_T]: result = implementation_fn(api_type) @@ -163,7 +164,7 @@ def _load_impl( return result def set_preferred_default_implementation( - implementation_factory: _UntrustedImplFactory) -> None: + implementation_factory: _UntrustedImplFactory[_T]) -> None: """Sets a factory function that may be called for any implementation object. See the :ref:`module docs ` for more details.""" global _DEFAULT_FACTORY #pylint:disable=global-statement From 1c5143b56779bdaf770f06cf3e1492a41d157d47 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 15 Jul 2019 15:41:48 -0700 Subject: [PATCH 6/6] Bring back tox changes from use d3d74fd that got lost in master merge. --- tox.ini | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tox.ini b/tox.ini index f40352c16d5..bd2dae1c566 100644 --- a/tox.ini +++ b/tox.ini @@ -24,9 +24,9 @@ commands = ; warnings. py37-lint: pylint opentelemetry-api/src/opentelemetry/ opentelemetry-api/tests/ opentelemetry-api/setup.py py37-mypy: mypy opentelemetry-api/src/opentelemetry/ -; For test code, we don't want to use the full mypy strictness, so we use its -; default flags instead. - py37-mypy: mypy opentelemetry-api/tests/ --config-file= +; For test code, we don't want to enforce the full mypy strictness + py37-mypy: mypy opentelemetry-api/src/opentelemetry/ + py37-mypy: mypy --config-file=mypy-relaxed.ini opentelemetry-api/tests/ opentelemetry-api/setup.py test: python -m unittest discover [testenv:docs]