From e813b0151c3b4baec8368fc0d099e7f19be22144 Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Mon, 14 Sep 2020 09:14:23 -0700 Subject: [PATCH] fix coverage output from synthetic packages (#71727) * fix coverage output from synthetic packages * synthetic packages (eg, implicit collection packages without `__init__.py`) were always created at runtime with empty string source, which was compiled to a code object and exec'd during the package load. When run with code coverage, it created a bogus coverage entry (since the `__synthetic__`-suffixed `__file__` entry didn't exist on disk). * modified collection loader `get_code` to preserve the distinction between `None` (eg synthetic package) and empty string (eg empty `__init__.py`) values from `get_source`, and to return `None` when the source is `None`. This allows the package loader to skip `exec`ing things that truly have no source file on disk, thus not creating bogus coverage entries, while preserving behavior and coverage reporting for empty package inits that actually exist. * add unit test --- changelogs/fragments/fix_bogus_coverage.yml | 2 ++ .../collection_loader/_collection_finder.py | 11 ++++++++--- .../collection_loader/test_collection_loader.py | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/fix_bogus_coverage.yml diff --git a/changelogs/fragments/fix_bogus_coverage.yml b/changelogs/fragments/fix_bogus_coverage.yml new file mode 100644 index 00000000000..c60ada5f6e1 --- /dev/null +++ b/changelogs/fragments/fix_bogus_coverage.yml @@ -0,0 +1,2 @@ +bugfixes: +- collection loader - fix bogus code coverage entries for synthetic packages diff --git a/lib/ansible/utils/collection_loader/_collection_finder.py b/lib/ansible/utils/collection_loader/_collection_finder.py index bc099b262f2..99d5ddc9169 100644 --- a/lib/ansible/utils/collection_loader/_collection_finder.py +++ b/lib/ansible/utils/collection_loader/_collection_finder.py @@ -355,7 +355,9 @@ class _AnsibleCollectionPkgLoaderBase: with self._new_or_existing_module(fullname, **module_attrs) as module: # execute the module's code in its namespace - exec(self.get_code(fullname), module.__dict__) + code_obj = self.get_code(fullname) + if code_obj is not None: # things like NS packages that can't have code on disk will return None + exec(code_obj, module.__dict__) return module @@ -428,8 +430,11 @@ class _AnsibleCollectionPkgLoaderBase: filename = '' source_code = self.get_source(fullname) - if not source_code: - source_code = '' + + # for things like synthetic modules that really have no source on disk, don't return a code object at all + # vs things like an empty package init (which has an empty string source on disk) + if source_code is None: + return None self._compiled_code = compile(source=source_code, filename=filename, mode='exec', flags=0, dont_inherit=True) diff --git a/test/units/utils/collection_loader/test_collection_loader.py b/test/units/utils/collection_loader/test_collection_loader.py index 496dc5416e6..6488188c087 100644 --- a/test/units/utils/collection_loader/test_collection_loader.py +++ b/test/units/utils/collection_loader/test_collection_loader.py @@ -594,6 +594,22 @@ def test_bogus_imports(): import_module(bogus_import) +def test_empty_vs_no_code(): + finder = get_default_finder() + reset_collections_loader_state(finder) + + from ansible_collections.testns import testcoll # synthetic package with no code on disk + from ansible_collections.testns.testcoll.plugins import module_utils # real package with empty code file + + # ensure synthetic packages have no code object at all (prevent bogus coverage entries) + assert testcoll.__loader__.get_source(testcoll.__name__) is None + assert testcoll.__loader__.get_code(testcoll.__name__) is None + + # ensure empty package inits do have a code object + assert module_utils.__loader__.get_source(module_utils.__name__) == b'' + assert module_utils.__loader__.get_code(module_utils.__name__) is not None + + def test_finder_playbook_paths(): finder = get_default_finder() reset_collections_loader_state(finder)