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.

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
 

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):
     """

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)

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)