From a5679d4dd1ad9c544937d98946f7bd6bf6fb2791 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Wed, 10 Jun 2020 09:39:25 -0700 Subject: [PATCH] Additional ansible-test sanity adjustments. (#69974) * Fix ansible-test check_pyyaml usage. * Avoid unnecessary pip commands. * Avoid pointless pip check. * Only install ansible-test requirements as needed. --- .../ansible_test/_internal/ansible_util.py | 23 +++++---- test/lib/ansible_test/_internal/executor.py | 47 +++++++++++++++++-- .../ansible_test/_internal/sanity/__init__.py | 4 +- .../ansible_test/_internal/sanity/yamllint.py | 2 +- 4 files changed, 59 insertions(+), 17 deletions(-) diff --git a/test/lib/ansible_test/_internal/ansible_util.py b/test/lib/ansible_test/_internal/ansible_util.py index ff307e0a3e1..cfee5904032 100644 --- a/test/lib/ansible_test/_internal/ansible_util.py +++ b/test/lib/ansible_test/_internal/ansible_util.py @@ -192,10 +192,12 @@ def get_ansible_python_path(): # type: () -> str return python_path -def check_pyyaml(args, version): +def check_pyyaml(args, version, required=True, quiet=False): """ :type args: EnvironmentConfig :type version: str + :type required: bool + :type quiet: bool """ try: return CHECK_YAML_VERSIONS[version] @@ -206,18 +208,21 @@ def check_pyyaml(args, version): stdout, _dummy = run_command(args, [python, os.path.join(ANSIBLE_TEST_DATA_ROOT, 'yamlcheck.py')], capture=True, always=True) - CHECK_YAML_VERSIONS[version] = result = json.loads(stdout) + result = json.loads(stdout) yaml = result['yaml'] cloader = result['cloader'] - if not yaml: - # do not warn about missing pyyaml - # if it is required by tests they will fail with an error message that should be descriptive enough - # warning here assumes all tests require pyyaml, which is not the case for many sanity tests - pass - elif not cloader: - display.warning('PyYAML will be slow due to installation without libyaml support for interpreter: %s' % python) + if yaml or required: + # results are cached only if pyyaml is required or present + # it is assumed that tests will not uninstall/re-install pyyaml -- if they do, those changes will go undetected + CHECK_YAML_VERSIONS[version] = result + + if not quiet: + if not yaml and required: + display.warning('PyYAML is not installed for interpreter: %s' % python) + elif not cloader: + display.warning('PyYAML will be slow due to installation without libyaml support for interpreter: %s' % python) return result diff --git a/test/lib/ansible_test/_internal/executor.py b/test/lib/ansible_test/_internal/executor.py index 05614a4a283..c399cd9a9ff 100644 --- a/test/lib/ansible_test/_internal/executor.py +++ b/test/lib/ansible_test/_internal/executor.py @@ -216,11 +216,12 @@ def get_cryptography_requirement(args, python_version): # type: (EnvironmentCon return cryptography -def install_command_requirements(args, python_version=None, context=None): +def install_command_requirements(args, python_version=None, context=None, enable_pyyaml_check=False): """ :type args: EnvironmentConfig :type python_version: str | None :type context: str | None + :type enable_pyyaml_check: bool """ if not args.explain: make_dirs(ResultType.COVERAGE.path) @@ -251,11 +252,24 @@ def install_command_requirements(args, python_version=None, context=None): pip = generate_pip_command(find_python(python_version)) - # make sure basic ansible-test requirements are met, including making sure that pip is recent enough to support constraints - # virtualenvs created by older distributions may include very old pip versions, such as those created in the centos6 test container (pip 6.0.8) - run_command(args, generate_pip_install(pip, 'ansible-test', use_constraints=False)) + # skip packages which have aleady been installed for python_version + + try: + package_cache = install_command_requirements.package_cache + except AttributeError: + package_cache = install_command_requirements.package_cache = {} + + installed_packages = package_cache.setdefault(python_version, set()) + skip_packages = [package for package in packages if package in installed_packages] + + for package in skip_packages: + packages.remove(package) + + installed_packages.update(packages) if args.command != 'sanity': + install_ansible_test_requirements(args, pip) + # make sure setuptools is available before trying to install cryptography # the installed version of setuptools affects the version of cryptography to install run_command(args, generate_pip_install(pip, '', packages=['setuptools'])) @@ -276,10 +290,14 @@ def install_command_requirements(args, python_version=None, context=None): commands = [cmd for cmd in commands if cmd] + if not commands: + return # no need to detect changes or run pip check since we are not making any changes + # only look for changes when more than one requirements file is needed detect_pip_changes = len(commands) > 1 # first pass to install requirements, changes expected unless environment is already set up + install_ansible_test_requirements(args, pip) changes = run_pip_commands(args, pip, commands, detect_pip_changes) if changes: @@ -300,6 +318,27 @@ def install_command_requirements(args, python_version=None, context=None): else: raise + if enable_pyyaml_check: + # pyyaml may have been one of the requirements that was installed, so perform an optional check for it + check_pyyaml(args, python_version, required=False) + + +def install_ansible_test_requirements(args, pip): # type: (EnvironmentConfig, t.List[str]) -> None + """Install requirements for ansible-test for the given pip if not already installed.""" + try: + installed = install_command_requirements.installed + except AttributeError: + installed = install_command_requirements.installed = set() + + if tuple(pip) in installed: + return + + # make sure basic ansible-test requirements are met, including making sure that pip is recent enough to support constraints + # virtualenvs created by older distributions may include very old pip versions, such as those created in the centos6 test container (pip 6.0.8) + run_command(args, generate_pip_install(pip, 'ansible-test', use_constraints=False)) + + installed.add(tuple(pip)) + def run_pip_commands(args, pip, commands, detect_pip_changes=False): """ diff --git a/test/lib/ansible_test/_internal/sanity/__init__.py b/test/lib/ansible_test/_internal/sanity/__init__.py index fdb7f9104f8..0ea080bf23c 100644 --- a/test/lib/ansible_test/_internal/sanity/__init__.py +++ b/test/lib/ansible_test/_internal/sanity/__init__.py @@ -39,7 +39,6 @@ from ..util_common import ( from ..ansible_util import ( ansible_environment, - check_pyyaml, ) from ..target import ( @@ -179,8 +178,7 @@ def command_sanity(args): sanity_targets = SanityTargets(tuple(all_targets), tuple(usable_targets)) if usable_targets or test.no_targets: - install_command_requirements(args, version, context=test.name) - check_pyyaml(args, version) + install_command_requirements(args, version, context=test.name, enable_pyyaml_check=True) if isinstance(test, SanityCodeSmellTest): result = test.test(args, sanity_targets, version) diff --git a/test/lib/ansible_test/_internal/sanity/yamllint.py b/test/lib/ansible_test/_internal/sanity/yamllint.py index 4bb9ce8d448..85a576d02d2 100644 --- a/test/lib/ansible_test/_internal/sanity/yamllint.py +++ b/test/lib/ansible_test/_internal/sanity/yamllint.py @@ -71,7 +71,7 @@ class YamllintTest(SanitySingleVersion): :type python_version: str :rtype: TestResult """ - pyyaml_presence = ansible_util.check_pyyaml(args, python_version) + pyyaml_presence = ansible_util.check_pyyaml(args, python_version, quiet=True) if not pyyaml_presence['cloader']: display.warning("Skipping sanity test '%s' due to missing libyaml support in PyYAML." % self.name)