diff --git a/test/runner/lib/sanity/__init__.py b/test/runner/lib/sanity/__init__.py index 35bd317e4ac..d5e5d22169f 100644 --- a/test/runner/lib/sanity/__init__.py +++ b/test/runner/lib/sanity/__init__.py @@ -7,6 +7,7 @@ import glob import json import os import re +import collections import lib.types as t @@ -35,6 +36,7 @@ from lib.ansible_util import ( from lib.target import ( walk_internal_targets, walk_sanity_targets, + TestTarget, ) from lib.executor import ( @@ -54,6 +56,8 @@ from lib.test import ( TestFailure, TestSkipped, TestMessage, + calculate_best_confidence, + calculate_confidence, ) from lib.data import ( @@ -147,7 +151,7 @@ def collect_code_smell_tests(): """ :rtype: tuple[SanityFunc] """ - skip_file = 'test/sanity/code-smell/skip.txt' + skip_file = os.path.join(ANSIBLE_ROOT, 'test/sanity/code-smell/skip.txt') ansible_only_file = os.path.join(ANSIBLE_ROOT, 'test/sanity/code-smell/ansible-only.txt') skip_tests = read_lines_without_comments(skip_file, remove_blank_lines=True, optional=True) @@ -369,6 +373,10 @@ class SanitySingleVersion(SanityFunc): :rtype: TestResult """ + def load_settings(self, args, code): # type: (SanityConfig, t.Optional[str]) -> SanitySettings + """Load settings for this sanity test.""" + return SanitySettings(args, self.name, code, None) + class SanityMultipleVersion(SanityFunc): """Base class for sanity test plugins which should run on multiple python versions.""" @@ -381,6 +389,223 @@ class SanityMultipleVersion(SanityFunc): :rtype: TestResult """ + def load_settings(self, args, code, python_version): # type: (SanityConfig, t.Optional[str], t.Optional[str]) -> SanitySettings + """Load settings for this sanity test.""" + return SanitySettings(args, self.name, code, python_version) + + +class SanitySettings: + """Settings for sanity tests.""" + def __init__(self, + args, # type: SanityConfig + name, # type: str + code, # type: t.Optional[str] + python_version, # type: t.Optional[str] + ): # type: (...) -> None + self.args = args + self.code = code + self.ignore_settings = SanitySettingsFile(args, name, 'ignore', code, python_version) + self.skip_settings = SanitySettingsFile(args, name, 'skip', code, python_version) + + def filter_skipped_paths(self, paths): # type: (t.List[str]) -> t.List[str] + """Return the given paths, with any skipped paths filtered out.""" + return sorted(set(paths) - set(self.skip_settings.entries.keys())) + + def filter_skipped_targets(self, targets): # type: (t.List[TestTarget]) -> t.List[TestTarget] + """Return the given targets, with any skipped paths filtered out.""" + return sorted(target for target in targets if target.path not in self.skip_settings.entries) + + def process_errors(self, errors, paths): # type: (t.List[SanityMessage], t.List[str]) -> t.List[SanityMessage] + """Return the given errors filtered for ignores and with any settings related errors included.""" + errors = self.ignore_settings.filter_messages(errors) + errors.extend(self.ignore_settings.get_errors(paths)) + errors.extend(self.skip_settings.get_errors([])) + + for ignore_path, ignore_entry in self.ignore_settings.entries.items(): + skip_entry = self.skip_settings.entries.get(ignore_path) + + if not skip_entry: + continue + + skip_line_no = skip_entry[SanitySettingsFile.NO_CODE] + + for ignore_line_no in ignore_entry.values(): + candidates = ((self.ignore_settings.path, ignore_line_no), (self.skip_settings.path, skip_line_no)) + + errors.append(SanityMessage( + code=self.code, + message="Ignoring '%s' is unnecessary due to skip entry on line %d of '%s'" % (ignore_path, skip_line_no, self.skip_settings.relative_path), + path=self.ignore_settings.relative_path, + line=ignore_line_no, + column=1, + confidence=calculate_best_confidence(candidates, self.args.metadata) if self.args.metadata.changes else None, + )) + + errors = sorted(set(errors)) + + return errors + + +class SanitySettingsFile: + """Interface to sanity ignore or sanity skip file settings.""" + NO_CODE = '_' + + def __init__(self, + args, # type: SanityConfig + name, # type: str + mode, # type: str + code, # type: t.Optional[str] + python_version, # type: t.Optional[str] + ): # type: (...) -> None + """ + :param mode: must be either "ignore" or "skip" + :param code: a code for ansible-test to use for internal errors, using a style that matches codes used by the test, or None if codes are not used + """ + if mode == 'ignore': + self.parse_codes = bool(code) + elif mode == 'skip': + self.parse_codes = False + else: + raise Exception('Unsupported mode: %s' % mode) + + if name == 'compile': + filename = 'python%s-%s' % (python_version, mode) + else: + filename = '%s-%s' % (mode, python_version) if python_version else mode + + self.args = args + self.code = code + self.relative_path = 'test/sanity/%s/%s.txt' % (name, filename) + self.path = os.path.join(data_context().content.root, self.relative_path) + self.entries = collections.defaultdict(dict) # type: t.Dict[str, t.Dict[str, int]] + self.parse_errors = [] # type: t.List[t.Tuple[int, int, str]] + self.file_not_found_errors = [] # type: t.List[t.Tuple[int, str]] + self.used_line_numbers = set() # type: t.Set[int] + + lines = read_lines_without_comments(self.path, optional=True) + paths = set(data_context().content.all_files()) + + for line_no, line in enumerate(lines, start=1): + if not line: + continue + + if line.startswith(' '): + self.parse_errors.append((line_no, 1, 'Line cannot start with a space')) + continue + + if line.endswith(' '): + self.parse_errors.append((line_no, len(line), 'Line cannot end with a space')) + continue + + parts = line.split(' ') + path = parts[0] + + if path not in paths: + self.file_not_found_errors.append((line_no, path)) + continue + + if self.parse_codes: + if len(parts) < 2: + self.parse_errors.append((line_no, len(line), 'Code required after path')) + continue + + code = parts[1] + + if not code: + self.parse_errors.append((line_no, len(path) + 1, 'Code after path cannot be empty')) + continue + + if len(parts) > 2: + self.parse_errors.append((line_no, len(path) + len(code) + 2, 'Code cannot contain spaces')) + continue + + existing = self.entries.get(path, {}).get(code) + + if existing: + self.parse_errors.append((line_no, 1, "Duplicate code '%s' for path '%s' first found on line %d" % (code, path, existing))) + continue + else: + if len(parts) > 1: + self.parse_errors.append((line_no, len(path) + 1, 'Path cannot contain spaces')) + continue + + code = self.NO_CODE + existing = self.entries.get(path) + + if existing: + self.parse_errors.append((line_no, 1, "Duplicate path '%s' first found on line %d" % (path, existing[code]))) + continue + + self.entries[path][code] = line_no + + def filter_messages(self, messages): # type: (t.List[SanityMessage]) -> t.List[SanityMessage] + """Return a filtered list of the given messages using the entries that have been loaded.""" + filtered = [] + + for message in messages: + path_entry = self.entries.get(message.path) + + if path_entry: + code = message.code if self.code else self.NO_CODE + line_no = path_entry.get(code) + + if line_no: + self.used_line_numbers.add(line_no) + continue + + filtered.append(message) + + return filtered + + def get_errors(self, paths): # type: (t.List[str]) -> t.List[SanityMessage] + """Return error messages related to issues with the file.""" + messages = [] + + # parse errors + + messages.extend(SanityMessage( + code=self.code, + message=message, + path=self.relative_path, + line=line, + column=column, + confidence=calculate_confidence(self.path, line, self.args.metadata) if self.args.metadata.changes else None, + ) for line, column, message in self.parse_errors) + + # file not found errors + + messages.extend(SanityMessage( + code=self.code, + message="File '%s' does not exist" % path, + path=self.relative_path, + line=line, + column=1, + confidence=calculate_best_confidence(((self.path, line), (path, 0)), self.args.metadata) if self.args.metadata.changes else None, + ) for line, path in self.file_not_found_errors) + + # unused errors + + unused = [] # type: t.List[t.Tuple[int, str, str]] + + for path in paths: + path_entry = self.entries.get(path) + + if not path_entry: + continue + + unused.extend((line_no, path, code) for code, line_no in path_entry.items() if line_no not in self.used_line_numbers) + + messages.extend(SanityMessage( + code=self.code, + message="Ignoring '%s' on '%s' is unnecessary" % (code, path) if self.code else "Ignoring '%s' is unnecessary" % path, + path=self.relative_path, + line=line, + column=1, + confidence=calculate_best_confidence(((self.path, line), (path, 0)), self.args.metadata) if self.args.metadata.changes else None, + ) for line, path, code in unused) + + return messages + SANITY_TESTS = ( ) diff --git a/test/runner/lib/sanity/ansible_doc.py b/test/runner/lib/sanity/ansible_doc.py index f2055d78fac..3ae4476b80b 100644 --- a/test/runner/lib/sanity/ansible_doc.py +++ b/test/runner/lib/sanity/ansible_doc.py @@ -7,7 +7,7 @@ import os import re from lib.sanity import ( - SanityMultipleVersion, + SanitySingleVersion, SanityFailure, SanitySuccess, SanitySkipped, @@ -17,7 +17,6 @@ from lib.sanity import ( from lib.util import ( SubprocessError, display, - read_lines_without_comments, ) from lib.util_common import ( @@ -41,19 +40,20 @@ from lib.coverage_util import ( ) -class AnsibleDocTest(SanityMultipleVersion): +class AnsibleDocTest(SanitySingleVersion): """Sanity test for ansible-doc.""" - def test(self, args, targets, python_version): + def test(self, args, targets): """ :type args: SanityConfig :type targets: SanityTargets - :type python_version: str :rtype: TestResult """ - skip_file = 'test/sanity/ansible-doc/skip.txt' - skip_paths = set(read_lines_without_comments(skip_file, remove_blank_lines=True, optional=True)) + settings = self.load_settings(args, None) - targets_include = [target for target in targets.include if target.path not in skip_paths and os.path.splitext(target.path)[1] == '.py'] + targets_include = [target for target in targets.include if os.path.splitext(target.path)[1] == '.py'] + targets_include = settings.filter_skipped_targets(targets_include) + + paths = [target.path for target in targets_include] # This should use documentable plugins from constants instead plugin_type_blacklist = set([ @@ -87,7 +87,7 @@ class AnsibleDocTest(SanityMultipleVersion): target_paths[plugin_type][data_context().content.prefix + plugin_name] = plugin_path if not doc_targets: - return SanitySkipped(self.name, python_version=python_version) + return SanitySkipped(self.name) target_paths['module'] = dict((data_context().content.prefix + t.module, t.path) for t in targets.targets if t.module) @@ -99,7 +99,7 @@ class AnsibleDocTest(SanityMultipleVersion): try: with coverage_context(args): - stdout, stderr = intercept_command(args, cmd, target_name='ansible-doc', env=env, capture=True, python_version=python_version) + stdout, stderr = intercept_command(args, cmd, target_name='ansible-doc', env=env, capture=True) status = 0 except SubprocessError as ex: @@ -117,19 +117,21 @@ class AnsibleDocTest(SanityMultipleVersion): if status: summary = u'%s' % SubprocessError(cmd=cmd, status=status, stderr=stderr) - return SanityFailure(self.name, summary=summary, python_version=python_version) + return SanityFailure(self.name, summary=summary) if stdout: display.info(stdout.strip(), verbosity=3) if stderr: summary = u'Output on stderr from ansible-doc is considered an error.\n\n%s' % SubprocessError(cmd, stderr=stderr) - return SanityFailure(self.name, summary=summary, python_version=python_version) + return SanityFailure(self.name, summary=summary) + + error_messages = settings.process_errors(error_messages, paths) if error_messages: - return SanityFailure(self.name, messages=error_messages, python_version=python_version) + return SanityFailure(self.name, messages=error_messages) - return SanitySuccess(self.name, python_version=python_version) + return SanitySuccess(self.name) @staticmethod def parse_error(error, target_paths): diff --git a/test/runner/lib/sanity/compile.py b/test/runner/lib/sanity/compile.py index 62cb9537171..3c24229e883 100644 --- a/test/runner/lib/sanity/compile.py +++ b/test/runner/lib/sanity/compile.py @@ -16,7 +16,6 @@ from lib.util import ( SubprocessError, display, find_python, - read_lines_without_comments, parse_to_list_of_dict, ANSIBLE_ROOT, ) @@ -29,10 +28,6 @@ from lib.config import ( SanityConfig, ) -from lib.test import ( - calculate_best_confidence, -) - class CompileTest(SanityMultipleVersion): """Sanity test for proper python syntax.""" @@ -43,14 +38,10 @@ class CompileTest(SanityMultipleVersion): :type python_version: str :rtype: TestResult """ - skip_file = 'test/sanity/compile/python%s-skip.txt' % python_version + settings = self.load_settings(args, None, python_version) - if os.path.exists(skip_file): - skip_paths = read_lines_without_comments(skip_file) - else: - skip_paths = [] - - paths = sorted(i.path for i in targets.include if (os.path.splitext(i.path)[1] == '.py' or i.path.startswith('bin/')) and i.path not in skip_paths) + paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] == '.py' or i.path.startswith('bin/')) + paths = settings.filter_skipped_paths(paths) if not paths: return SanitySkipped(self.name, python_version=python_version) @@ -86,24 +77,7 @@ class CompileTest(SanityMultipleVersion): column=int(r['column']), ) for r in results] - 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. - results.append(SanityMessage( - code='A101', - message='Remove "%s" since it does not exist' % path, - path=skip_file, - line=line, - column=1, - confidence=calculate_best_confidence(((skip_file, line), (path, 0)), args.metadata) if args.metadata.changes else None, - )) + results = settings.process_errors(results, paths) if results: return SanityFailure(self.name, messages=results, python_version=python_version) diff --git a/test/runner/lib/sanity/import.py b/test/runner/lib/sanity/import.py index 62cd46b637a..29270883faf 100644 --- a/test/runner/lib/sanity/import.py +++ b/test/runner/lib/sanity/import.py @@ -17,7 +17,6 @@ from lib.util import ( remove_tree, display, find_python, - read_lines_without_comments, parse_to_list_of_dict, make_dirs, is_subdir, @@ -59,19 +58,17 @@ class ImportTest(SanityMultipleVersion): :type python_version: str :rtype: TestResult """ - skip_file = 'test/sanity/import/skip.txt' - skip_paths = read_lines_without_comments(skip_file, remove_blank_lines=True, optional=True) - - skip_paths_set = set(skip_paths) + settings = self.load_settings(args, None, python_version) paths = sorted( i.path for i in targets.include if os.path.splitext(i.path)[1] == '.py' and - (is_subdir(i.path, data_context().content.module_path) or is_subdir(i.path, data_context().content.module_utils_path)) and - i.path not in skip_paths_set + (is_subdir(i.path, data_context().content.module_path) or is_subdir(i.path, data_context().content.module_utils_path)) ) + paths = settings.filter_skipped_paths(paths) + if not paths: return SanitySkipped(self.name, python_version=python_version) @@ -156,7 +153,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_set] + results = settings.process_errors(results, paths) 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 088531da744..8b09c2de965 100644 --- a/test/runner/lib/sanity/pep8.py +++ b/test/runner/lib/sanity/pep8.py @@ -2,8 +2,8 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import json import os -import re from lib.sanity import ( SanitySingleVersion, @@ -14,7 +14,6 @@ from lib.sanity import ( from lib.util import ( SubprocessError, - display, read_lines_without_comments, parse_to_list_of_dict, ANSIBLE_ROOT, @@ -28,13 +27,6 @@ from lib.config import ( SanityConfig, ) -from lib.test import ( - calculate_best_confidence, -) - -PEP8_SKIP_PATH = 'test/sanity/pep8/skip.txt' -PEP8_LEGACY_PATH = 'test/sanity/pep8/legacy-files.txt' - class Pep8Test(SanitySingleVersion): """Sanity test for PEP 8 style guidelines using pycodestyle.""" @@ -44,19 +36,13 @@ class Pep8Test(SanitySingleVersion): :type targets: SanityTargets :rtype: TestResult """ - skip_paths = read_lines_without_comments(PEP8_SKIP_PATH, optional=True) - legacy_paths = read_lines_without_comments(PEP8_LEGACY_PATH, optional=True) - - legacy_ignore_file = os.path.join(ANSIBLE_ROOT, 'test/sanity/pep8/legacy-ignore.txt') - legacy_ignore = set(read_lines_without_comments(legacy_ignore_file, remove_blank_lines=True)) - current_ignore_file = os.path.join(ANSIBLE_ROOT, '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) + settings = self.load_settings(args, 'A100') - paths = sorted(i.path for i in targets.include if (os.path.splitext(i.path)[1] == '.py' or i.path.startswith('bin/')) and i.path not in skip_paths_set) + paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] == '.py' or i.path.startswith('bin/')) + paths = settings.filter_skipped_paths(paths) cmd = [ args.python_executable, @@ -99,90 +85,7 @@ class Pep8Test(SanitySingleVersion): code=r['code'], ) for r in results] - failed_result_paths = set([result.path for result in results]) - used_paths = set(paths) - - errors = [] - summary = {} - - line = 0 - - 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( - code='A101', - message='Remove "%s" since it does not exist' % path, - path=PEP8_LEGACY_PATH, - line=line, - column=1, - confidence=calculate_best_confidence(((PEP8_LEGACY_PATH, line), (path, 0)), args.metadata) if args.metadata.changes else None, - )) - - if path in used_paths and path not in failed_result_paths: - # Keep files out of the list which no longer require the relaxed rule set. - errors.append(SanityMessage( - code='A201', - message='Remove "%s" since it passes the current rule set' % path, - path=PEP8_LEGACY_PATH, - line=line, - column=1, - confidence=calculate_best_confidence(((PEP8_LEGACY_PATH, line), (path, 0)), 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( - code='A101', - message='Remove "%s" since it does not exist' % path, - path=PEP8_SKIP_PATH, - line=line, - column=1, - confidence=calculate_best_confidence(((PEP8_SKIP_PATH, line), (path, 0)), args.metadata) if args.metadata.changes else None, - )) - - for result in results: - if result.path in legacy_paths_set and result.code in legacy_ignore: - # Files on the legacy list are permitted to have errors on the legacy ignore list. - # However, we want to report on their existence to track progress towards eliminating these exceptions. - display.info('PEP 8: %s (legacy)' % result, verbosity=3) - - key = '%s %s' % (result.code, re.sub('[0-9]+', 'NNN', result.message)) - - if key not in summary: - summary[key] = 0 - - summary[key] += 1 - else: - # Files not on the legacy list and errors not on the legacy ignore list are PEP 8 policy errors. - errors.append(result) - - if summary: - lines = [] - count = 0 - - for key in sorted(summary): - count += summary[key] - lines.append('PEP 8: %5d %s' % (summary[key], key)) - - display.info('PEP 8: There were %d different legacy issues found (%d total):' % (len(summary), count), verbosity=1) - display.info('PEP 8: Count Code Message', verbosity=1) - - for line in lines: - display.info(line, verbosity=1) + errors = settings.process_errors(results, paths) if errors: return SanityFailure(self.name, messages=errors) diff --git a/test/runner/lib/sanity/pslint.py b/test/runner/lib/sanity/pslint.py index 18bd44327a6..d2585cd5108 100644 --- a/test/runner/lib/sanity/pslint.py +++ b/test/runner/lib/sanity/pslint.py @@ -2,13 +2,10 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -import collections import json import os import re -import lib.types as t - from lib.sanity import ( SanitySingleVersion, SanityMessage, @@ -20,7 +17,6 @@ from lib.sanity import ( from lib.util import ( SubprocessError, find_executable, - read_lines_without_comments, ) from lib.util_common import ( @@ -32,18 +28,10 @@ from lib.config import ( SanityConfig, ) -from lib.test import ( - calculate_confidence, - calculate_best_confidence, -) - from lib.data import ( data_context, ) -PSLINT_SKIP_PATH = 'test/sanity/pslint/skip.txt' -PSLINT_IGNORE_PATH = 'test/sanity/pslint/ignore.txt' - class PslintTest(SanitySingleVersion): """Sanity test using PSScriptAnalyzer.""" @@ -53,33 +41,10 @@ class PslintTest(SanitySingleVersion): :type targets: SanityTargets :rtype: TestResult """ - skip_paths = read_lines_without_comments(PSLINT_SKIP_PATH, optional=True) + settings = self.load_settings(args, 'AnsibleTest') - invalid_ignores = [] - - ignore_entries = read_lines_without_comments(PSLINT_IGNORE_PATH, optional=True) - ignore = collections.defaultdict(dict) # type: t.Dict[str, t.Dict[str, int]] - line = 0 - - for ignore_entry in ignore_entries: - line += 1 - - 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 - - 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) + paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] in ('.ps1', '.psm1', '.psd1')) + paths = settings.filter_skipped_paths(paths) if not paths: return SanitySkipped(self.name) @@ -87,11 +52,12 @@ class PslintTest(SanitySingleVersion): if not find_executable('pwsh', required='warning'): return SanitySkipped(self.name) - # Make sure requirements are installed before running sanity checks - cmds = [ - [os.path.join(ANSIBLE_ROOT, 'test/runner/requirements/sanity.ps1')], - [os.path.join(ANSIBLE_ROOT, 'test/sanity/pslint/pslint.ps1')] + paths - ] + cmds = [] + + if args.requirements: + cmds.append([os.path.join(ANSIBLE_ROOT, 'test/runner/requirements/sanity.ps1')]) + + cmds.append([os.path.join(ANSIBLE_ROOT, 'test/sanity/pslint/pslint.ps1')] + paths) stdout = '' @@ -135,63 +101,7 @@ class PslintTest(SanitySingleVersion): level=severity[m['Severity']], ) for m in messages] - line = 0 - - filtered = [] - - for error in errors: - if error.code in ignore[error.path]: - ignore[error.path][error.code] = 0 # error ignored, clear line number of ignore entry to track usage - else: - filtered.append(error) # error not ignored - - errors = filtered - - for invalid_ignore in invalid_ignores: - errors.append(SanityMessage( - code='A201', - message=invalid_ignore[1], - path=PSLINT_IGNORE_PATH, - line=invalid_ignore[0], - column=1, - confidence=calculate_confidence(PSLINT_IGNORE_PATH, line, args.metadata) if args.metadata.changes else None, - )) - - 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( - code='A101', - message='Remove "%s" since it does not exist' % path, - path=PSLINT_SKIP_PATH, - line=line, - column=1, - confidence=calculate_best_confidence(((PSLINT_SKIP_PATH, line), (path, 0)), args.metadata) if args.metadata.changes else None, - )) - - for path in paths: - if path not in ignore: - continue - - for code in ignore[path]: - line = ignore[path][code] - - if not line: - continue - - errors.append(SanityMessage( - code='A102', - message='Remove since "%s" passes "%s" test' % (path, code), - path=PSLINT_IGNORE_PATH, - line=line, - column=1, - confidence=calculate_best_confidence(((PSLINT_IGNORE_PATH, line), (path, 0)), args.metadata) if args.metadata.changes else None, - )) + errors = settings.process_errors(errors, paths) if errors: return SanityFailure(self.name, messages=errors) diff --git a/test/runner/lib/sanity/pylint.py b/test/runner/lib/sanity/pylint.py index 4ec039541d6..bab2301caf5 100644 --- a/test/runner/lib/sanity/pylint.py +++ b/test/runner/lib/sanity/pylint.py @@ -2,7 +2,6 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -import collections import itertools import json import os @@ -21,7 +20,6 @@ from lib.sanity import ( from lib.util import ( SubprocessError, display, - read_lines_without_comments, ConfigParser, ANSIBLE_ROOT, is_subdir, @@ -31,10 +29,6 @@ from lib.util_common import ( run_command, ) -from lib.executor import ( - SUPPORTED_PYTHON_VERSIONS, -) - from lib.ansible_util import ( ansible_environment, ) @@ -43,19 +37,11 @@ from lib.config import ( SanityConfig, ) -from lib.test import ( - calculate_confidence, - calculate_best_confidence, -) - from lib.data import ( data_context, ) -PYLINT_SKIP_PATH = 'test/sanity/pylint/skip.txt' -PYLINT_IGNORE_PATH = 'test/sanity/pylint/ignore.txt' - UNSUPPORTED_PYTHON_VERSIONS = ( '2.6', '2.7', @@ -78,48 +64,10 @@ class PylintTest(SanitySingleVersion): plugin_names = sorted(p[0] for p in [ os.path.splitext(p) for p in os.listdir(plugin_dir)] if p[1] == '.py' and p[0] != '__init__') - skip_paths = read_lines_without_comments(PYLINT_SKIP_PATH, optional=True) + settings = self.load_settings(args, 'ansible-test') - 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 - - ignore_entries = read_lines_without_comments(PYLINT_IGNORE_PATH, optional=True) - ignore = collections.defaultdict(dict) # type: t.Dict[str, t.Dict[str, int]] - line = 0 - - for ignore_entry in ignore_entries: - line += 1 - - 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 - - if version not in (args.python_version, args.python_version.split('.')[0]): - continue # ignore version specific entries for other versions - - ignore[path][code] = line - - skip_paths_set = set(skip_paths) - - paths = sorted(i.path for i in targets.include if (os.path.splitext(i.path)[1] == '.py' or is_subdir(i.path, 'bin/')) and i.path not in skip_paths_set) + paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] == '.py' or is_subdir(i.path, 'bin/')) + paths = settings.filter_skipped_paths(paths) module_paths = [os.path.relpath(p, data_context().content.module_path).split(os.path.sep) for p in paths if is_subdir(p, data_context().content.module_path)] @@ -215,63 +163,7 @@ class PylintTest(SanitySingleVersion): if args.explain: return SanitySuccess(self.name) - line = 0 - - filtered = [] - - for error in errors: - if error.code in ignore[error.path]: - ignore[error.path][error.code] = 0 # error ignored, clear line number of ignore entry to track usage - else: - filtered.append(error) # error not ignored - - errors = filtered - - for invalid_ignore in invalid_ignores: - errors.append(SanityMessage( - code='A201', - message=invalid_ignore[1], - path=PYLINT_IGNORE_PATH, - line=invalid_ignore[0], - column=1, - confidence=calculate_confidence(PYLINT_IGNORE_PATH, line, args.metadata) if args.metadata.changes else None, - )) - - 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( - code='A101', - message='Remove "%s" since it does not exist' % path, - path=PYLINT_SKIP_PATH, - line=line, - column=1, - confidence=calculate_best_confidence(((PYLINT_SKIP_PATH, line), (path, 0)), args.metadata) if args.metadata.changes else None, - )) - - for path in paths: - if path not in ignore: - continue - - for code in ignore[path]: - line = ignore[path][code] - - if not line: - continue - - errors.append(SanityMessage( - code='A102', - message='Remove since "%s" passes "%s" pylint test' % (path, code), - path=PYLINT_IGNORE_PATH, - line=line, - column=1, - confidence=calculate_best_confidence(((PYLINT_IGNORE_PATH, line), (path, 0)), args.metadata) if args.metadata.changes else None, - )) + errors = settings.process_errors(errors, paths) if errors: return SanityFailure(self.name, messages=errors) diff --git a/test/runner/lib/sanity/rstcheck.py b/test/runner/lib/sanity/rstcheck.py index 8fe3a984cab..cffa3c49104 100644 --- a/test/runner/lib/sanity/rstcheck.py +++ b/test/runner/lib/sanity/rstcheck.py @@ -48,7 +48,10 @@ class RstcheckTest(SanitySingleVersion): ignore_file = os.path.join(ANSIBLE_ROOT, 'test/sanity/rstcheck/ignore-substitutions.txt') ignore_substitutions = sorted(set(read_lines_without_comments(ignore_file, remove_blank_lines=True))) + settings = self.load_settings(args, None) + paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] in ('.rst',)) + paths = settings.filter_skipped_paths(paths) if not paths: return SanitySkipped(self.name) @@ -86,6 +89,8 @@ class RstcheckTest(SanitySingleVersion): level=r['level'], ) for r in results] + settings.process_errors(results, paths) + if results: return SanityFailure(self.name, messages=results) diff --git a/test/runner/lib/sanity/shellcheck.py b/test/runner/lib/sanity/shellcheck.py index 59be717c138..48c416a1f2f 100644 --- a/test/runner/lib/sanity/shellcheck.py +++ b/test/runner/lib/sanity/shellcheck.py @@ -20,6 +20,7 @@ from lib.sanity import ( from lib.util import ( SubprocessError, read_lines_without_comments, + ANSIBLE_ROOT, ) from lib.util_common import ( @@ -39,13 +40,13 @@ class ShellcheckTest(SanitySingleVersion): :type targets: SanityTargets :rtype: TestResult """ - skip_file = 'test/sanity/shellcheck/skip.txt' - skip_paths = set(read_lines_without_comments(skip_file, remove_blank_lines=True, optional=True)) - - exclude_file = 'test/sanity/shellcheck/exclude.txt' + exclude_file = os.path.join(ANSIBLE_ROOT, 'test/sanity/shellcheck/exclude.txt') exclude = set(read_lines_without_comments(exclude_file, remove_blank_lines=True, optional=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) + settings = self.load_settings(args, 'AT1000') + + paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] == '.sh') + paths = settings.filter_skipped_paths(paths) if not paths: return SanitySkipped(self.name) @@ -86,6 +87,8 @@ class ShellcheckTest(SanitySingleVersion): code=entry.attrib['source'].replace('ShellCheck.', ''), )) + results = settings.process_errors(results, paths) + if results: return SanityFailure(self.name, messages=results) diff --git a/test/runner/lib/sanity/validate_modules.py b/test/runner/lib/sanity/validate_modules.py index d64333e7e03..e533d2423c4 100644 --- a/test/runner/lib/sanity/validate_modules.py +++ b/test/runner/lib/sanity/validate_modules.py @@ -2,12 +2,9 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -import collections import json import os -import lib.types as t - from lib.sanity import ( SanitySingleVersion, SanityMessage, @@ -19,7 +16,6 @@ from lib.sanity import ( from lib.util import ( SubprocessError, display, - read_lines_without_comments, ANSIBLE_ROOT, ) @@ -35,18 +31,10 @@ from lib.config import ( SanityConfig, ) -from lib.test import ( - calculate_confidence, - calculate_best_confidence, -) - from lib.data import ( data_context, ) -VALIDATE_SKIP_PATH = 'test/sanity/validate-modules/skip.txt' -VALIDATE_IGNORE_PATH = 'test/sanity/validate-modules/ignore.txt' - UNSUPPORTED_PYTHON_VERSIONS = ( '2.6', '2.7', @@ -72,12 +60,12 @@ class ValidateModulesTest(SanitySingleVersion): 'E502', # only ansible content requires __init__.py for module subdirectories )) - skip_paths = read_lines_without_comments(VALIDATE_SKIP_PATH, optional=True) - skip_paths_set = set(skip_paths) - env = ansible_environment(args, color=False) - paths = sorted([i.path for i in targets.include if i.module and i.path not in skip_paths_set]) + settings = self.load_settings(args, 'A100') + + paths = sorted(i.path for i in targets.include if i.module) + paths = settings.filter_skipped_paths(paths) if not paths: return SanitySkipped(self.name) @@ -89,26 +77,6 @@ class ValidateModulesTest(SanitySingleVersion): '--arg-spec', ] + paths - invalid_ignores = [] - - ignore_entries = read_lines_without_comments(VALIDATE_IGNORE_PATH, optional=True) - ignore = collections.defaultdict(dict) # type: t.Dict[str, t.Dict[str, int]] - line = 0 - - for ignore_entry in ignore_entries: - line += 1 - - if not ignore_entry: - continue - - if ' ' not in ignore_entry: - invalid_ignores.append((line, 'Invalid syntax')) - continue - - path, code = ignore_entry.split(' ', 1) - - ignore[path][code] = line - if args.base_branch: cmd.extend([ '--base-branch', args.base_branch, @@ -147,80 +115,8 @@ class ValidateModulesTest(SanitySingleVersion): message=item['msg'], )) - filtered = [] - errors = [error for error in errors if error.code not in ignore_codes] - - for error in errors: - if error.code in ignore[error.path]: - ignore[error.path][error.code] = 0 # error ignored, clear line number of ignore entry to track usage - else: - filtered.append(error) # error not ignored - - errors = filtered - - for invalid_ignore in invalid_ignores: - errors.append(SanityMessage( - code='A201', - message=invalid_ignore[1], - path=VALIDATE_IGNORE_PATH, - line=invalid_ignore[0], - column=1, - 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( - code='A101', - message='Remove "%s" since it does not exist' % path, - path=VALIDATE_SKIP_PATH, - line=line, - column=1, - confidence=calculate_best_confidence(((VALIDATE_SKIP_PATH, line), (path, 0)), args.metadata) if args.metadata.changes else None, - )) - - for path in sorted(ignore.keys()): - if os.path.exists(path): - continue - - for line in sorted(ignore[path].values()): - # Keep files out of the list which no longer exist in the repo. - errors.append(SanityMessage( - code='A101', - message='Remove "%s" since it does not exist' % path, - path=VALIDATE_IGNORE_PATH, - line=line, - column=1, - confidence=calculate_best_confidence(((VALIDATE_IGNORE_PATH, line), (path, 0)), args.metadata) if args.metadata.changes else None, - )) - - for path in paths: - if path not in ignore: - continue - - for code in ignore[path]: - line = ignore[path][code] - - if not line: - continue - - errors.append(SanityMessage( - code='A102', - message='Remove since "%s" passes "%s" test' % (path, code), - path=VALIDATE_IGNORE_PATH, - line=line, - column=1, - confidence=calculate_best_confidence(((VALIDATE_IGNORE_PATH, line), (path, 0)), args.metadata) if args.metadata.changes else None, - )) + errors = settings.process_errors(errors, paths) if errors: return SanityFailure(self.name, messages=errors) diff --git a/test/runner/lib/sanity/yamllint.py b/test/runner/lib/sanity/yamllint.py index 880986ad8d5..ab825476d56 100644 --- a/test/runner/lib/sanity/yamllint.py +++ b/test/runner/lib/sanity/yamllint.py @@ -41,28 +41,26 @@ class YamllintTest(SanitySingleVersion): :type targets: SanityTargets :rtype: TestResult """ - paths = [ - [i.path for i in targets.include if os.path.splitext(i.path)[1] in ('.yml', '.yaml')], - ] + settings = self.load_settings(args, 'ansible-test') + + paths = [i.path for i in targets.include if os.path.splitext(i.path)[1] in ('.yml', '.yaml')] for plugin_type, plugin_path in sorted(data_context().content.plugin_paths.items()): if plugin_type == 'module_utils': continue - paths.append([target.path for target in targets.include if + paths.extend([target.path for target in targets.include if os.path.splitext(target.path)[1] == '.py' and os.path.basename(target.path) != '__init__.py' and is_subdir(target.path, plugin_path)]) - paths = [sorted(p) for p in paths if p] + paths = settings.filter_skipped_paths(paths) if not paths: return SanitySkipped(self.name) - results = [] - - for test_paths in paths: - results += self.test_paths(args, test_paths) + results = self.test_paths(args, paths) + results = settings.process_errors(results, paths) if results: return SanityFailure(self.name, messages=results) diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/test/sanity/pep8/legacy-ignore.txt b/test/sanity/pep8/legacy-ignore.txt deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/test/sanity/pslint/ignore.txt b/test/sanity/pslint/ignore.txt index 5d1565e433f..902bea4a53f 100644 --- a/test/sanity/pslint/ignore.txt +++ b/test/sanity/pslint/ignore.txt @@ -101,7 +101,6 @@ lib/ansible/modules/windows/win_dsc.ps1 PSAvoidUsingEmptyCatchBlock # Keep lib/ansible/modules/windows/win_find.ps1 PSAvoidUsingEmptyCatchBlock # Keep lib/ansible/modules/windows/win_region.ps1 PSAvoidUsingEmptyCatchBlock # Keep lib/ansible/modules/windows/win_uri.ps1 PSAvoidUsingEmptyCatchBlock # Keep -lib/ansible/modules/windows/win_find.ps1 PSAvoidUsingEmptyCatchBlock # Keep for now lib/ansible/modules/windows/win_domain_membership.ps1 PSAvoidGlobalVars # New PR lib/ansible/modules/windows/win_domain_controller.ps1 PSAvoidGlobalVars # New PR lib/ansible/modules/windows/win_pagefile.ps1 PSUseDeclaredVarsMoreThanAssignments # New PR - bug test_path should be testPath