Bug fixes and cleanup for ansible-test. (ansible#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.
empire · Sep 21, 2018 · ac49247 · ac49247
1 parent b608543
commit ac49247
Show file tree

Hide file tree

Showing 24 changed files with 36 additions and 128 deletions.
diff --git 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
@@ -25,7 +25,6 @@
 
 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
@@ -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
@@ -28,13 +28,6 @@
     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
@@ -17,6 +17,7 @@
     display,
     SubprocessError,
     is_shippable,
+    ConfigParser,
 )
 
 from lib.http import (
@@ -34,13 +35,6 @@
     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 @@ def cleanup(self):
 
     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 @@ def _get_simulator_address(self):
             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
@@ -6,19 +6,14 @@
 import os
 
 from lib.util import (
-    ApplicationError,
     display,
-    is_shippable,
 )
 
 from lib.cloud import (
     CloudProvider,
     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
@@ -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
@@ -4,20 +4,13 @@
 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.core_ci import (
     AnsibleCoreCI,
-    InstanceConnection,
-)
-
-from lib.manage_ci import (
-    ManagePosixCI,
-)
-
-from lib.http import (
-    HttpClient,
 )
 
 
@@ -219,7 +203,7 @@ def parse(path):
         :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
@@ -21,13 +21,6 @@
     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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -6,11 +6,6 @@
 import os
 import datetime
 
-try:
-    import ConfigParser as configparser
-except ImportError:
-    import configparser
-
 from lib.sanity import (
     SanitySingleVersion,
     SanityMessage,
@@ -23,8 +18,8 @@
     SubprocessError,
     run_command,
     display,
-    find_executable,
     read_lines_without_comments,
+    ConfigParser,
 )
 
 from lib.executor import (
@@ -245,7 +240,7 @@ def pylint(self, args, context, paths):
         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 @@ def pylint(self, args, context, paths):
         ] + 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
@@ -16,7 +16,6 @@
     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
@@ -62,7 +62,8 @@ def test(self, args, targets):
 
         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
@@ -30,7 +30,7 @@ def run(self):
         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
@@ -5,7 +5,6 @@
 import atexit
 import contextlib
 import errno
-import filecmp
 import fcntl
 import inspect
 import json
@@ -32,6 +31,13 @@
     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
@@ -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
@@ -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
@@ -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
@@ -1,7 +1,6 @@
 [MESSAGES CONTROL]
 
 disable=
-    no-self-use,
     too-few-public-methods,
     too-many-arguments,
     too-many-branches,