From ac492476e5389e17b8d401174f18b73e59a7fb06 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Fri, 21 Sep 2018 11:38:22 -0700 Subject: [PATCH] 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. --- Makefile | 2 +- test/runner/injector/injector.py | 5 ++- test/runner/lib/ansible_util.py | 4 +-- test/runner/lib/cloud/acme.py | 7 ---- test/runner/lib/cloud/cs.py | 12 ++----- test/runner/lib/cloud/gcp.py | 5 --- test/runner/lib/cloud/opennebula.py | 5 --- test/runner/lib/cloud/tower.py | 20 ++---------- test/runner/lib/cloud/vcenter.py | 7 ---- test/runner/lib/docker_util.py | 8 ++--- test/runner/lib/executor.py | 1 + test/runner/lib/sanity/compile.py | 1 - test/runner/lib/sanity/import.py | 1 - test/runner/lib/sanity/pylint.py | 11 ++----- test/runner/lib/sanity/rstcheck.py | 1 - test/runner/lib/sanity/yamllint.py | 3 +- test/runner/lib/thread.py | 2 +- test/runner/lib/util.py | 16 ++++++--- test/runner/setup/docker.sh | 1 + test/runner/setup/remote.sh | 1 + test/sanity/code-smell/empty-init.py | 1 - test/sanity/pylint/config/ansible-test | 1 - test/sanity/pylint/ignore.txt | 45 -------------------------- test/utils/shippable/shippable.sh | 4 +-- 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