Improve ansible-test environment checking between tests. (#46459)

* Add unified diff output to environment validation.

This makes it easier to see where the environment changed.

* Compare Python interpreters by version to pip shebangs.

This helps expose cases where pip executables use a different
Python interpreter than is expected.

* Query `pip.__version__` instead of using `pip --version`.

This is a much faster way to query the pip version. It also more
closely matches how we invoke pip within ansible-test.

* Remove redundant environment scan between tests.

This reuses the environment scan from the end of the previous test
as the basis for comparison during the next test.

test/runner/lib/executor.py

@@ -14,6 +14,8 @@ import functools
 import pipes
 import sys
 import hashlib
+import difflib
+import filecmp
 
 import lib.pytar
 import lib.thread
@@ -741,6 +743,8 @@ def command_integration_filtered(args, targets, all_targets):
 
     results = {}
 
+    current_environment = None  # type: EnvironmentDescription | None
+
     for target in targets_iter:
         if args.start_at and not found:
             found = target.name == args.start_at
@@ -757,7 +761,8 @@ def command_integration_filtered(args, targets, all_targets):
 
         cloud_environment = get_cloud_environment(args, target)
 
-        original_environment = EnvironmentDescription(args)
+        original_environment = current_environment if current_environment else EnvironmentDescription(args)
+        current_environment = None
 
         display.info('>>> Environment Description\n%s' % original_environment, verbosity=3)
 
@@ -816,9 +821,11 @@ def command_integration_filtered(args, targets, all_targets):
                     display.verbosity = args.verbosity = 6
 
             start_time = time.time()
-            original_environment.validate(target.name, throw=True)
+            current_environment = EnvironmentDescription(args)
             end_time = time.time()
 
+            EnvironmentDescription.check(original_environment, current_environment, target.name, throw=True)
+
             results[target.name]['validation_seconds'] = int(end_time - start_time)
 
             passed.append(target)
@@ -1534,27 +1541,84 @@ class EnvironmentDescription(object):
             self.data = {}
             return
 
+        warnings = []
+
         versions = ['']
         versions += SUPPORTED_PYTHON_VERSIONS
         versions += list(set(v.split('.')[0] for v in SUPPORTED_PYTHON_VERSIONS))
 
         python_paths = dict((v, find_executable('python%s' % v, required=False)) for v in sorted(versions))
-        python_versions = dict((v, self.get_version([python_paths[v], '-V'])) for v in sorted(python_paths) if python_paths[v])
-
         pip_paths = dict((v, find_executable('pip%s' % v, required=False)) for v in sorted(versions))
-        pip_versions = dict((v, self.get_version([pip_paths[v], '--version'])) for v in sorted(pip_paths) if pip_paths[v])
+        program_versions = dict((v, self.get_version([python_paths[v], 'test/runner/versions.py'], warnings)) for v in sorted(python_paths) if python_paths[v])
         pip_interpreters = dict((v, self.get_shebang(pip_paths[v])) for v in sorted(pip_paths) if pip_paths[v])
         known_hosts_hash = self.get_hash(os.path.expanduser('~/.ssh/known_hosts'))
 
+        for version in sorted(versions):
+            self.check_python_pip_association(version, python_paths, pip_paths, pip_interpreters, warnings)
+
+        for warning in warnings:
+            display.warning(warning, unique=True)
+
         self.data = dict(
             python_paths=python_paths,
-            python_versions=python_versions,
             pip_paths=pip_paths,
-            pip_versions=pip_versions,
+            program_versions=program_versions,
             pip_interpreters=pip_interpreters,
             known_hosts_hash=known_hosts_hash,
+            warnings=warnings,
         )
 
+    @staticmethod
+    def check_python_pip_association(version, python_paths, pip_paths, pip_interpreters, warnings):
+        """
+        :type version: str
+        :param python_paths: dict[str, str]
+        :param pip_paths:  dict[str, str]
+        :param pip_interpreters:  dict[str, str]
+        :param warnings: list[str]
+        """
+        python_label = 'Python%s' % (' %s' % version if version else '')
+
+        pip_path = pip_paths.get(version)
+        python_path = python_paths.get(version)
+
+        if not python_path and not pip_path:
+            # neither python or pip is present for this version
+            return
+
+        if not python_path:
+            warnings.append('A %s interpreter was not found, yet a matching pip was found at "%s".' % (python_label, pip_path))
+            return
+
+        if not pip_path:
+            warnings.append('A %s interpreter was found at "%s", yet a matching pip was not found.' % (python_label, python_path))
+            return
+
+        pip_shebang = pip_interpreters.get(version)
+
+        match = re.search(r'#!\s*(?P<command>[^\s]+)', pip_shebang)
+
+        if not match:
+            warnings.append('A %s pip was found at "%s", but it does not have a valid shebang: %s' % (python_label, pip_path, pip_shebang))
+            return
+
+        pip_interpreter = os.path.realpath(match.group('command'))
+        python_interpreter = os.path.realpath(python_path)
+
+        if pip_interpreter == python_interpreter:
+            return
+
+        try:
+            identical = filecmp.cmp(pip_interpreter, python_interpreter)
+        except OSError:
+            identical = False
+
+        if identical:
+            return
+
+        warnings.append('A %s pip was found at "%s", but it uses interpreter "%s" instead of "%s".' % (
+            python_label, pip_path, pip_interpreter, python_interpreter))
+
     def __str__(self):
         """
         :rtype: str
@@ -1569,18 +1633,40 @@ class EnvironmentDescription(object):
         """
         current = EnvironmentDescription(self.args)
 
-        original_json = str(self)
+        return self.check(self, current, target_name, throw)
+
+    @staticmethod
+    def check(original, current, target_name, throw):
+        """
+        :type original: EnvironmentDescription
+        :type current: EnvironmentDescription
+        :type target_name: str
+        :type throw: bool
+        :rtype: bool
+        """
+        original_json = str(original)
         current_json = str(current)
 
         if original_json == current_json:
             return True
 
+        unified_diff = '\n'.join(difflib.unified_diff(
+            a=original_json.splitlines(),
+            b=current_json.splitlines(),
+            fromfile='original.json',
+            tofile='current.json',
+            lineterm='',
+        ))
+
         message = ('Test target "%s" has changed the test environment!\n'
                    'If these changes are necessary, they must be reverted before the test finishes.\n'
                    '>>> Original Environment\n'
                    '%s\n'
                    '>>> Current Environment\n'
-                   '%s' % (target_name, original_json, current_json))
+                   '%s\n'
+                   '>>> Environment Diff\n'
+                   '%s'
+                   % (target_name, original_json, current_json, unified_diff))
 
         if throw:
             raise ApplicationError(message)
