From fa2adf3b7b36b1a24f3eb801f8838cd9f578eb9b Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Mon, 5 Aug 2019 16:38:21 -0700 Subject: [PATCH] More prep for ansible-test relocation. (#60114) * Update pytest plugins. * Fix update-bundled sanity test. * Remove old validate-modules comment. * Fix ansible-test plugin loading * Update code coverage comments. * Fix Makefile ansible-test reference. * Remove incorrect path from lint output. * Update ansible-test unit tests. * Make ansible-test's own unit tests singular. --- test/runner/Makefile | 12 ++++++------ test/runner/lib/test.py | 2 +- test/runner/lib/util.py | 10 +++++++--- test/runner/lib/util_common.py | 4 ++-- test/runner/{units => unit}/test_diff.py | 17 +++++++++++------ test/sanity/code-smell/update-bundled.json | 11 ++++++----- test/sanity/code-smell/update-bundled.py | 3 --- test/sanity/ignore.txt | 2 -- test/sanity/validate-modules/utils.py | 2 -- .../plugins/ansible_pytest_collections.py | 8 ++++++-- .../pytest/plugins/ansible_pytest_coverage.py | 9 ++++++--- 11 files changed, 45 insertions(+), 35 deletions(-) rename test/runner/{units => unit}/test_diff.py (85%) diff --git a/test/runner/Makefile b/test/runner/Makefile index a2f9d131ee1..baee64a9dd6 100644 --- a/test/runner/Makefile +++ b/test/runner/Makefile @@ -1,9 +1,9 @@ -.PHONY: all sanity units - -all: sanity units +all: sanity unit +.PHONY: sanity sanity: - ./ansible-test sanity test/runner/ ${TEST_FLAGS} + ansible-test sanity test/runner/ ${FLAGS} -units: - PYTHONPATH=. pytest units ${TEST_FLAGS} +.PHONY: unit +unit: + PYTHONPATH=.:.. pytest unit ${FLAGS} diff --git a/test/runner/lib/test.py b/test/runner/lib/test.py index 13d02cfbbad..dd46dfca839 100644 --- a/test/runner/lib/test.py +++ b/test/runner/lib/test.py @@ -285,7 +285,7 @@ class TestFailure(TestResult): if self.summary: command = self.format_command() message = 'The test `%s` failed. See stderr output for details.' % command - path = 'test/runner/ansible-test' + path = '' message = TestMessage(message, path) print(message) else: diff --git a/test/runner/lib/util.py b/test/runner/lib/util.py index 01d46ab1a8a..be7df05c59b 100644 --- a/test/runner/lib/util.py +++ b/test/runner/lib/util.py @@ -812,10 +812,11 @@ def import_plugins(directory, root=None): # type: (str, t.Optional[str]) -> Non root = os.path.dirname(__file__) path = os.path.join(root, directory) - prefix = 'lib.%s.' % directory.replace(os.sep, '.') + package = __name__.rsplit('.', 1)[0] + prefix = '%s.%s.' % (package, directory.replace(os.sep, '.')) for (_module_loader, name, _ispkg) in pkgutil.iter_modules([path], prefix=prefix): - module_path = os.path.join(root, name[4:].replace('.', os.sep) + '.py') + module_path = os.path.join(root, name[len(package) + 1:].replace('.', os.sep) + '.py') load_module(module_path, name) @@ -824,7 +825,7 @@ def load_plugins(base_type, database): # type: (t.Type[C], t.Dict[str, t.Type[C Load plugins of the specified type and track them in the specified database. Only plugins which have already been imported will be loaded. """ - plugins = dict((sc.__module__.split('.')[2], sc) for sc in get_subclasses(base_type)) # type: t.Dict[str, t.Type[C]] + plugins = dict((sc.__module__.rsplit('.', 1)[1], sc) for sc in get_subclasses(base_type)) # type: t.Dict[str, t.Type[C]] for plugin in plugins: database[plugin] = plugins[plugin] @@ -832,6 +833,9 @@ def load_plugins(base_type, database): # type: (t.Type[C], t.Dict[str, t.Type[C def load_module(path, name): # type: (str, str) -> None """Load a Python module using the given name and path.""" + if name in sys.modules: + return + if sys.version_info >= (3, 4): # noinspection PyUnresolvedReferences import importlib.util diff --git a/test/runner/lib/util_common.py b/test/runner/lib/util_common.py index 6cba3a084c1..baa056480e7 100644 --- a/test/runner/lib/util_common.py +++ b/test/runner/lib/util_common.py @@ -169,8 +169,8 @@ def get_coverage_environment(args, target_name, version, temp_path, module_cover coverage_file = '' # Enable code coverage collection on local Python programs (this does not include Ansible modules). - # Used by the injectors in test/runner/injector/ to support code coverage. - # Used by unit tests in test/units/conftest.py to support code coverage. + # Used by the injectors to support code coverage. + # Used by the pytest unit test plugin to support code coverage. # The COVERAGE_FILE variable is also used directly by the 'coverage' module. env = dict( COVERAGE_CONF=config_file, diff --git a/test/runner/units/test_diff.py b/test/runner/unit/test_diff.py similarity index 85% rename from test/runner/units/test_diff.py rename to test/runner/unit/test_diff.py index 4f94ff4d5b3..f44537a60d1 100644 --- a/test/runner/units/test_diff.py +++ b/test/runner/unit/test_diff.py @@ -6,6 +6,11 @@ import os import subprocess import pytest +from lib.util import ( + to_text, + to_bytes, +) + from lib.diff import ( parse_diff, FileDiff, @@ -19,18 +24,18 @@ def get_diff(base, head=None): :rtype: list[str] """ if not head or head == 'HEAD': - head = subprocess.check_output(['git', 'rev-parse', 'HEAD']).strip() + head = to_text(subprocess.check_output(['git', 'rev-parse', 'HEAD'])).strip() cache = '/tmp/git-diff-cache-%s-%s.log' % (base, head) if os.path.exists(cache): - with open(cache, 'r') as cache_fd: - lines = cache_fd.read().splitlines() + with open(cache, 'rb') as cache_fd: + lines = to_text(cache_fd.read()).splitlines() else: - lines = subprocess.check_output(['git', 'diff', base, head]).splitlines() + lines = to_text(subprocess.check_output(['git', 'diff', base, head]), errors='replace').splitlines() - with open(cache, 'w') as cache_fd: - cache_fd.write('\n'.join(lines)) + with open(cache, 'wb') as cache_fd: + cache_fd.write(to_bytes('\n'.join(lines))) assert lines diff --git a/test/sanity/code-smell/update-bundled.json b/test/sanity/code-smell/update-bundled.json index 77c7254b55c..379bf4d7b4e 100644 --- a/test/sanity/code-smell/update-bundled.json +++ b/test/sanity/code-smell/update-bundled.json @@ -1,7 +1,8 @@ { - "all_targets": true, - "extensions": [ - ".py" - ], - "output": "path-message" + "all_targets": true, + "ignore_self": true, + "extensions": [ + ".py" + ], + "output": "path-message" } diff --git a/test/sanity/code-smell/update-bundled.py b/test/sanity/code-smell/update-bundled.py index 5050c62c448..121e225f396 100755 --- a/test/sanity/code-smell/update-bundled.py +++ b/test/sanity/code-smell/update-bundled.py @@ -70,9 +70,6 @@ def get_files_with_bundled_metadata(paths): with_metadata = set() for path in paths: - if path == 'test/sanity/code-smell/update-bundled.py': - continue - with open(path, 'rb') as f: body = f.read() diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index e7c64aa10f9..2ee1d0f95e9 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -7914,8 +7914,6 @@ test/units/plugins/shell/test_cmd.py metaclass-boilerplate test/units/plugins/shell/test_powershell.py future-import-boilerplate test/units/plugins/shell/test_powershell.py metaclass-boilerplate test/units/plugins/test_plugins.py pylint:blacklisted-name -test/units/pytest/plugins/ansible_pytest_collections.py metaclass-boilerplate -test/units/pytest/plugins/ansible_pytest_coverage.py metaclass-boilerplate test/units/template/test_templar.py pylint:blacklisted-name test/units/test_constants.py future-import-boilerplate test/units/test_context.py future-import-boilerplate diff --git a/test/sanity/validate-modules/utils.py b/test/sanity/validate-modules/utils.py index 82ee711f3d2..3a083497131 100644 --- a/test/sanity/validate-modules/utils.py +++ b/test/sanity/validate-modules/utils.py @@ -38,8 +38,6 @@ class AnsibleTextIOWrapper(TextIOWrapper): def find_executable(executable, cwd=None, path=None): """Finds the full path to the executable specified""" - # This is mostly a copy from test/runner/lib/util.py. Should be removed once validate-modules has been integrated - # into ansible-test match = None real_cwd = os.getcwd() diff --git a/test/units/pytest/plugins/ansible_pytest_collections.py b/test/units/pytest/plugins/ansible_pytest_collections.py index 16ec8235b83..ae87187b835 100644 --- a/test/units/pytest/plugins/ansible_pytest_collections.py +++ b/test/units/pytest/plugins/ansible_pytest_collections.py @@ -1,6 +1,6 @@ """Enable unit testing of Ansible collections.""" from __future__ import (absolute_import, division, print_function) - +__metaclass__ = type import os import sys @@ -10,6 +10,7 @@ ANSIBLE_COLLECTIONS_PATH = os.path.join(os.environ['ANSIBLE_COLLECTIONS_PATHS'], def collection_pypkgpath(self): + """Configure the Python package path so that pytest can find our collections.""" for parent in self.parts(reverse=True): if str(parent) == ANSIBLE_COLLECTIONS_PATH: return parent @@ -18,13 +19,16 @@ def collection_pypkgpath(self): def pytest_configure(): + """Configure this pytest plugin.""" from ansible.utils.collection_loader import AnsibleCollectionLoader # allow unit tests to import code from collections sys.meta_path.insert(0, AnsibleCollectionLoader()) + # noinspection PyProtectedMember import py._path.local # force collections unit tests to be loaded with the ansible_collections namespace # original idea from https://stackoverflow.com/questions/50174130/how-do-i-pytest-a-project-using-pep-420-namespace-packages/50175552#50175552 - py._path.local.LocalPath.pypkgpath = collection_pypkgpath + # noinspection PyProtectedMember + py._path.local.LocalPath.pypkgpath = collection_pypkgpath # pylint: disable=protected-access diff --git a/test/units/pytest/plugins/ansible_pytest_coverage.py b/test/units/pytest/plugins/ansible_pytest_coverage.py index da4fc70e17d..344a091e1c5 100644 --- a/test/units/pytest/plugins/ansible_pytest_coverage.py +++ b/test/units/pytest/plugins/ansible_pytest_coverage.py @@ -1,15 +1,17 @@ """Monkey patch os._exit when running under coverage so we don't lose coverage data in forks, such as with `pytest --boxed`.""" from __future__ import (absolute_import, division, print_function) +__metaclass__ = type def pytest_configure(): + """Configure this pytest plugin.""" try: import coverage except ImportError: coverage = None try: - test = coverage.Coverage + coverage.Coverage except AttributeError: coverage = None @@ -41,7 +43,8 @@ def pytest_configure(): else: cov = None - os_exit = os._exit + # noinspection PyProtectedMember + os_exit = os._exit # pylint: disable=protected-access def coverage_exit(*args, **kwargs): for instance in coverage_instances: @@ -50,7 +53,7 @@ def pytest_configure(): os_exit(*args, **kwargs) - os._exit = coverage_exit + os._exit = coverage_exit # pylint: disable=protected-access if cov: cov.start()