From ac1698099d579bf778d5634c7d8c68db6873a1a3 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Tue, 27 Feb 2018 15:05:39 -0800 Subject: [PATCH] Overhaul additional sanity tests. (#36803) * Remove unnecessary sys.exit calls. * Add files filtering for code-smell tests. * Enhance test-constraints code-smell test. * Simplify compile sanity test. * Pass paths to importer on stdin. * Pass paths to yamllinter on stdin. * Add work-around for unicode path filtering. * Enhance configure-remoting-ps1 code-smell test. * Enhance integration-aliases code-smell test. * Enhance azure-requirements code-smell test. * Enhance no-illegal-filenames code-smell test. --- test/runner/lib/sanity/__init__.py | 18 +++- test/runner/lib/sanity/compile.py | 9 +- test/runner/lib/sanity/import.py | 10 +- test/runner/lib/sanity/yamllint.py | 9 +- .../sanity/code-smell/azure-requirements.json | 4 + test/sanity/code-smell/azure-requirements.py | 20 ++-- .../code-smell/configure-remoting-ps1.json | 4 + .../code-smell/configure-remoting-ps1.py | 31 +++++++ .../code-smell/configure-remoting-ps1.sh | 9 -- test/sanity/code-smell/empty-init.json | 4 +- test/sanity/code-smell/empty-init.py | 3 - .../code-smell/integration-aliases.json | 4 + test/sanity/code-smell/integration-aliases.py | 44 +-------- test/sanity/code-smell/no-assert.py | 6 -- .../code-smell/no-illegal-filenames.json | 4 + .../sanity/code-smell/no-illegal-filenames.py | 26 +++--- test/sanity/code-smell/no-tests-as-filters.py | 6 -- test/sanity/code-smell/test-constraints.json | 9 ++ test/sanity/code-smell/test-constraints.py | 27 ++++++ test/sanity/code-smell/test-constraints.sh | 17 ---- test/sanity/compile/compile.py | 93 +------------------ test/sanity/import/importer.py | 2 +- test/sanity/yamllint/yamllinter.py | 2 +- 23 files changed, 153 insertions(+), 208 deletions(-) create mode 100644 test/sanity/code-smell/azure-requirements.json create mode 100644 test/sanity/code-smell/configure-remoting-ps1.json create mode 100755 test/sanity/code-smell/configure-remoting-ps1.py delete mode 100755 test/sanity/code-smell/configure-remoting-ps1.sh create mode 100644 test/sanity/code-smell/integration-aliases.json create mode 100644 test/sanity/code-smell/no-illegal-filenames.json create mode 100644 test/sanity/code-smell/test-constraints.json create mode 100755 test/sanity/code-smell/test-constraints.py delete mode 100755 test/sanity/code-smell/test-constraints.sh diff --git a/test/runner/lib/sanity/__init__.py b/test/runner/lib/sanity/__init__.py index 052ff900e6b..e9fe96d536c 100644 --- a/test/runner/lib/sanity/__init__.py +++ b/test/runner/lib/sanity/__init__.py @@ -6,6 +6,7 @@ import glob import json import os import re +import sys from lib.util import ( ApplicationError, @@ -233,6 +234,8 @@ class SanityCodeSmellTest(SanityTest): output = config.get('output') extensions = config.get('extensions') prefixes = config.get('prefixes') + files = config.get('files') + always = config.get('always') if output == 'path-line-column-message': pattern = '^(?P[^:]*):(?P[0-9]+):(?P[0-9]+): (?P.*)$' @@ -243,18 +246,29 @@ class SanityCodeSmellTest(SanityTest): paths = sorted(i.path for i in targets.include) + if always: + paths = [] + + # short-term work-around for paths being str instead of unicode on python 2.x + if sys.version_info[0] == 2: + paths = [p.decode('utf-8') for p in paths] + if extensions: paths = [p for p in paths if os.path.splitext(p)[1] in extensions or (p.startswith('bin/') and '.py' in extensions)] if prefixes: paths = [p for p in paths if any(p.startswith(pre) for pre in prefixes)] - if not paths: + if files: + paths = [p for p in paths if os.path.basename(p) in files] + + if not paths and not always: return SanitySkipped(self.name) data = '\n'.join(paths) - display.info(data, verbosity=4) + if data: + display.info(data, verbosity=4) try: stdout, stderr = run_command(args, cmd, data=data, env=env, capture=True) status = 0 diff --git a/test/runner/lib/sanity/compile.py b/test/runner/lib/sanity/compile.py index 9381ac70e03..e754448057f 100644 --- a/test/runner/lib/sanity/compile.py +++ b/test/runner/lib/sanity/compile.py @@ -15,6 +15,7 @@ from lib.sanity import ( from lib.util import ( SubprocessError, run_command, + display, ) from lib.config import ( @@ -49,10 +50,14 @@ class CompileTest(SanityMultipleVersion): if not paths: return SanitySkipped(self.name, python_version=python_version) - cmd = ['python%s' % python_version, 'test/sanity/compile/compile.py'] + paths + cmd = ['python%s' % python_version, 'test/sanity/compile/compile.py'] + + data = '\n'.join(paths) + + display.info(data, verbosity=4) try: - stdout, stderr = run_command(args, cmd, capture=True) + stdout, stderr = run_command(args, cmd, data=data, capture=True) status = 0 except SubprocessError as ex: stdout = ex.stdout diff --git a/test/runner/lib/sanity/import.py b/test/runner/lib/sanity/import.py index d11ce76c41f..b6c7ede108b 100644 --- a/test/runner/lib/sanity/import.py +++ b/test/runner/lib/sanity/import.py @@ -17,6 +17,7 @@ from lib.util import ( run_command, intercept_command, remove_tree, + display, ) from lib.ansible_util import ( @@ -88,12 +89,17 @@ class ImportTest(SanityMultipleVersion): run_command(args, ['pip', 'uninstall', '--disable-pip-version-check', '-y', 'setuptools'], env=env) run_command(args, ['pip', 'uninstall', '--disable-pip-version-check', '-y', 'pip'], env=env) - cmd = ['importer.py'] + paths + cmd = ['importer.py'] + + data = '\n'.join(paths) + + display.info(data, verbosity=4) results = [] try: - stdout, stderr = intercept_command(args, cmd, target_name=self.name, env=env, capture=True, python_version=python_version, path=env['PATH']) + stdout, stderr = intercept_command(args, cmd, data=data, target_name=self.name, env=env, capture=True, python_version=python_version, + path=env['PATH']) if stdout or stderr: raise SubprocessError(cmd, stdout=stdout, stderr=stderr) diff --git a/test/runner/lib/sanity/yamllint.py b/test/runner/lib/sanity/yamllint.py index c9a19edba8a..9aed5b94ad4 100644 --- a/test/runner/lib/sanity/yamllint.py +++ b/test/runner/lib/sanity/yamllint.py @@ -15,6 +15,7 @@ from lib.sanity import ( from lib.util import ( SubprocessError, run_command, + display, ) from lib.config import ( @@ -66,10 +67,14 @@ class YamllintTest(SanitySingleVersion): cmd = [ 'python%s' % args.python_version, 'test/sanity/yamllint/yamllinter.py', - ] + paths + ] + + data = '\n'.join(paths) + + display.info(data, verbosity=4) try: - stdout, stderr = run_command(args, cmd, capture=True) + stdout, stderr = run_command(args, cmd, data=data, capture=True) status = 0 except SubprocessError as ex: stdout = ex.stdout diff --git a/test/sanity/code-smell/azure-requirements.json b/test/sanity/code-smell/azure-requirements.json new file mode 100644 index 00000000000..39ac4bd57f0 --- /dev/null +++ b/test/sanity/code-smell/azure-requirements.json @@ -0,0 +1,4 @@ +{ + "always": true, + "output": "path-message" +} diff --git a/test/sanity/code-smell/azure-requirements.py b/test/sanity/code-smell/azure-requirements.py index aeaf480b88c..95ab47d0f48 100755 --- a/test/sanity/code-smell/azure-requirements.py +++ b/test/sanity/code-smell/azure-requirements.py @@ -2,11 +2,19 @@ """Make sure the Azure requirements files match.""" import filecmp +import os -src = 'packaging/requirements/requirements-azure.txt' -dst = 'test/runner/requirements/integration.cloud.azure.txt' -if not filecmp.cmp(src, dst): - print('Update the Azure integration test requirements with the packaging test requirements:') - print('cp %s %s' % (src, dst)) - exit(1) +def main(): + src = 'packaging/requirements/requirements-azure.txt' + dst = 'test/runner/requirements/integration.cloud.azure.txt' + + if not filecmp.cmp(src, dst): + print('%s: must be identical to `%s`' % (dst, src)) + + if os.path.islink(dst): + print('%s: must not be a symbolic link' % dst) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/configure-remoting-ps1.json b/test/sanity/code-smell/configure-remoting-ps1.json new file mode 100644 index 00000000000..39ac4bd57f0 --- /dev/null +++ b/test/sanity/code-smell/configure-remoting-ps1.json @@ -0,0 +1,4 @@ +{ + "always": true, + "output": "path-message" +} diff --git a/test/sanity/code-smell/configure-remoting-ps1.py b/test/sanity/code-smell/configure-remoting-ps1.py new file mode 100755 index 00000000000..614bf6986a2 --- /dev/null +++ b/test/sanity/code-smell/configure-remoting-ps1.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python + +import os + + +def main(): + # required by external automated processes and should not be moved, renamed or converted to a symbolic link + path = 'examples/scripts/ConfigureRemotingForAnsible.ps1' + directory = path + + while True: + directory = os.path.dirname(directory) + + if not directory: + break + + if not os.path.isdir(directory): + print('%s: must be a directory' % directory) + + if os.path.islink(directory): + print('%s: cannot be a symbolic link' % directory) + + if not os.path.isfile(path): + print('%s: must be a file' % path) + + if os.path.islink(path): + print('%s: cannot be a symbolic link' % path) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/configure-remoting-ps1.sh b/test/sanity/code-smell/configure-remoting-ps1.sh deleted file mode 100755 index 1eb79031377..00000000000 --- a/test/sanity/code-smell/configure-remoting-ps1.sh +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh - -FILE='examples/scripts/ConfigureRemotingForAnsible.ps1' - -if [ ! -f "${FILE}" ] || [ -h "${FILE}" ]; then - echo 'The file "ConfigureRemotingForAnsible.ps1" is missing or is not a regular file.' - echo 'It is required by external automated processes and should not be moved or renamed.' - exit 1 -fi diff --git a/test/sanity/code-smell/empty-init.json b/test/sanity/code-smell/empty-init.json index 43d0bd35013..88487ae0892 100644 --- a/test/sanity/code-smell/empty-init.json +++ b/test/sanity/code-smell/empty-init.json @@ -4,8 +4,8 @@ "lib/ansible/module_utils/", "test/units/" ], - "extensions": [ - ".py" + "files": [ + "__init__.py" ], "output": "path-message" } diff --git a/test/sanity/code-smell/empty-init.py b/test/sanity/code-smell/empty-init.py index b0f5c320c34..453bbe04a1b 100755 --- a/test/sanity/code-smell/empty-init.py +++ b/test/sanity/code-smell/empty-init.py @@ -19,9 +19,6 @@ def main(): if path in skip: continue - if os.path.basename(path) != '__init__.py': - continue - if os.path.getsize(path) > 0: print('%s: empty __init__.py required' % path) diff --git a/test/sanity/code-smell/integration-aliases.json b/test/sanity/code-smell/integration-aliases.json new file mode 100644 index 00000000000..39ac4bd57f0 --- /dev/null +++ b/test/sanity/code-smell/integration-aliases.json @@ -0,0 +1,4 @@ +{ + "always": true, + "output": "path-message" +} diff --git a/test/sanity/code-smell/integration-aliases.py b/test/sanity/code-smell/integration-aliases.py index 50301e3b38d..b3ab6b5c455 100755 --- a/test/sanity/code-smell/integration-aliases.py +++ b/test/sanity/code-smell/integration-aliases.py @@ -10,8 +10,6 @@ def main(): with open('test/integration/target-prefixes.network', 'r') as prefixes_fd: network_prefixes = prefixes_fd.read().splitlines() - missing_aliases = [] - for target in sorted(os.listdir(targets_dir)): target_dir = os.path.join(targets_dir, target) aliases_path = os.path.join(target_dir, 'aliases') @@ -38,47 +36,7 @@ def main(): if any(target.startswith('%s_' % prefix) for prefix in network_prefixes): continue - missing_aliases.append(target_dir) - - if missing_aliases: - message = textwrap.dedent(''' - The following integration target directories are missing `aliases` files: - - %s - - If these tests cannot be run as part of CI (requires external services, unsupported dependencies, etc.), - then they most likely belong in `test/integration/roles/` instead of `test/integration/targets/`. - In that case, do not add an `aliases` file. Instead, just relocate the tests. - - However, if you think that the tests should be able to be supported by CI, please discuss test - organization with @mattclay or @gundalow on GitHub or #ansible-devel on IRC. - - If these tests can be run as part of CI, you'll need to add an appropriate CI alias, such as: - - posix/ci/group1 - windows/ci/group2 - - The CI groups are used to balance tests across multiple jobs to minimize test run time. - Using the relevant `group1` entry is fine in most cases. Groups can be changed later to redistribute tests. - - Aliases can also be used to express test requirements: - - needs/privileged - needs/root - needs/ssh - - Other aliases are used to skip tests under certain conditions: - - skip/freebsd - skip/osx - skip/python3 - - Take a look at existing `aliases` files to see what aliases are available and how they're used. - ''').strip() % '\n'.join(missing_aliases) - - print(message) - - exit(1) + print('%s: missing integration test `aliases` file' % aliases_path) if __name__ == '__main__': diff --git a/test/sanity/code-smell/no-assert.py b/test/sanity/code-smell/no-assert.py index abb663d5779..42fb59321d2 100755 --- a/test/sanity/code-smell/no-assert.py +++ b/test/sanity/code-smell/no-assert.py @@ -8,22 +8,16 @@ ASSERT_RE = re.compile(r'.*(? 0: - print('Ansible git repo should not contain any illegal filenames') - for error in errors: - print(error) - exit(1) + check_path(os.path.join(root, file_name), dir=False) if __name__ == '__main__': diff --git a/test/sanity/code-smell/no-tests-as-filters.py b/test/sanity/code-smell/no-tests-as-filters.py index 3975f436b33..18581bdc207 100755 --- a/test/sanity/code-smell/no-tests-as-filters.py +++ b/test/sanity/code-smell/no-tests-as-filters.py @@ -49,8 +49,6 @@ FILTER_RE = re.compile(r'((.+?)\s*(?P[\w \.\'"]+)(\s*)\|(\s*)(?P\w def main(): - failed = False - for path in sys.argv[1:] or sys.stdin.read().splitlines(): with open(path) as f: text = f.read() @@ -67,7 +65,6 @@ def main(): if test_name not in TESTS: continue - failed = True left = match.group('left').strip() start = match.start('left') @@ -80,9 +77,6 @@ def main(): print('%s:%d:%d: use `%s is %s` instead of `%s | %s`' % (path, lineno, colno, left, test_name, left, filter_name)) - if failed: - sys.exit(1) - if __name__ == '__main__': main() diff --git a/test/sanity/code-smell/test-constraints.json b/test/sanity/code-smell/test-constraints.json new file mode 100644 index 00000000000..2ddf53c65f2 --- /dev/null +++ b/test/sanity/code-smell/test-constraints.json @@ -0,0 +1,9 @@ +{ + "prefixes": [ + "test/runner/requirements/" + ], + "extensions": [ + ".txt" + ], + "output": "path-line-column-message" +} diff --git a/test/sanity/code-smell/test-constraints.py b/test/sanity/code-smell/test-constraints.py new file mode 100755 index 00000000000..d26515de16e --- /dev/null +++ b/test/sanity/code-smell/test-constraints.py @@ -0,0 +1,27 @@ +#!/usr/bin/env python + +import re +import sys + + +def main(): + skip = set([ + 'test/runner/requirements/constraints.txt', + 'test/runner/requirements/integration.cloud.azure.txt', + ]) + + for path in sys.argv[1:] or sys.stdin.read().splitlines(): + if path in skip: + continue + + with open(path, 'r') as path_fd: + for line, text in enumerate(path_fd.readlines()): + match = re.search(r'^[^;#]*?([<>=])(?!.*sanity_ok.*)', text) + + if match: + print('%s:%d:%d: put constraints in `test/runner/requirements/constraints.txt`' % ( + path, line + 1, match.start(1) + 1)) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/test-constraints.sh b/test/sanity/code-smell/test-constraints.sh deleted file mode 100755 index 34020e68a42..00000000000 --- a/test/sanity/code-smell/test-constraints.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/sh - -constraints=$( - grep '.' test/runner/requirements/*.txt \ - | grep -v '(sanity_ok)$' \ - | sed 's/ *;.*$//; s/ #.*$//' \ - | grep -v '/constraints.txt:' \ - | grep -v '/integration.cloud.azure.txt:' \ - | grep '[<>=]' -) - -if [ "${constraints}" ]; then - echo 'Constraints for test requirements should be in "test/runner/requirements/constraints.txt".' - echo 'The following constraints were found outside the "constraints.txt" file:' - echo "${constraints}" - exit 1 -fi diff --git a/test/sanity/compile/compile.py b/test/sanity/compile/compile.py index 01144c643e1..0641c693988 100755 --- a/test/sanity/compile/compile.py +++ b/test/sanity/compile/compile.py @@ -1,105 +1,16 @@ #!/usr/bin/env python """Python syntax checker with lint friendly output.""" -import os import parser -import re import sys def main(): - paths, verbose, skip_patterns = parse_options() - paths = filter_paths(paths, skip_patterns) - check(paths, verbose) - - -def parse_options(): - paths = [] - skip_patterns = [] - option = None - verbose = False - valid_options = [ - '-x', - '-v', - ] - - for arg in sys.argv[1:]: - if option == '-x': - skip_patterns.append(re.compile(arg)) - option = None - elif arg.startswith('-'): - if arg not in valid_options: - raise Exception('Unknown Option: %s' % arg) - if arg == '-v': - verbose = True - else: - option = arg - else: - paths.append(arg) - - if option: - raise Exception('Incomplete Option: %s' % option) - - return paths, verbose, skip_patterns - - -def filter_paths(paths, skip_patterns): - if not paths: - paths = ['.'] - - candidates = paths - paths = [] - - for candidate in candidates: - if os.path.isdir(candidate): - for root, directories, files in os.walk(candidate): - remove = [] - - for directory in directories: - if directory.startswith('.'): - remove.append(directory) - - for path in remove: - directories.remove(path) - - for f in files: - if f.endswith('.py'): - paths.append(os.path.join(root, f)) - else: - paths.append(candidate) - - final_paths = [] - - for path in sorted(paths): - skip = False - - for skip_pattern in skip_patterns: - if skip_pattern.search(path): - skip = True - break - - if skip: - continue - - final_paths.append(path) - - return final_paths - - -def check(paths, verbose): status = 0 - for path in paths: - if verbose: - sys.stderr.write('%s\n' % path) - sys.stderr.flush() - - source_fd = open(path, 'r') - - try: + for path in sys.argv[1:] or sys.stdin.read().splitlines(): + with open(path, 'r') as source_fd: source = source_fd.read() - finally: - source_fd.close() try: parser.suite(source) diff --git a/test/sanity/import/importer.py b/test/sanity/import/importer.py index 0e51527e76f..fbcd570c2ef 100755 --- a/test/sanity/import/importer.py +++ b/test/sanity/import/importer.py @@ -43,7 +43,7 @@ def main(): base_dir = os.getcwd() messages = set() - for path in sys.argv[1:]: + for path in sys.argv[1:] or sys.stdin.read().splitlines(): test_python_module(path, base_dir, messages, False) test_python_module(path, base_dir, messages, True) diff --git a/test/sanity/yamllint/yamllinter.py b/test/sanity/yamllint/yamllinter.py index 197b0dc45b0..06ff6fec744 100755 --- a/test/sanity/yamllint/yamllinter.py +++ b/test/sanity/yamllint/yamllinter.py @@ -14,7 +14,7 @@ from yamllint.config import YamlLintConfig def main(): """Main program body.""" - paths = sys.argv[1:] + paths = sys.argv[1:] or sys.stdin.read().splitlines() checker = YamlChecker() checker.check(paths)