From 2fbee1d183a643d1fe3aeb644c3dfa7d40c36f6c Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Wed, 19 Sep 2018 23:20:27 -0700 Subject: [PATCH] Support comments in ansible-test flat files. (cherry picked from commit 5a3000af19b81c1baf592e970683c7fd0ac0d43f) --- test/runner/lib/cli.py | 16 +++--- test/runner/lib/sanity/__init__.py | 5 +- test/runner/lib/sanity/ansible_doc.py | 5 +- test/runner/lib/sanity/compile.py | 8 +-- test/runner/lib/sanity/import.py | 8 +-- test/runner/lib/sanity/pep8.py | 22 +++++---- test/runner/lib/sanity/pslint.py | 37 ++++++++------ test/runner/lib/sanity/pylint.py | 57 ++++++++++++---------- test/runner/lib/sanity/rstcheck.py | 5 +- test/runner/lib/sanity/shellcheck.py | 9 ++-- test/runner/lib/sanity/validate_modules.py | 34 +++++++------ test/runner/lib/target.py | 5 +- test/runner/lib/util.py | 20 +++++++- 13 files changed, 135 insertions(+), 96 deletions(-) diff --git a/test/runner/lib/cli.py b/test/runner/lib/cli.py index 4f404fe91f..b32887807d 100644 --- a/test/runner/lib/cli.py +++ b/test/runner/lib/cli.py @@ -12,6 +12,7 @@ from lib.util import ( raw_command, get_docker_completion, generate_pip_command, + read_lines_without_comments, ) from lib.delegation import ( @@ -687,8 +688,7 @@ def complete_remote(prefix, parsed_args, **_): """ del parsed_args - with open('test/runner/completion/remote.txt', 'r') as completion_fd: - images = completion_fd.read().splitlines() + images = read_lines_without_comments('test/runner/completion/remote.txt', remove_blank_lines=True) return [i for i in images if i.startswith(prefix)] @@ -701,12 +701,10 @@ def complete_remote_shell(prefix, parsed_args, **_): """ del parsed_args - with open('test/runner/completion/remote.txt', 'r') as completion_fd: - images = completion_fd.read().splitlines() + images = read_lines_without_comments('test/runner/completion/remote.txt', remove_blank_lines=True) # 2008 doesn't support SSH so we do not add to the list of valid images - with open('test/runner/completion/windows.txt', 'r') as completion_fd: - images.extend(["windows/%s" % i for i in completion_fd.read().splitlines() if i != '2008']) + images.extend(["windows/%s" % i for i in read_lines_without_comments('test/runner/completion/windows.txt', remove_blank_lines=True) if i != '2008']) return [i for i in images if i.startswith(prefix)] @@ -730,8 +728,7 @@ def complete_windows(prefix, parsed_args, **_): :type parsed_args: any :rtype: list[str] """ - with open('test/runner/completion/windows.txt', 'r') as completion_fd: - images = completion_fd.read().splitlines() + images = read_lines_without_comments('test/runner/completion/windows.txt', remove_blank_lines=True) return [i for i in images if i.startswith(prefix) and (not parsed_args.windows or i not in parsed_args.windows)] @@ -742,8 +739,7 @@ def complete_network_platform(prefix, parsed_args, **_): :type parsed_args: any :rtype: list[str] """ - with open('test/runner/completion/network.txt', 'r') as completion_fd: - images = completion_fd.read().splitlines() + images = read_lines_without_comments('test/runner/completion/network.txt', remove_blank_lines=True) return [i for i in images if i.startswith(prefix) and (not parsed_args.platform or i not in parsed_args.platform)] diff --git a/test/runner/lib/sanity/__init__.py b/test/runner/lib/sanity/__init__.py index 57d237cf4a..9f8c9b8f11 100644 --- a/test/runner/lib/sanity/__init__.py +++ b/test/runner/lib/sanity/__init__.py @@ -18,6 +18,7 @@ from lib.util import ( parse_to_dict, ABC, is_binary_file, + read_lines_without_comments, ) from lib.ansible_util import ( @@ -134,8 +135,8 @@ def collect_code_smell_tests(): """ :rtype: tuple[SanityCodeSmellTest] """ - with open('test/sanity/code-smell/skip.txt', 'r') as skip_fd: - skip_tests = skip_fd.read().splitlines() + skip_file = 'test/sanity/code-smell/skip.txt' + skip_tests = read_lines_without_comments(skip_file, remove_blank_lines=True) paths = glob.glob('test/sanity/code-smell/*') paths = sorted(p for p in paths if os.access(p, os.X_OK) and os.path.isfile(p) and os.path.basename(p) not in skip_tests) diff --git a/test/runner/lib/sanity/ansible_doc.py b/test/runner/lib/sanity/ansible_doc.py index 2e5c73e809..a3b35d93e6 100644 --- a/test/runner/lib/sanity/ansible_doc.py +++ b/test/runner/lib/sanity/ansible_doc.py @@ -17,6 +17,7 @@ from lib.util import ( SubprocessError, display, intercept_command, + read_lines_without_comments, ) from lib.ansible_util import ( @@ -37,8 +38,8 @@ class AnsibleDocTest(SanityMultipleVersion): :type python_version: str :rtype: TestResult """ - with open('test/sanity/ansible-doc/skip.txt', 'r') as skip_fd: - skip_modules = set(skip_fd.read().splitlines()) + skip_file = 'test/sanity/ansible-doc/skip.txt' + skip_modules = set(read_lines_without_comments(skip_file, remove_blank_lines=True)) plugin_type_blacklist = set([ # not supported by ansible-doc diff --git a/test/runner/lib/sanity/compile.py b/test/runner/lib/sanity/compile.py index 9427fe73a0..b2ecb8f86b 100644 --- a/test/runner/lib/sanity/compile.py +++ b/test/runner/lib/sanity/compile.py @@ -17,6 +17,7 @@ from lib.util import ( run_command, display, find_python, + read_lines_without_comments, ) from lib.config import ( @@ -37,12 +38,10 @@ class CompileTest(SanityMultipleVersion): :type python_version: str :rtype: TestResult """ - # optional list of regex patterns to exclude from tests skip_file = 'test/sanity/compile/python%s-skip.txt' % python_version if os.path.exists(skip_file): - with open(skip_file, 'r') as skip_fd: - skip_paths = skip_fd.read().splitlines() + skip_paths = read_lines_without_comments(skip_file) else: skip_paths = [] @@ -87,6 +86,9 @@ class CompileTest(SanityMultipleVersion): for path in skip_paths: line += 1 + if not path: + continue + if not os.path.exists(path): # Keep files out of the list which no longer exist in the repo. results.append(SanityMessage( diff --git a/test/runner/lib/sanity/import.py b/test/runner/lib/sanity/import.py index 60daeff79e..ebe3893af5 100644 --- a/test/runner/lib/sanity/import.py +++ b/test/runner/lib/sanity/import.py @@ -19,6 +19,7 @@ from lib.util import ( remove_tree, display, find_python, + read_lines_without_comments, ) from lib.ansible_util import ( @@ -43,9 +44,8 @@ class ImportTest(SanityMultipleVersion): :type python_version: str :rtype: TestResult """ - with open('test/sanity/import/skip.txt', 'r') as skip_fd: - skip_paths = skip_fd.read().splitlines() - + skip_file = 'test/sanity/import/skip.txt' + skip_paths = read_lines_without_comments(skip_file, remove_blank_lines=True) skip_paths_set = set(skip_paths) paths = sorted( @@ -121,7 +121,7 @@ class ImportTest(SanityMultipleVersion): column=int(r['column']), ) for r in results] - results = [result for result in results if result.path not in skip_paths] + results = [result for result in results if result.path not in skip_paths_set] if results: return SanityFailure(self.name, messages=results, python_version=python_version) diff --git a/test/runner/lib/sanity/pep8.py b/test/runner/lib/sanity/pep8.py index 92ddd08341..cf48749385 100644 --- a/test/runner/lib/sanity/pep8.py +++ b/test/runner/lib/sanity/pep8.py @@ -15,6 +15,7 @@ from lib.util import ( SubprocessError, display, run_command, + read_lines_without_comments, ) from lib.config import ( @@ -37,17 +38,14 @@ class Pep8Test(SanitySingleVersion): :type targets: SanityTargets :rtype: TestResult """ - with open(PEP8_SKIP_PATH, 'r') as skip_fd: - skip_paths = skip_fd.read().splitlines() + skip_paths = read_lines_without_comments(PEP8_SKIP_PATH) + legacy_paths = read_lines_without_comments(PEP8_LEGACY_PATH) - with open(PEP8_LEGACY_PATH, 'r') as legacy_fd: - legacy_paths = legacy_fd.read().splitlines() + legacy_ignore_file = 'test/sanity/pep8/legacy-ignore.txt' + legacy_ignore = set(read_lines_without_comments(legacy_ignore_file, remove_blank_lines=True)) - with open('test/sanity/pep8/legacy-ignore.txt', 'r') as ignore_fd: - legacy_ignore = set(ignore_fd.read().splitlines()) - - with open('test/sanity/pep8/current-ignore.txt', 'r') as ignore_fd: - current_ignore = sorted(ignore_fd.read().splitlines()) + current_ignore_file = 'test/sanity/pep8/current-ignore.txt' + current_ignore = sorted(read_lines_without_comments(current_ignore_file, remove_blank_lines=True)) skip_paths_set = set(skip_paths) legacy_paths_set = set(legacy_paths) @@ -106,6 +104,9 @@ class Pep8Test(SanitySingleVersion): for path in legacy_paths: line += 1 + if not path: + continue + if not os.path.exists(path): # Keep files out of the list which no longer exist in the repo. errors.append(SanityMessage( @@ -133,6 +134,9 @@ class Pep8Test(SanitySingleVersion): for path in skip_paths: line += 1 + if not path: + continue + if not os.path.exists(path): # Keep files out of the list which no longer exist in the repo. errors.append(SanityMessage( diff --git a/test/runner/lib/sanity/pslint.py b/test/runner/lib/sanity/pslint.py index f2f39a9072..8e028e7cb5 100644 --- a/test/runner/lib/sanity/pslint.py +++ b/test/runner/lib/sanity/pslint.py @@ -18,6 +18,7 @@ from lib.util import ( SubprocessError, run_command, find_executable, + read_lines_without_comments, ) from lib.config import ( @@ -41,30 +42,31 @@ class PslintTest(SanitySingleVersion): :type targets: SanityTargets :rtype: TestResult """ - with open(PSLINT_SKIP_PATH, 'r') as skip_fd: - skip_paths = skip_fd.read().splitlines() + skip_paths = read_lines_without_comments(PSLINT_SKIP_PATH) invalid_ignores = [] - with open(PSLINT_IGNORE_PATH, 'r') as ignore_fd: - ignore_entries = ignore_fd.read().splitlines() - ignore = collections.defaultdict(dict) - line = 0 + ignore_entries = read_lines_without_comments(PSLINT_IGNORE_PATH) + ignore = collections.defaultdict(dict) + line = 0 - for ignore_entry in ignore_entries: - line += 1 + for ignore_entry in ignore_entries: + line += 1 - if ' ' not in ignore_entry: - invalid_ignores.append((line, 'Invalid syntax')) - continue + if not ignore_entry: + continue - path, code = ignore_entry.split(' ', 1) + if ' ' not in ignore_entry: + invalid_ignores.append((line, 'Invalid syntax')) + continue - if not os.path.exists(path): - invalid_ignores.append((line, 'Remove "%s" since it does not exist' % path)) - continue + path, code = ignore_entry.split(' ', 1) - ignore[path][code] = line + if not os.path.exists(path): + invalid_ignores.append((line, 'Remove "%s" since it does not exist' % path)) + continue + + ignore[path][code] = line paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] in ('.ps1', '.psm1', '.psd1') and i.path not in skip_paths) @@ -138,6 +140,9 @@ class PslintTest(SanitySingleVersion): for path in skip_paths: line += 1 + if not path: + continue + if not os.path.exists(path): # Keep files out of the list which no longer exist in the repo. errors.append(SanityMessage( diff --git a/test/runner/lib/sanity/pylint.py b/test/runner/lib/sanity/pylint.py index 86701b8d8a..9736ac56dc 100644 --- a/test/runner/lib/sanity/pylint.py +++ b/test/runner/lib/sanity/pylint.py @@ -24,6 +24,7 @@ from lib.util import ( run_command, display, find_executable, + read_lines_without_comments, ) from lib.executor import ( @@ -69,43 +70,44 @@ class PylintTest(SanitySingleVersion): display.warning('Skipping pylint on unsupported Python version %s.' % args.python_version) return SanitySkipped(self.name) - with open(PYLINT_SKIP_PATH, 'r') as skip_fd: - skip_paths = skip_fd.read().splitlines() + skip_paths = read_lines_without_comments(PYLINT_SKIP_PATH) invalid_ignores = [] supported_versions = set(SUPPORTED_PYTHON_VERSIONS) - set(UNSUPPORTED_PYTHON_VERSIONS) supported_versions = set([v.split('.')[0] for v in supported_versions]) | supported_versions - with open(PYLINT_IGNORE_PATH, 'r') as ignore_fd: - ignore_entries = ignore_fd.read().splitlines() - ignore = collections.defaultdict(dict) - line = 0 + ignore_entries = read_lines_without_comments(PYLINT_IGNORE_PATH) + ignore = collections.defaultdict(dict) + line = 0 - for ignore_entry in ignore_entries: - line += 1 + for ignore_entry in ignore_entries: + line += 1 - if ' ' not in ignore_entry: - invalid_ignores.append((line, 'Invalid syntax')) + if not ignore_entry: + continue + + if ' ' not in ignore_entry: + invalid_ignores.append((line, 'Invalid syntax')) + continue + + path, code = ignore_entry.split(' ', 1) + + if not os.path.exists(path): + invalid_ignores.append((line, 'Remove "%s" since it does not exist' % path)) + continue + + if ' ' in code: + code, version = code.split(' ', 1) + + if version not in supported_versions: + invalid_ignores.append((line, 'Invalid version: %s' % version)) continue - path, code = ignore_entry.split(' ', 1) + if version != args.python_version and version != args.python_version.split('.')[0]: + continue # ignore version specific entries for other versions - if not os.path.exists(path): - invalid_ignores.append((line, 'Remove "%s" since it does not exist' % path)) - continue - - if ' ' in code: - code, version = code.split(' ', 1) - - if version not in supported_versions: - invalid_ignores.append((line, 'Invalid version: %s' % version)) - continue - - if version != args.python_version and version != args.python_version.split('.')[0]: - continue # ignore version specific entries for other versions - - ignore[path][code] = line + ignore[path][code] = line skip_paths_set = set(skip_paths) @@ -193,6 +195,9 @@ class PylintTest(SanitySingleVersion): for path in skip_paths: line += 1 + if not path: + continue + if not os.path.exists(path): # Keep files out of the list which no longer exist in the repo. errors.append(SanityMessage( diff --git a/test/runner/lib/sanity/rstcheck.py b/test/runner/lib/sanity/rstcheck.py index 33bab62d37..9cd9d8313d 100644 --- a/test/runner/lib/sanity/rstcheck.py +++ b/test/runner/lib/sanity/rstcheck.py @@ -17,6 +17,7 @@ from lib.util import ( parse_to_dict, display, find_executable, + read_lines_without_comments, ) from lib.config import ( @@ -40,8 +41,8 @@ class RstcheckTest(SanitySingleVersion): display.warning('Skipping rstcheck on unsupported Python version %s.' % args.python_version) return SanitySkipped(self.name) - with open('test/sanity/rstcheck/ignore-substitutions.txt', 'r') as ignore_fd: - ignore_substitutions = sorted(set(ignore_fd.read().splitlines())) + ignore_file = 'test/sanity/rstcheck/ignore-substitutions.txt' + ignore_substitutions = sorted(set(read_lines_without_comments(ignore_file, remove_blank_lines=True))) paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] in ('.rst',)) diff --git a/test/runner/lib/sanity/shellcheck.py b/test/runner/lib/sanity/shellcheck.py index cee0c29f06..229391e304 100644 --- a/test/runner/lib/sanity/shellcheck.py +++ b/test/runner/lib/sanity/shellcheck.py @@ -19,6 +19,7 @@ from lib.sanity import ( from lib.util import ( SubprocessError, run_command, + read_lines_without_comments, ) from lib.config import ( @@ -34,11 +35,11 @@ class ShellcheckTest(SanitySingleVersion): :type targets: SanityTargets :rtype: TestResult """ - with open('test/sanity/shellcheck/skip.txt', 'r') as skip_fd: - skip_paths = set(skip_fd.read().splitlines()) + skip_file = 'test/sanity/shellcheck/skip.txt' + skip_paths = set(read_lines_without_comments(skip_file, remove_blank_lines=True)) - with open('test/sanity/shellcheck/exclude.txt', 'r') as exclude_fd: - exclude = set(exclude_fd.read().splitlines()) + exclude_file = 'test/sanity/shellcheck/exclude.txt' + exclude = set(read_lines_without_comments(exclude_file, remove_blank_lines=True)) paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] == '.sh' and i.path not in skip_paths) diff --git a/test/runner/lib/sanity/validate_modules.py b/test/runner/lib/sanity/validate_modules.py index e570281322..98f2c5f972 100644 --- a/test/runner/lib/sanity/validate_modules.py +++ b/test/runner/lib/sanity/validate_modules.py @@ -17,6 +17,7 @@ from lib.util import ( SubprocessError, display, run_command, + read_lines_without_comments, ) from lib.ansible_util import ( @@ -44,9 +45,7 @@ class ValidateModulesTest(SanitySingleVersion): :type targets: SanityTargets :rtype: TestResult """ - with open(VALIDATE_SKIP_PATH, 'r') as skip_fd: - skip_paths = skip_fd.read().splitlines() - + skip_paths = read_lines_without_comments(VALIDATE_SKIP_PATH) skip_paths_set = set(skip_paths) env = ansible_environment(args, color=False) @@ -65,21 +64,23 @@ class ValidateModulesTest(SanitySingleVersion): invalid_ignores = [] - with open(VALIDATE_IGNORE_PATH, 'r') as ignore_fd: - ignore_entries = ignore_fd.read().splitlines() - ignore = collections.defaultdict(dict) - line = 0 + ignore_entries = read_lines_without_comments(VALIDATE_IGNORE_PATH) + ignore = collections.defaultdict(dict) + line = 0 - for ignore_entry in ignore_entries: - line += 1 + for ignore_entry in ignore_entries: + line += 1 - if ' ' not in ignore_entry: - invalid_ignores.append((line, 'Invalid syntax')) - continue + if not ignore_entry: + continue - path, code = ignore_entry.split(' ', 1) + if ' ' not in ignore_entry: + invalid_ignores.append((line, 'Invalid syntax')) + continue - ignore[path][code] = line + path, code = ignore_entry.split(' ', 1) + + ignore[path][code] = line if args.base_branch: cmd.extend([ @@ -139,9 +140,14 @@ class ValidateModulesTest(SanitySingleVersion): confidence=calculate_confidence(VALIDATE_IGNORE_PATH, line, args.metadata) if args.metadata.changes else None, )) + line = 0 + for path in skip_paths: line += 1 + if not path: + continue + if not os.path.exists(path): # Keep files out of the list which no longer exist in the repo. errors.append(SanityMessage( diff --git a/test/runner/lib/target.py b/test/runner/lib/target.py index 260189f206..55c3c35d72 100644 --- a/test/runner/lib/target.py +++ b/test/runner/lib/target.py @@ -12,6 +12,7 @@ import sys from lib.util import ( ApplicationError, + read_lines_without_comments, ) MODULE_EXTENSIONS = '.py', '.ps1' @@ -511,8 +512,8 @@ class IntegrationTarget(CompletionTarget): # static_aliases try: - with open(os.path.join(path, 'aliases'), 'r') as aliases_file: - static_aliases = tuple(aliases_file.read().splitlines()) + aliases_path = os.path.join(path, 'aliases') + static_aliases = tuple(read_lines_without_comments(aliases_path, remove_blank_lines=True)) except IOError as ex: if ex.errno != errno.ENOENT: raise diff --git a/test/runner/lib/util.py b/test/runner/lib/util.py index aeda05a28f..7ccf7169b3 100644 --- a/test/runner/lib/util.py +++ b/test/runner/lib/util.py @@ -42,8 +42,7 @@ def get_docker_completion(): :rtype: dict[str, str] """ if not DOCKER_COMPLETION: - with open('test/runner/completion/docker.txt', 'r') as completion_fd: - images = completion_fd.read().splitlines() + images = read_lines_without_comments('test/runner/completion/docker.txt', remove_blank_lines=True) DOCKER_COMPLETION.update(dict(kvp for kvp in [parse_docker_completion(i) for i in images] if kvp)) @@ -81,6 +80,23 @@ def remove_file(path): os.remove(path) +def read_lines_without_comments(path, remove_blank_lines=False): + """ + :type path: str + :type remove_blank_lines: bool + :rtype: list[str] + """ + with open(path, 'r') as path_fd: + lines = path_fd.read().splitlines() + + lines = [re.sub(r' *#.*$', '', line) for line in lines] + + if remove_blank_lines: + lines = [line for line in lines if line] + + return lines + + def find_executable(executable, cwd=None, path=None, required=True): """ :type executable: str