From f81238012b36ed57ca49629629e82ac7b89d56f2 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Mon, 15 Jul 2019 13:47:16 -0700 Subject: [PATCH] Clean up pylint issues in test infra. --- test/runner/lib/cloud/cs.py | 4 ++-- test/runner/lib/core_ci.py | 4 ++-- test/runner/lib/delegation.py | 2 +- test/runner/lib/docker_util.py | 8 ++++---- test/runner/lib/executor.py | 8 ++++---- test/runner/lib/http.py | 2 +- test/runner/lib/import_analysis.py | 2 +- test/runner/lib/target.py | 2 +- test/runner/lib/util.py | 2 +- test/sanity/import/importer.py | 2 +- test/sanity/pylint/ignore.txt | 11 ----------- test/sanity/validate-modules/main.py | 12 ++++++------ 12 files changed, 24 insertions(+), 35 deletions(-) diff --git a/test/runner/lib/cloud/cs.py b/test/runner/lib/cloud/cs.py index 5805575c8d6..a5cbe5d8f36 100644 --- a/test/runner/lib/cloud/cs.py +++ b/test/runner/lib/cloud/cs.py @@ -226,7 +226,7 @@ class CsCloudProvider(CloudProvider): client = HttpClient(self.args, always=True) endpoint = self.endpoint - for _ in range(1, 30): + for _iteration in range(1, 30): display.info('Waiting for CloudStack service: %s' % endpoint, verbosity=1) try: @@ -246,7 +246,7 @@ class CsCloudProvider(CloudProvider): client = HttpClient(self.args, always=True) endpoint = '%s/admin.json' % self.endpoint - for _ in range(1, 30): + for _iteration in range(1, 30): display.info('Waiting for CloudStack credentials: %s' % endpoint, verbosity=1) response = client.get(endpoint) diff --git a/test/runner/lib/core_ci.py b/test/runner/lib/core_ci.py index c31c1649a94..52f2b25ab7d 100644 --- a/test/runner/lib/core_ci.py +++ b/test/runner/lib/core_ci.py @@ -186,7 +186,7 @@ class AnsibleCoreCI: display.info('Getting available endpoints...', verbosity=1) sleep = 3 - for _ in range(1, 10): + for _iteration in range(1, 10): response = client.get('https://s3.amazonaws.com/ansible-ci-files/ansible-test/parallels-endpoints.txt') if response.status_code == 200: @@ -324,7 +324,7 @@ class AnsibleCoreCI: def wait(self): """Wait for the instance to become ready.""" - for _ in range(1, 90): + for _iteration in range(1, 90): if self.get().running: return time.sleep(10) diff --git a/test/runner/lib/delegation.py b/test/runner/lib/delegation.py index 99c8671cab7..94709ae4ae8 100644 --- a/test/runner/lib/delegation.py +++ b/test/runner/lib/delegation.py @@ -288,7 +288,7 @@ def delegate_docker(args, exclude, require, integration_targets): for cloud_platform in cloud_platforms: test_options += cloud_platform.get_docker_run_options() - test_id, _ = docker_run(args, test_image, options=test_options) + test_id = docker_run(args, test_image, options=test_options)[0] if args.explain: test_id = 'test_id' diff --git a/test/runner/lib/docker_util.py b/test/runner/lib/docker_util.py index c481bbfba52..b92a7d9d5d8 100644 --- a/test/runner/lib/docker_util.py +++ b/test/runner/lib/docker_util.py @@ -91,7 +91,7 @@ def docker_pull(args, image): display.warning('Skipping docker pull for "%s". Image may be out-of-date.' % image) return - for _ in range(1, 10): + for _iteration in range(1, 10): try: docker_command(args, ['pull', image]) return @@ -142,7 +142,7 @@ def docker_run(args, image, options, cmd=None): if not cmd: cmd = [] - for _ in range(1, 3): + for _iteration in range(1, 3): try: return docker_command(args, ['run'] + options + [image] + cmd, capture=True) except SubprocessError as ex: @@ -182,7 +182,7 @@ def docker_inspect(args, container_id): return [] try: - stdout, _ = docker_command(args, ['inspect', container_id], capture=True) + stdout = docker_command(args, ['inspect', container_id], capture=True)[0] return json.loads(stdout) except SubprocessError as ex: try: @@ -210,7 +210,7 @@ def docker_network_inspect(args, network): return [] try: - stdout, _ = docker_command(args, ['network', 'inspect', network], capture=True) + stdout = docker_command(args, ['network', 'inspect', network], capture=True)[0] return json.loads(stdout) except SubprocessError as ex: try: diff --git a/test/runner/lib/executor.py b/test/runner/lib/executor.py index d37e59a6f60..956e60f2581 100644 --- a/test/runner/lib/executor.py +++ b/test/runner/lib/executor.py @@ -152,7 +152,7 @@ def check_legacy_modules(): for directory in 'core', 'extras': path = 'lib/ansible/modules/%s' % directory - for root, _, file_names in os.walk(path): + for root, _dir_names, file_names in os.walk(path): if file_names: # the directory shouldn't exist, but if it does, it must contain no files raise ApplicationError('Files prohibited in "%s". ' @@ -271,7 +271,7 @@ def pip_list(args, pip): :type pip: list[str] :rtype: str """ - stdout, _ = run_command(args, pip + ['list'], capture=True) + stdout = run_command(args, pip + ['list'], capture=True)[0] return stdout @@ -841,7 +841,7 @@ def command_integration_filtered(args, targets, all_targets, inventory_path, pre # common temporary directory path that will be valid on both the controller and the remote # it must be common because it will be referenced in environment variables that are shared across multiple hosts - common_temp_path = '/tmp/ansible-test-%s' % ''.join(random.choice(string.ascii_letters + string.digits) for _ in range(8)) + common_temp_path = '/tmp/ansible-test-%s' % ''.join(random.choice(string.ascii_letters + string.digits) for _idx in range(8)) setup_common_temp_dir(args, common_temp_path) @@ -1039,7 +1039,7 @@ def run_httptester(args, ports=None): for localhost_port, container_port in ports.items(): options += ['-p', '%d:%d' % (localhost_port, container_port)] - httptester_id, _ = docker_run(args, args.httptester, options=options) + httptester_id = docker_run(args, args.httptester, options=options)[0] if args.explain: httptester_id = 'httptester_id' diff --git a/test/runner/lib/http.py b/test/runner/lib/http.py index 59d6128ba51..18a429c51fd 100644 --- a/test/runner/lib/http.py +++ b/test/runner/lib/http.py @@ -123,7 +123,7 @@ class HttpClient: attempts += 1 try: - stdout, _ = run_command(self.args, cmd, capture=True, always=self.always, cmd_verbosity=2) + stdout = run_command(self.args, cmd, capture=True, always=self.always, cmd_verbosity=2)[0] break except SubprocessError as ex: if ex.status in retry_on_status and attempts < max_attempts: diff --git a/test/runner/lib/import_analysis.py b/test/runner/lib/import_analysis.py index dd511c1e4f9..f9e452ff437 100644 --- a/test/runner/lib/import_analysis.py +++ b/test/runner/lib/import_analysis.py @@ -122,7 +122,7 @@ def enumerate_module_utils(): module_utils = [] base_path = 'lib/ansible/module_utils' - for root, _, file_names in os.walk(base_path): + for root, _dir_names, file_names in os.walk(base_path): for file_name in file_names: path = os.path.join(root, file_name) name, ext = os.path.splitext(file_name) diff --git a/test/runner/lib/target.py b/test/runner/lib/target.py index 44225fadbb7..dbe8ec2f407 100644 --- a/test/runner/lib/target.py +++ b/test/runner/lib/target.py @@ -243,7 +243,7 @@ def walk_test_targets(path=None, module_path=None, extensions=None, prefix=None, :type extra_dirs: tuple[str] | None :rtype: collections.Iterable[TestTarget] """ - for root, _, file_names in os.walk(path or '.', topdown=False): + for root, _dir_names, file_names in os.walk(path or '.', topdown=False): if root.endswith('/__pycache__'): continue diff --git a/test/runner/lib/util.py b/test/runner/lib/util.py index d00182f2fc1..be2784d9c0f 100644 --- a/test/runner/lib/util.py +++ b/test/runner/lib/util.py @@ -758,7 +758,7 @@ def import_plugins(directory, root=None): # type: (str, t.Optional[str]) -> Non path = os.path.join(root, directory) prefix = 'lib.%s.' % directory.replace(os.sep, '.') - for (_, name, _) in pkgutil.iter_modules([path], prefix=prefix): + for (_module_loader, name, _ispkg) in pkgutil.iter_modules([path], prefix=prefix): module_path = os.path.join(root, name[4:].replace('.', os.sep) + '.py') load_module(module_path, name) diff --git a/test/sanity/import/importer.py b/test/sanity/import/importer.py index 975f15c5f21..43bb9e622a3 100755 --- a/test/sanity/import/importer.py +++ b/test/sanity/import/importer.py @@ -104,7 +104,7 @@ def test_python_module(path, base_dir, messages, ansible_module): except BaseException as ex: # pylint: disable=locally-disabled, broad-except capture_report(path, capture, messages) - exc_type, _, exc_tb = sys.exc_info() + exc_type, _exc, exc_tb = sys.exc_info() message = str(ex) results = list(reversed(traceback.extract_tb(exc_tb))) source = None diff --git a/test/sanity/pylint/ignore.txt b/test/sanity/pylint/ignore.txt index ab52a7e1403..767d49e42ad 100644 --- a/test/sanity/pylint/ignore.txt +++ b/test/sanity/pylint/ignore.txt @@ -83,17 +83,6 @@ test/integration/targets/module_utils/module_utils/sub/bar/bar.py blacklisted-na test/integration/targets/module_utils/module_utils/yak/zebra/foo.py blacklisted-name test/legacy/cleanup_gce.py blacklisted-name test/legacy/gce_credentials.py blacklisted-name -test/runner/lib/cloud/cs.py blacklisted-name -test/runner/lib/core_ci.py blacklisted-name -test/runner/lib/delegation.py blacklisted-name -test/runner/lib/docker_util.py blacklisted-name -test/runner/lib/executor.py blacklisted-name -test/runner/lib/http.py blacklisted-name -test/runner/lib/import_analysis.py blacklisted-name -test/runner/lib/target.py blacklisted-name -test/runner/lib/util.py blacklisted-name -test/sanity/import/importer.py blacklisted-name -test/sanity/validate-modules/main.py blacklisted-name test/units/contrib/inventory/test_vmware_inventory.py blacklisted-name test/units/executor/test_play_iterator.py blacklisted-name test/units/module_utils/basic/test_run_command.py blacklisted-name diff --git a/test/sanity/validate-modules/main.py b/test/sanity/validate-modules/main.py index ff09138e202..42bdb8eeca9 100755 --- a/test/sanity/validate-modules/main.py +++ b/test/sanity/validate-modules/main.py @@ -186,7 +186,7 @@ class Reporter: warnings is not respected in this output """ - ret = [len(r['errors']) for _, r in self.files.items()] + ret = [len(r['errors']) for r in self.files.values()] with Reporter._output_handle(output) as handle: print(json.dumps(Reporter._filter_out_ok(self.files), indent=4, cls=ReporterEncoder), file=handle) @@ -242,7 +242,7 @@ class ModuleValidator(Validator): self.path = path self.basename = os.path.basename(self.path) - self.name, _ = os.path.splitext(self.basename) + self.name = os.path.splitext(self.basename)[0] self.analyze_arg_spec = analyze_arg_spec @@ -1022,9 +1022,9 @@ class ModuleValidator(Validator): msg='No EXAMPLES provided' ) else: - _, errors, traces = parse_yaml(doc_info['EXAMPLES']['value'], - doc_info['EXAMPLES']['lineno'], - self.name, 'EXAMPLES', load_all=True) + _doc, errors, traces = parse_yaml(doc_info['EXAMPLES']['value'], + doc_info['EXAMPLES']['lineno'], + self.name, 'EXAMPLES', load_all=True) for error in errors: self.reporter.error( path=self.object_path, @@ -1487,7 +1487,7 @@ class ModuleValidator(Validator): @staticmethod def is_blacklisted(path): base_name = os.path.basename(path) - file_name, _ = os.path.splitext(base_name) + file_name = os.path.splitext(base_name)[0] if file_name.startswith('_') and os.path.islink(path): return True