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 a2fda66ea54e29b7f5962401518045765bd51ce5 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Wed, 10 Jul 2019 13:33:24 -0700 Subject: [PATCH 2/6] Add flake8 and isort to tox lint check --- tox.ini | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/tox.ini b/tox.ini index 20eb439414c..197dc8429ff 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] skipsdist = True -envlist = py37-{lint,mypy,test}, docs +envlist = py{34,35,36,37}-test, lint, py37-mypy, docs [travis] python = @@ -8,7 +8,6 @@ python = [testenv] deps = - lint: pylint~=2.3.1 mypy: mypy~=0.711 setenv = @@ -18,23 +17,33 @@ setenv = changedir = test: opentelemetry-api/tests - commands = -; 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:lint] +deps = + pylint~=2.3.1 + flake8~=3.7.8 + isort~=4.3.21 + +commands = +; Prefer putting everything in one pylint command to profit from duplication +; warnings. + pylint opentelemetry-api/src/opentelemetry/ opentelemetry-api/tests/ + flake8 opentelemetry-api/src/opentelemetry/ opentelemetry-api/tests/ + isort --check-only --recursive opentelemetry-api/src + [testenv:docs] deps = sphinx~=2.1.2 sphinx-rtd-theme~=0.4.3 sphinx-autodoc-typehints~=1.6.0 +changedir = docs + commands = sphinx-build -W --keep-going -b html -T . _build/html -changedir = docs From 2e86654beeb39d32e4c5e6ae566978ad50bc0052 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 11 Jul 2019 09:11:10 -0700 Subject: [PATCH 3/6] More future-compatible test dep versions --- tox.ini | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tox.ini b/tox.ini index 197dc8429ff..4f0944cd909 100644 --- a/tox.ini +++ b/tox.ini @@ -26,9 +26,9 @@ commands = [testenv:lint] deps = - pylint~=2.3.1 - flake8~=3.7.8 - isort~=4.3.21 + pylint~=2.3 + flake8~=3.7 + isort~=4.3 commands = ; Prefer putting everything in one pylint command to profit from duplication @@ -39,9 +39,9 @@ commands = [testenv:docs] deps = - sphinx~=2.1.2 - sphinx-rtd-theme~=0.4.3 - sphinx-autodoc-typehints~=1.6.0 + sphinx~=2.1 + sphinx-rtd-theme~=0.4 + sphinx-autodoc-typehints~=1.6 changedir = docs From b8f31643a53eef879281a35f577b232d7236e481 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Thu, 11 Jul 2019 09:25:23 -0700 Subject: [PATCH 4/6] Add lint check to travis --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 4f0944cd909..798c1afac21 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ envlist = py{34,35,36,37}-test, lint, py37-mypy, docs [travis] python = - 3.7: py37, docs + 3.7: py37, lint, docs [testenv] deps = From b86b6f052e57c15b0c116d016f61f33b04bc4a70 Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 15 Jul 2019 16:16:03 -0700 Subject: [PATCH 5/6] Fix existing flake8, isort violations --- opentelemetry-api/src/opentelemetry/loader.py | 17 ++++++++++------- .../src/opentelemetry/trace/__init__.py | 1 + opentelemetry-api/tests/test_loader.py | 16 +++++++++++----- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/loader.py b/opentelemetry-api/src/opentelemetry/loader.py index 3c2fdcab6ac..cf2069edb42 100644 --- a/opentelemetry-api/src/opentelemetry/loader.py +++ b/opentelemetry-api/src/opentelemetry/loader.py @@ -61,7 +61,7 @@ def my_factory_for_t(api_type: typing.Type[T]) -> typing.Optional[T]: means that the Python interpreter was invoked with the ``-E`` or ``-I`` flag). """ -from typing import Type, TypeVar, Optional, Callable +from typing import Callable, Optional, Type, TypeVar import importlib import os import sys @@ -78,10 +78,11 @@ def my_factory_for_t(api_type: typing.Type[T]) -> typing.Optional[T]: # annotate setters, were it not for https://github.com/python/mypy/issues/7092 # Once that bug is resolved, setters should use this instead of duplicating the # code. -#ImplementationFactory = Callable[[Type[_T]], Optional[_T]] +# ImplementationFactory = Callable[[Type[_T]], Optional[_T]] _DEFAULT_FACTORY: Optional[_UntrustedImplFactory[object]] = None + def _try_load_impl_from_modname( implementation_modname: str, api_type: Type[_T]) -> Optional[_T]: try: @@ -92,6 +93,7 @@ def _try_load_impl_from_modname( return _try_load_impl_from_mod(implementation_mod, api_type) + def _try_load_impl_from_mod( implementation_mod: object, api_type: Type[_T]) -> Optional[_T]: @@ -109,6 +111,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[_T], api_type: Type[_T] @@ -149,11 +152,10 @@ def _try_load_configured_impl( return _try_load_impl_from_callback(_DEFAULT_FACTORY, api_type) return None + # Public to other opentelemetry-api modules -def _load_impl( - api_type: Type[_T], - factory: Optional[Callable[[Type[_T]], Optional[_T]]] - ) -> _T: +def _load_impl(api_type: Type[_T], + factory: Optional[Callable[[Type[_T]], Optional[_T]]]) -> _T: """Tries to load a configured implementation, if unsuccessful, returns a fast no-op implemenation that is always available. """ @@ -163,9 +165,10 @@ def _load_impl( return api_type() return result + def set_preferred_default_implementation( 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 + global _DEFAULT_FACTORY # pylint:disable=global-statement _DEFAULT_FACTORY = implementation_factory diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 0660f9e3940..d5fd5446a48 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -64,6 +64,7 @@ from contextlib import contextmanager import typing + from opentelemetry import loader diff --git a/opentelemetry-api/tests/test_loader.py b/opentelemetry-api/tests/test_loader.py index 72d008499f1..68809468f5d 100644 --- a/opentelemetry-api/tests/test_loader.py +++ b/opentelemetry-api/tests/test_loader.py @@ -13,25 +13,28 @@ # limitations under the License. from importlib import reload +import os 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 + 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 + +# pylint:disable=redefined-outer-name,protected-access,unidiomatic-typecheck class TestLoader(unittest.TestCase): @@ -43,7 +46,6 @@ def setUp(self): # class after reloading `trace`. reload(sys.modules[__name__]) - def test_get_default(self): tracer = trace.tracer() self.assertIs(type(tracer), trace.Tracer) @@ -60,8 +62,10 @@ 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) @@ -75,7 +79,7 @@ def test_try_set_again(self): self.assertIn('already loaded', str(einfo.exception)) def do_test_get_envvar(self, envvar_suffix): - global DUMMY_TRACER #pylint:disable=global-statement + global DUMMY_TRACER # pylint:disable=global-statement # Test is not runnable with this! self.assertFalse(sys.flags.ignore_environment) @@ -89,7 +93,9 @@ def do_test_get_envvar(self, envvar_suffix): 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') From a7c947ca07ffb9c973fb376d939b32979a3ef8aa Mon Sep 17 00:00:00 2001 From: Chris Kleinknecht Date: Mon, 15 Jul 2019 16:28:14 -0700 Subject: [PATCH 6/6] Make isort match default gnu sort ordering --- .isort.cfg | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .isort.cfg diff --git a/.isort.cfg b/.isort.cfg new file mode 100644 index 00000000000..3d19a1c6e01 --- /dev/null +++ b/.isort.cfg @@ -0,0 +1,2 @@ +[settings] +from_first=True