@@ -1590,17 +1676,19 @@ class EnvironmentDescription(object):
         return False
 
     @staticmethod
-    def get_version(command):
+    def get_version(command, warnings):
         """
         :type command: list[str]
-        :rtype: str
+        :type warnings: list[str]
+        :rtype: list[str]
         """
         try:
             stdout, stderr = raw_command(command, capture=True, cmd_verbosity=2)
-        except SubprocessError:
+        except SubprocessError as ex:
+            warnings.append(u'%s' % ex)
             return None  # all failures are equal, we don't care why it failed, only that it did
 
-        return (stdout or '').strip() + (stderr or '').strip()
+        return [line.strip() for line in ((stdout or '').strip() + (stderr or '').strip()).splitlines()]
 
     @staticmethod
     def get_shebang(path):
@@ -1609,7 +1697,7 @@ class EnvironmentDescription(object):
         :rtype: str
         """
         with open(path) as script_fd:
-            return script_fd.readline()
+            return script_fd.readline().strip()
 
     @staticmethod
     def get_hash(path):

test/runner/versions.py

@@ -0,0 +1,15 @@
+#!/usr/bin/env python
+"""Show python and pip versions."""
+
+import os
+import sys
+
+try:
+    import pip
+except ImportError:
+    pip = None
+
+print(sys.version)
+
+if pip:
+    print('pip %s from %s' % (pip.__version__, os.path.dirname(pip.__file__)))