Bug fixes and cleanup for ansible-test. (#45991)

* Remove unused imports.
* Clean up ConfigParser usage in ansible-test.
* Fix bare except statements in ansible-test.
* Miscellaneous cleanup from PyCharm inspections.
* Enable pylint no-self-use for ansible-test.
* Remove obsolete pylint ignores for Python 3.7.
* Fix shellcheck issuers under newer shellcheck.
* Use newer path for ansible-test.
* Fix issues in code-smell tests.

(cherry picked from commit ac492476e5)

Makefile

@@ -122,7 +122,7 @@ ifneq ($(REPOTAG),)
 endif
 
 # ansible-test parameters
-ANSIBLE_TEST ?= test/runner/ansible-test
+ANSIBLE_TEST ?= bin/ansible-test
 TEST_FLAGS ?=
 
 # ansible-test units parameters (make test / make test-py3)

test/runner/injector/injector.py

@@ -25,7 +25,6 @@ NOTE: Running ansible-test with the --tox option or inside a virtual environment
 
 from __future__ import absolute_import, print_function
 
-import errno
 import json
 import os
 import sys
@@ -203,10 +202,10 @@ def find_executable(executable):
     :rtype: str
     """
     self = os.path.abspath(__file__)
-    path = os.environ.get('PATH', os.defpath)
+    path = os.environ.get('PATH', os.path.defpath)
     seen_dirs = set()
 
-    for path_dir in path.split(os.pathsep):
+    for path_dir in path.split(os.path.pathsep):
         if path_dir in seen_dirs:
             continue
 

test/runner/lib/ansible_util.py

@@ -25,8 +25,8 @@ def ansible_environment(args, color=True):
 
     ansible_path = os.path.join(os.getcwd(), 'bin')
 
-    if not path.startswith(ansible_path + os.pathsep):
+    if not path.startswith(ansible_path + os.path.pathsep):
-        path = ansible_path + os.pathsep + path
+        path = ansible_path + os.path.pathsep + path
 
     if isinstance(args, IntegrationConfig):
         ansible_config = 'test/integration/%s.cfg' % args.command

test/runner/lib/cloud/acme.py

@@ -28,13 +28,6 @@ from lib.docker_util import (
     get_docker_container_id,
 )
 
-try:
-    # noinspection PyPep8Naming
-    import ConfigParser as configparser
-except ImportError:
-    # noinspection PyUnresolvedReferences
-    import configparser
-
 
 class ACMEProvider(CloudProvider):
     """ACME plugin. Sets up cloud resources for tests."""

test/runner/lib/cloud/cs.py

@@ -17,6 +17,7 @@ from lib.util import (
     display,
     SubprocessError,
     is_shippable,
+    ConfigParser,
 )
 
 from lib.http import (
@@ -34,13 +35,6 @@ from lib.docker_util import (
     get_docker_container_id,
 )
 
-try:
-    # noinspection PyPep8Naming
-    import ConfigParser as configparser
-except ImportError:
-    # noinspection PyUnresolvedReferences
-    import configparser
-
 
 class CsCloudProvider(CloudProvider):
     """CloudStack cloud provider plugin. Sets up cloud resources before delegation."""
@@ -119,7 +113,7 @@ class CsCloudProvider(CloudProvider):
 
     def _setup_static(self):
         """Configure CloudStack tests for use with static configuration."""
-        parser = configparser.RawConfigParser()
+        parser = ConfigParser()
         parser.read(self.config_static_path)
 
         self.endpoint = parser.get('cloudstack', 'endpoint')
@@ -211,7 +205,7 @@ class CsCloudProvider(CloudProvider):
             containers = bridge['Containers']
             container = [containers[container] for container in containers if containers[container]['Name'] == self.DOCKER_SIMULATOR_NAME][0]
             return re.sub(r'/[0-9]+$', '', container['IPv4Address'])
-        except:
+        except Exception:
             display.error('Failed to process the following docker network inspect output:\n%s' %
                           json.dumps(networks, indent=4, sort_keys=True))
             raise

test/runner/lib/cloud/gcp.py

@@ -6,9 +6,7 @@ from __future__ import absolute_import, print_function
 import os
 
 from lib.util import (
-    ApplicationError,
     display,
-    is_shippable,
 )
 
 from lib.cloud import (
@@ -16,9 +14,6 @@ from lib.cloud import (
     CloudEnvironment,
 )
 
-from lib.core_ci import (
-    AnsibleCoreCI, )
-
 
 class GcpCloudProvider(CloudProvider):
     """GCP cloud provider plugin. Sets up cloud resources before delegation."""

test/runner/lib/cloud/opennebula.py

@@ -1,17 +1,12 @@
 """OpenNebula plugin for integration tests."""
 
-import os
-
 from lib.cloud import (
     CloudProvider,
     CloudEnvironment
 )
 
 from lib.util import (
-    find_executable,
-    ApplicationError,
     display,
-    is_shippable,
 )
 
 

test/runner/lib/cloud/tower.py

@@ -4,20 +4,13 @@ from __future__ import absolute_import, print_function
 import os
 import time
 
-try:
-    # noinspection PyPep8Naming
-    import ConfigParser as configparser
-except ImportError:
-    # noinspection PyUnresolvedReferences
-    import configparser
-
 from lib.util import (
     display,
     ApplicationError,
     is_shippable,
     run_command,
-    generate_password,
     SubprocessError,
+    ConfigParser,
 )
 
 from lib.cloud import (
@@ -27,15 +20,6 @@ from lib.cloud import (
 
 from lib.core_ci import (
     AnsibleCoreCI,
-    InstanceConnection,
-)
-
-from lib.manage_ci import (
-    ManagePosixCI,
-)
-
-from lib.http import (
-    HttpClient,
 )
 
 
@@ -219,7 +203,7 @@ class TowerConfig(object):
         :type path: str
         :rtype: TowerConfig
         """
-        parser = configparser.RawConfigParser()
+        parser = ConfigParser()
         parser.read(path)
 
         keys = (

test/runner/lib/cloud/vcenter.py

@@ -21,13 +21,6 @@ from lib.docker_util import (
     get_docker_container_id,
 )
 
-try:
-    # noinspection PyPep8Naming
-    import ConfigParser as configparser
-except ImportError:
-    # noinspection PyUnresolvedReferences
-    import configparser
-
 
 class VcenterProvider(CloudProvider):
     """VMware vcenter/esx plugin. Sets up cloud resources for tests."""

test/runner/lib/docker_util.py

@@ -172,8 +172,8 @@ def docker_inspect(args, container_id):
     except SubprocessError as ex:
         try:
             return json.loads(ex.stdout)
-        except:
+        except Exception:
-            raise ex  # pylint: disable=locally-disabled, raising-bad-type
+            raise ex
 
 
 def docker_network_disconnect(args, container_id, network):
@@ -200,8 +200,8 @@ def docker_network_inspect(args, network):
     except SubprocessError as ex:
         try:
             return json.loads(ex.stdout)
-        except:
+        except Exception:
-            raise ex  # pylint: disable=locally-disabled, raising-bad-type
+            raise ex
 
 
 def docker_exec(args, container_id, cmd, options=None, capture=False, stdin=None, stdout=None):

test/runner/lib/executor.py

@@ -541,6 +541,7 @@ def command_windows_integration(args):
                 instance.result.stop()
 
 
+# noinspection PyUnusedLocal
 def windows_init(args, internal_targets):  # pylint: disable=locally-disabled, unused-argument
     """
     :type args: WindowsIntegrationConfig

test/runner/lib/sanity/compile.py

@@ -2,7 +2,6 @@
 from __future__ import absolute_import, print_function
 
 import os
-import re
 
 from lib.sanity import (
     SanityMultipleVersion,

test/runner/lib/sanity/import.py

@@ -2,7 +2,6 @@
 from __future__ import absolute_import, print_function
 
 import os
-import re
 
 from lib.sanity import (
     SanityMultipleVersion,

test/runner/lib/sanity/pylint.py

@@ -6,11 +6,6 @@ import json
 import os
 import datetime
 
-try:
-    import ConfigParser as configparser
-except ImportError:
-    import configparser
-
 from lib.sanity import (
     SanitySingleVersion,
     SanityMessage,
@@ -23,8 +18,8 @@ from lib.util import (
     SubprocessError,
     run_command,
     display,
-    find_executable,
     read_lines_without_comments,
+    ConfigParser,
 )
 
 from lib.executor import (
@@ -245,7 +240,7 @@ class PylintTest(SanitySingleVersion):
         if not os.path.exists(rcfile):
             rcfile = 'test/sanity/pylint/config/default'
 
-        parser = configparser.SafeConfigParser()
+        parser = ConfigParser()
         parser.read(rcfile)
 
         if parser.has_section('ansible-test'):
@@ -268,7 +263,7 @@ class PylintTest(SanitySingleVersion):
         ] + paths
 
         env = ansible_environment(args)
-        env['PYTHONPATH'] += '%s%s' % (os.pathsep, self.plugin_dir)
+        env['PYTHONPATH'] += '%s%s' % (os.path.pathsep, self.plugin_dir)
 
         if paths:
             try:

test/runner/lib/sanity/rstcheck.py

@@ -16,7 +16,6 @@ from lib.util import (
     run_command,
     parse_to_list_of_dict,
     display,
-    find_executable,
     read_lines_without_comments,
 )
 

test/runner/lib/sanity/yamllint.py

@@ -62,7 +62,8 @@ class YamllintTest(SanitySingleVersion):
 
         return SanitySuccess(self.name)
 
-    def test_paths(self, args, paths):
+    @staticmethod
+    def test_paths(args, paths):
         """
         :type args: SanityConfig
         :type paths: list[str]

test/runner/lib/thread.py

@@ -30,7 +30,7 @@ class WrappedThread(threading.Thread):
         Run action and capture results or exception.
         Do not override. Do not call directly. Executed by the start() method.
         """
-        # noinspection PyBroadException
+        # noinspection PyBroadException, PyPep8
         try:
             self._result.put((self.action(), None))
         except:  # pylint: disable=locally-disabled, bare-except

test/runner/lib/util.py

@@ -5,7 +5,6 @@ from __future__ import absolute_import, print_function
 import atexit
 import contextlib
 import errno
-import filecmp
 import fcntl
 import inspect
 import json
@@ -32,6 +31,13 @@ except ImportError:
     from abc import ABCMeta
     ABC = ABCMeta('ABC', (), {})
 
+try:
+    # noinspection PyCompatibility
+    from ConfigParser import SafeConfigParser as ConfigParser
+except ImportError:
+    # noinspection PyCompatibility
+    from configparser import ConfigParser
+
 DOCKER_COMPLETION = {}
 
 coverage_path = ''  # pylint: disable=locally-disabled, invalid-name
@@ -117,10 +123,10 @@ def find_executable(executable, cwd=None, path=None, required=True):
             match = executable
     else:
         if path is None:
-            path = os.environ.get('PATH', os.defpath)
+            path = os.environ.get('PATH', os.path.defpath)
 
         if path:
-            path_dirs = path.split(os.pathsep)
+            path_dirs = path.split(os.path.pathsep)
             seen_dirs = set()
 
             for path_dir in path_dirs:
@@ -197,7 +203,7 @@ def intercept_command(args, cmd, target_name, capture=False, env=None, data=None
     coverage_file = os.path.abspath(os.path.join(inject_path, '..', 'output', '%s=%s=%s=%s=coverage' % (
         args.command, target_name, args.coverage_label or 'local-%s' % version, 'python-%s' % version)))
 
-    env['PATH'] = inject_path + os.pathsep + env['PATH']
+    env['PATH'] = inject_path + os.path.pathsep + env['PATH']
     env['ANSIBLE_TEST_PYTHON_VERSION'] = version
     env['ANSIBLE_TEST_PYTHON_INTERPRETER'] = interpreter
 
@@ -388,7 +394,7 @@ def common_environment():
     """Common environment used for executing all programs."""
     env = dict(
         LC_ALL='en_US.UTF-8',
-        PATH=os.environ.get('PATH', os.defpath),
+        PATH=os.environ.get('PATH', os.path.defpath),
     )
 
     required = (

test/runner/setup/docker.sh

@@ -17,6 +17,7 @@ if [ ! -f /usr/bin/virtualenv ] && [ -f /usr/bin/virtualenv-3 ]; then
 fi
 
 # Improve prompts on remote host for interactive use.
+# shellcheck disable=SC1117
 cat << EOF > ~/.bashrc
 alias ls='ls --color=auto'
 export PS1='\[\e]0;\u@\h: \w\a\]\[\033[01;32m\]\u@\h\[\033[00m\]:\[\033[01;34m\]\w\[\033[00m\]\$ '

test/runner/setup/remote.sh

@@ -76,6 +76,7 @@ if [ ! -f "${HOME}/.ssh/id_rsa.pub" ]; then
 fi
 
 # Improve prompts on remote host for interactive use.
+# shellcheck disable=SC1117
 cat << EOF > ~/.bashrc
 alias ls='ls -G'
 export PS1='\[\e]0;\u@\h: \w\a\]\[\033[01;32m\]\u@\h\[\033[00m\]:\[\033[01;34m\]\w\[\033[00m\]\$ '

test/sanity/code-smell/empty-init.py

@@ -1,7 +1,6 @@
 #!/usr/bin/env python
 
 import os
-import re
 import sys
 
 

test/sanity/pylint/config/ansible-test

@@ -1,7 +1,6 @@
 [MESSAGES CONTROL]
 
 disable=
-    no-self-use,
     too-few-public-methods,
     too-many-arguments,
     too-many-branches,

test/sanity/pylint/ignore.txt

@@ -63,48 +63,3 @@ lib/ansible/modules/storage/infinidat/infini_vol.py ansible-format-automatic-spe
 lib/ansible/modules/storage/purestorage/purefa_host.py ansible-format-automatic-specification
 lib/ansible/modules/storage/purestorage/purefa_pg.py ansible-format-automatic-specification
 lib/ansible/modules/system/firewalld.py ansible-format-automatic-specification
-test/runner/injector/importer.py missing-docstring 3.7
-test/runner/injector/injector.py missing-docstring 3.7
-test/runner/lib/ansible_util.py missing-docstring 3.7
-test/runner/lib/changes.py missing-docstring 3.7
-test/runner/lib/classification.py missing-docstring 3.7
-test/runner/lib/cloud/__init__.py missing-docstring 3.7
-test/runner/lib/cloud/aws.py missing-docstring 3.7
-test/runner/lib/cloud/azure.py missing-docstring 3.7
-test/runner/lib/cloud/cs.py missing-docstring 3.7
-test/runner/lib/cloud/vcenter.py missing-docstring 3.7
-test/runner/lib/config.py missing-docstring 3.7
-test/runner/lib/core_ci.py missing-docstring 3.7
-test/runner/lib/cover.py missing-docstring 3.7
-test/runner/lib/delegation.py missing-docstring 3.7
-test/runner/lib/delegation.py redefined-variable-type 2.7
-test/runner/lib/diff.py missing-docstring 3.7
-test/runner/lib/docker_util.py missing-docstring 3.7
-test/runner/lib/executor.py missing-docstring 3.7
-test/runner/lib/git.py missing-docstring 3.7
-test/runner/lib/http.py missing-docstring 3.7
-test/runner/lib/import_analysis.py missing-docstring 3.7
-test/runner/lib/manage_ci.py missing-docstring 3.7
-test/runner/lib/metadata.py missing-docstring 3.7
-test/runner/lib/powershell_import_analysis.py missing-docstring 3.7
-test/runner/lib/pytar.py missing-docstring 3.7
-test/runner/lib/sanity/__init__.py missing-docstring 3.7
-test/runner/lib/sanity/ansible_doc.py missing-docstring 3.7
-test/runner/lib/sanity/compile.py missing-docstring 3.7
-test/runner/lib/sanity/import.py missing-docstring 3.7
-test/runner/lib/sanity/pep8.py missing-docstring 3.7
-test/runner/lib/sanity/pslint.py missing-docstring 3.7
-test/runner/lib/sanity/pylint.py missing-docstring 3.7
-test/runner/lib/sanity/rstcheck.py missing-docstring 3.7
-test/runner/lib/sanity/sanity_docs.py missing-docstring 3.7
-test/runner/lib/sanity/shellcheck.py missing-docstring 3.7
-test/runner/lib/sanity/validate_modules.py missing-docstring 3.7
-test/runner/lib/sanity/yamllint.py missing-docstring 3.7
-test/runner/lib/target.py missing-docstring 3.7
-test/runner/lib/test.py missing-docstring 3.7
-test/runner/lib/thread.py missing-docstring 3.7
-test/runner/lib/util.py missing-docstring 3.7
-test/runner/retry.py missing-docstring 3.7
-test/runner/shippable.py missing-docstring 3.7
-test/runner/units/test_diff.py missing-docstring 3.7
-test/sanity/import/importer.py missing-docstring 3.7

test/utils/shippable/shippable.sh

@@ -23,10 +23,10 @@ if [ -d /home/shippable/cache/ ]; then
     ls -la /home/shippable/cache/
 fi
 
-which python
+command -v python
 python -V
 
-which pip
+command -v pip
 pip --version
 pip list --disable-pip-version-check