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.

24 files changed, 36 insertions, 128 deletions

diff --git a/Makefile b/Makefile
index 5f6b4d4601..a7d21b3a60 100644
--- a/Makefile
+++ b/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)
diff --git a/test/runner/injector/injector.py b/test/runner/injector/injector.py
index a7f107b064..054eba71c4 100755
--- a/test/runner/injector/injector.py
+++ b/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
 
diff --git a/test/runner/lib/ansible_util.py b/test/runner/lib/ansible_util.py
index a9f6a06f99..545d067158 100644
--- a/test/runner/lib/ansible_util.py
+++ b/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):
-        path = ansible_path + os.pathsep + path
+    if not path.startswith(ansible_path + os.path.pathsep):
+        path = ansible_path + os.path.pathsep + path
 
     if isinstance(args, IntegrationConfig):
         ansible_config = 'test/integration/%s.cfg' % args.command
diff --git a/test/runner/lib/cloud/acme.py b/test/runner/lib/cloud/acme.py
index 0676cb30de..4e4646e9fa 100644
--- a/test/runner/lib/cloud/acme.py
+++ b/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."""
diff --git a/test/runner/lib/cloud/cs.py b/test/runner/lib/cloud/cs.py
index cdba9b1d50..014f49b3da 100644
--- a/test/runner/lib/cloud/cs.py
+++ b/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
diff --git a/test/runner/lib/cloud/gcp.py b/test/runner/lib/cloud/gcp.py
index 2f3255fbd9..26fd0cb37c 100644
--- a/test/runner/lib/cloud/gcp.py
+++ b/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."""
diff --git a/test/runner/lib/cloud/opennebula.py b/test/runner/lib/cloud/opennebula.py
index 4cbe5faf46..107121b029 100644
--- a/test/runner/lib/cloud/opennebula.py
+++ b/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,
 )
 
 
diff --git a/test/runner/lib/cloud/tower.py b/test/runner/lib/cloud/tower.py
index c05a32d3f6..89c7b985bf 100644
--- a/test/runner/lib/cloud/tower.py
+++ b/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 = (
diff --git a/test/runner/lib/cloud/vcenter.py b/test/runner/lib/cloud/vcenter.py
index ab9253726a..574eb1145a 100644
--- a/test/runner/lib/cloud/vcenter.py
+++ b/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."""
diff --git a/test/runner/lib/docker_util.py b/test/runner/lib/docker_util.py
index afa81d93b7..033f4d84e7 100644
--- a/test/runner/lib/docker_util.py
+++ b/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:
-            raise ex  # pylint: disable=locally-disabled, raising-bad-type
+        except Exception:
+            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:
-            raise ex  # pylint: disable=locally-disabled, raising-bad-type
+        except Exception:
+            raise ex
 
 
 def docker_exec(args, container_id, cmd, options=None, capture=False, stdin=None, stdout=None):
diff --git a/test/runner/lib/executor.py b/test/runner/lib/executor.py
index 10caa0779b..5fcca6cbe9 100644
--- a/test/runner/lib/executor.py
+++ b/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
diff --git a/test/runner/lib/sanity/compile.py b/test/runner/lib/sanity/compile.py
index 633d8b70be..3d079732bc 100644
--- a/test/runner/lib/sanity/compile.py
+++ b/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,
diff --git a/test/runner/lib/sanity/import.py b/test/runner/lib/sanity/import.py
index 296fce0e82..5f63ac2110 100644
--- a/test/runner/lib/sanity/import.py
+++ b/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,
diff --git a/test/runner/lib/sanity/pylint.py b/test/runner/lib/sanity/pylint.py
index 9736ac56dc..bd25e42619 100644
--- a/test/runner/lib/sanity/pylint.py
+++ b/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:
diff --git a/test/runner/lib/sanity/rstcheck.py b/test/runner/lib/sanity/rstcheck.py
index 14a0dd0aad..a6a4226cf1 100644
--- a/test/runner/lib/sanity/rstcheck.py
+++ b/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,
 )
 
diff --git a/test/runner/lib/sanity/yamllint.py b/test/runner/lib/sanity/yamllint.py
index 75b42f6068..1073fb26eb 100644
--- a/test/runner/lib/sanity/yamllint.py
+++ b/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]
diff --git a/test/runner/lib/thread.py b/test/runner/lib/thread.py
index 8bded9e926..ce4717e008 100644
--- a/test/runner/lib/thread.py
+++ b/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
diff --git a/test/runner/lib/util.py b/test/runner/lib/util.py
index 30808f9fc7..5780ad9642 100644
--- a/test/runner/lib/util.py
+++ b/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 = (
diff --git a/test/runner/setup/docker.sh b/test/runner/setup/docker.sh
index 2d8b515e76..352413624e 100644
--- a/test/runner/setup/docker.sh
+++ b/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\]\$ '
diff --git a/test/runner/setup/remote.sh b/test/runner/setup/remote.sh
index c7510e3d1f..a23c164be0 100644
--- a/test/runner/setup/remote.sh
+++ b/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\]\$ '
diff --git a/test/sanity/code-smell/empty-init.py b/test/sanity/code-smell/empty-init.py
index 453bbe04a1..ee9f901885 100755
--- a/test/sanity/code-smell/empty-init.py
+++ b/test/sanity/code-smell/empty-init.py
@@ -1,7 +1,6 @@
 #!/usr/bin/env python
 
 import os
-import re
 import sys
 
 
diff --git a/test/sanity/pylint/config/ansible-test b/test/sanity/pylint/config/ansible-test
index b024f5aa93..a757d80cfe 100644
--- a/test/sanity/pylint/config/ansible-test
+++ b/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,
diff --git a/test/sanity/pylint/ignore.txt b/test/sanity/pylint/ignore.txt
index 69de8bcab4..e464cce360 100644
--- a/test/sanity/pylint/ignore.txt
+++ b/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
diff --git a/test/utils/shippable/shippable.sh b/test/utils/shippable/shippable.sh
index e6e2d7d88a..bf31da0643 100755
--- a/test/utils/shippable/shippable.sh
+++ b/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