From 7abdab6c9e6347851fbe106266e57788c58e29c7 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Wed, 24 Jan 2018 11:07:24 -0800 Subject: [PATCH] Convert ansible-test compile into a sanity test. --- .../rst/dev_guide/testing/sanity/compile.rst | 4 + .../docsite/rst/dev_guide/testing_compile.rst | 16 +-- shippable.yml | 3 +- test/runner/Makefile | 7 +- test/runner/lib/classification.py | 37 ++++-- test/runner/lib/config.py | 9 -- test/runner/lib/executor.py | 119 ------------------ test/runner/lib/sanity/compile.py | 98 +++++++++++++++ test/runner/test.py | 20 --- test/{ => sanity}/compile/compile.py | 0 test/utils/shippable/other.sh | 24 ---- test/utils/shippable/sanity.sh | 26 ++++ tox.ini | 1 - 13 files changed, 166 insertions(+), 198 deletions(-) create mode 100644 docs/docsite/rst/dev_guide/testing/sanity/compile.rst create mode 100644 test/runner/lib/sanity/compile.py rename test/{ => sanity}/compile/compile.py (100%) delete mode 100755 test/utils/shippable/other.sh create mode 100755 test/utils/shippable/sanity.sh diff --git a/docs/docsite/rst/dev_guide/testing/sanity/compile.rst b/docs/docsite/rst/dev_guide/testing/sanity/compile.rst new file mode 100644 index 00000000000..55a49b60b18 --- /dev/null +++ b/docs/docsite/rst/dev_guide/testing/sanity/compile.rst @@ -0,0 +1,4 @@ +Sanity Tests ยป compile +====================== + +See :doc:`../../testing_compile` for more information. diff --git a/docs/docsite/rst/dev_guide/testing_compile.rst b/docs/docsite/rst/dev_guide/testing_compile.rst index 240c6e5c2ba..3ac4ece1ce6 100644 --- a/docs/docsite/rst/dev_guide/testing_compile.rst +++ b/docs/docsite/rst/dev_guide/testing_compile.rst @@ -15,34 +15,36 @@ Compile tests check source files for valid syntax on all supported python versio - 3.5 - 3.6 +NOTE: In Ansible 2.4 and earlier the compile test was provided by a dedicated sub-command ``ansible-test compile`` instead of a sanity test using ``ansible-test sanity --test compile``. + Running compile tests locally ============================= -Unit tests can be run across the whole code base by doing: +Compile tests can be run across the whole code base by doing: .. code:: shell cd /path/to/ansible/source source hacking/env-setup - ansible-test compile + ansible-test sanity --test compile Against a single file by doing: .. code:: shell - ansible-test compile lineinfile + ansible-test sanity --test compile lineinfile Or against a specific Python version by doing: .. code:: shell - ansible-test compile --python 2.7 lineinfile + ansible-test sanity --test compile --python 2.7 lineinfile For advanced usage see the help: .. code:: shell - ansible-test units --help + ansible-test sanity --help Installing dependencies @@ -54,7 +56,7 @@ The dependencies can be installed using the ``--requirements`` argument. For exa .. code:: shell - ansible-test units --requirements lineinfile + ansible-test sanity --test compile --requirements lineinfile @@ -64,4 +66,4 @@ The full list of requirements can be found at `test/runner/requirements `_ so it can be discussed. +If you believe changes are needed to the compile tests please add a comment on the `Testing Working Group Agenda `_ so it can be discussed. diff --git a/shippable.yml b/shippable.yml index 101f5911239..ccc756b7b83 100644 --- a/shippable.yml +++ b/shippable.yml @@ -8,7 +8,8 @@ matrix: exclude: - env: T=none include: - - env: T=other + - env: T=sanity/1 + - env: T=sanity/2 - env: T=units/2.6 - env: T=units/2.7 diff --git a/test/runner/Makefile b/test/runner/Makefile index cdb071ad69c..a2f9d131ee1 100644 --- a/test/runner/Makefile +++ b/test/runner/Makefile @@ -1,9 +1,6 @@ -.PHONY: all compile sanity units +.PHONY: all sanity units -all: compile sanity units - -compile: - ./ansible-test compile test/runner/ ${TEST_FLAGS} +all: sanity units sanity: ./ansible-test sanity test/runner/ ${TEST_FLAGS} diff --git a/test/runner/lib/classification.py b/test/runner/lib/classification.py index c935f1ab127..49ba9b13491 100644 --- a/test/runner/lib/classification.py +++ b/test/runner/lib/classification.py @@ -44,7 +44,6 @@ def categorize_changes(args, paths, verbose_command=None): commands = { 'sanity': set(), - 'compile': set(), 'units': set(), 'integration': set(), 'windows-integration': set(), @@ -128,7 +127,6 @@ class PathMapper(object): self.sanity_targets = list(walk_sanity_targets()) self.powershell_targets = [t for t in self.sanity_targets if os.path.splitext(t.path)[1] == '.ps1'] - self.compile_paths = set(t.path for t in self.compile_targets) self.units_modules = set(t.module for t in self.units_targets if t.module) self.units_paths = set(a for t in self.units_targets for a in t.aliases) self.sanity_paths = set(t.path for t in self.sanity_targets) @@ -228,10 +226,6 @@ class PathMapper(object): if result is None: return None - # compile path if eligible - if path in self.compile_paths: - result['compile'] = path - # run sanity on path unless result specified otherwise if path in self.sanity_paths and 'sanity' not in result: result['sanity'] = path @@ -393,11 +387,6 @@ class PathMapper(object): if path.startswith('test/cache/'): return minimal - if path.startswith('test/compile/'): - return { - 'compile': 'all', - } - if path.startswith('test/results/'): return minimal @@ -544,7 +533,32 @@ class PathMapper(object): return all_tests(self.args) # test infrastructure, run all tests + if path.startswith('test/utils/shippable/tools/'): + return minimal # not used by tests + if path.startswith('test/utils/shippable/'): + if dirname == 'test/utils/shippable': + test_map = { + 'cloud.sh': 'integration:cloud/', + 'freebsd.sh': 'integration:all', + 'linux.sh': 'integration:all', + 'network.sh': 'network-integration:all', + 'osx.sh': 'integration:all', + 'rhel.sh': 'integration:all', + 'sanity.sh': 'sanity:all', + 'units.sh': 'units:all', + 'windows.sh': 'windows-integration:all', + } + + test_match = test_map.get(filename) + + if test_match: + test_command, test_target = test_match.split(':') + + return { + test_command: test_target, + } + return all_tests(self.args) # test infrastructure, run all tests if path.startswith('test/utils/'): @@ -602,7 +616,6 @@ def all_tests(args, force=False): return { 'sanity': 'all', - 'compile': 'all', 'units': 'all', 'integration': integration_all_target, 'windows-integration': integration_all_target, diff --git a/test/runner/lib/config.py b/test/runner/lib/config.py index bdec3f3976a..a9e4bd011b5 100644 --- a/test/runner/lib/config.py +++ b/test/runner/lib/config.py @@ -206,15 +206,6 @@ class UnitsConfig(TestConfig): self.collect_only = args.collect_only # type: bool -class CompileConfig(TestConfig): - """Configuration for the compile command.""" - def __init__(self, args): - """ - :type args: any - """ - super(CompileConfig, self).__init__(args, 'compile') - - class CoverageConfig(EnvironmentConfig): """Configuration for the coverage command.""" def __init__(self, args): diff --git a/test/runner/lib/executor.py b/test/runner/lib/executor.py index dab3d73098f..4c4f5f22a29 100644 --- a/test/runner/lib/executor.py +++ b/test/runner/lib/executor.py @@ -63,7 +63,6 @@ from lib.target import ( walk_network_integration_targets, walk_windows_integration_targets, walk_units_targets, - walk_compile_targets, ) from lib.changes import ( @@ -82,7 +81,6 @@ from lib.classification import ( from lib.config import ( TestConfig, EnvironmentConfig, - CompileConfig, IntegrationConfig, NetworkIntegrationConfig, PosixIntegrationConfig, @@ -91,13 +89,6 @@ from lib.config import ( WindowsIntegrationConfig, ) -from lib.test import ( - TestMessage, - TestSuccess, - TestFailure, - TestSkipped, -) - SUPPORTED_PYTHON_VERSIONS = ( '2.6', '2.7', @@ -106,8 +97,6 @@ SUPPORTED_PYTHON_VERSIONS = ( '3.7', ) -COMPILE_PYTHON_VERSIONS = SUPPORTED_PYTHON_VERSIONS - def check_startup(): """Checks to perform at startup before running commands.""" @@ -1024,114 +1013,6 @@ def command_units(args): raise -def command_compile(args): - """ - :type args: CompileConfig - """ - changes = get_changes_filter(args) - require = (args.require or []) + changes - include, exclude = walk_external_targets(walk_compile_targets(), args.include, args.exclude, require) - - if not include: - raise AllTargetsSkipped() - - if args.delegate: - raise Delegate(require=changes) - - install_command_requirements(args) - - total = 0 - failed = [] - - for version in COMPILE_PYTHON_VERSIONS: - # run all versions unless version given, in which case run only that version - if args.python and version != args.python_version: - continue - - display.info('Compile with Python %s' % version) - - result = compile_version(args, version, include, exclude) - result.write(args) - - total += 1 - - if isinstance(result, TestFailure): - failed.append('compile --python %s' % version) - - if failed: - message = 'The %d compile test(s) listed below (out of %d) failed. See error output above for details.\n%s' % ( - len(failed), total, '\n'.join(failed)) - - if args.failure_ok: - display.error(message) - else: - raise ApplicationError(message) - - -def compile_version(args, python_version, include, exclude): - """ - :type args: CompileConfig - :type python_version: str - :type include: tuple[CompletionTarget] - :type exclude: tuple[CompletionTarget] - :rtype: TestResult - """ - command = 'compile' - test = '' - - # optional list of regex patterns to exclude from tests - skip_file = 'test/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() - else: - skip_paths = [] - - # augment file exclusions - skip_paths += [e.path for e in exclude] - - skip_paths = sorted(skip_paths) - - python = 'python%s' % python_version - cmd = [python, 'test/compile/compile.py'] - - if skip_paths: - cmd += ['-x', '|'.join(skip_paths)] - - cmd += [target.path if target.path == '.' else './%s' % target.path for target in include] - - try: - stdout, stderr = run_command(args, cmd, capture=True) - status = 0 - except SubprocessError as ex: - stdout = ex.stdout - stderr = ex.stderr - status = ex.status - - if stderr: - raise SubprocessError(cmd=cmd, status=status, stderr=stderr, stdout=stdout) - - if args.explain: - return TestSkipped(command, test, python_version=python_version) - - pattern = r'^(?P[^:]*):(?P[0-9]+):(?P[0-9]+): (?P.*)$' - - results = [re.search(pattern, line).groupdict() for line in stdout.splitlines()] - - results = [TestMessage( - message=r['message'], - path=r['path'].replace('./', ''), - line=int(r['line']), - column=int(r['column']), - ) for r in results] - - if results: - return TestFailure(command, test, messages=results, python_version=python_version) - - return TestSuccess(command, test, python_version=python_version) - - def get_changes_filter(args): """ :type args: TestConfig diff --git a/test/runner/lib/sanity/compile.py b/test/runner/lib/sanity/compile.py new file mode 100644 index 00000000000..9381ac70e03 --- /dev/null +++ b/test/runner/lib/sanity/compile.py @@ -0,0 +1,98 @@ +"""Sanity test for proper python syntax.""" +from __future__ import absolute_import, print_function + +import os +import re + +from lib.sanity import ( + SanityMultipleVersion, + SanityMessage, + SanityFailure, + SanitySuccess, + SanitySkipped, +) + +from lib.util import ( + SubprocessError, + run_command, +) + +from lib.config import ( + SanityConfig, +) + +from lib.test import ( + calculate_best_confidence, +) + + +class CompileTest(SanityMultipleVersion): + """Sanity test for proper python syntax.""" + def test(self, args, targets, python_version): + """ + :type args: SanityConfig + :type targets: SanityTargets + :type python_version: str + :rtype: SanityResult + """ + # 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() + 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) + + if not paths: + return SanitySkipped(self.name, python_version=python_version) + + cmd = ['python%s' % python_version, 'test/sanity/compile/compile.py'] + paths + + try: + stdout, stderr = run_command(args, cmd, capture=True) + status = 0 + except SubprocessError as ex: + stdout = ex.stdout + stderr = ex.stderr + status = ex.status + + if stderr: + raise SubprocessError(cmd=cmd, status=status, stderr=stderr, stdout=stdout) + + if args.explain: + return SanitySuccess(self.name, python_version=python_version) + + pattern = r'^(?P[^:]*):(?P[0-9]+):(?P[0-9]+): (?P.*)$' + + results = [re.search(pattern, line).groupdict() for line in stdout.splitlines()] + + results = [SanityMessage( + message=r['message'], + path=r['path'].replace('./', ''), + line=int(r['line']), + column=int(r['column']), + ) for r in results] + + line = 0 + + for path in skip_paths: + line += 1 + + 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, + )) + + if results: + return SanityFailure(self.name, messages=results, python_version=python_version) + + return SanitySuccess(self.name, python_version=python_version) diff --git a/test/runner/test.py b/test/runner/test.py index 8f0f282b655..f6bbfee96ac 100755 --- a/test/runner/test.py +++ b/test/runner/test.py @@ -25,10 +25,8 @@ from lib.executor import ( command_network_integration, command_windows_integration, command_units, - command_compile, command_shell, SUPPORTED_PYTHON_VERSIONS, - COMPILE_PYTHON_VERSIONS, ApplicationWarning, Delegate, generate_pip_install, @@ -42,7 +40,6 @@ from lib.config import ( NetworkIntegrationConfig, SanityConfig, UnitsConfig, - CompileConfig, ShellConfig, ) @@ -58,7 +55,6 @@ from lib.target import ( walk_network_integration_targets, walk_windows_integration_targets, walk_units_targets, - walk_compile_targets, walk_sanity_targets, ) @@ -302,22 +298,6 @@ def parse_args(): add_extra_docker_options(units, integration=False) - compiler = subparsers.add_parser('compile', - parents=[test], - help='compile tests') - - compiler.set_defaults(func=command_compile, - targets=walk_compile_targets, - config=CompileConfig) - - compiler.add_argument('--python', - metavar='VERSION', - choices=COMPILE_PYTHON_VERSIONS + ('default',), - help='python version: %s' % ', '.join(COMPILE_PYTHON_VERSIONS)) - - add_lint(compiler) - add_extra_docker_options(compiler, integration=False) - sanity = subparsers.add_parser('sanity', parents=[test], help='sanity tests') diff --git a/test/compile/compile.py b/test/sanity/compile/compile.py similarity index 100% rename from test/compile/compile.py rename to test/sanity/compile/compile.py diff --git a/test/utils/shippable/other.sh b/test/utils/shippable/other.sh deleted file mode 100755 index 526339759e5..00000000000 --- a/test/utils/shippable/other.sh +++ /dev/null @@ -1,24 +0,0 @@ -#!/bin/bash -eux - -set -o pipefail - -shippable.py - -echo '{"verified": false, "results": []}' > test/results/bot/ansible-test-failure.json - -if [ "${BASE_BRANCH:-}" ]; then - base_branch="origin/${BASE_BRANCH}" -else - base_branch="" -fi -# shellcheck disable=SC2086 -ansible-test compile --failure-ok --color -v --junit ${COVERAGE:+"$COVERAGE"} ${CHANGED:+"$CHANGED"} --docker default -# shellcheck disable=SC2086 -ansible-test sanity --failure-ok --color -v --junit ${COVERAGE:+"$COVERAGE"} ${CHANGED:+"$CHANGED"} --docker default --docker-keep-git --base-branch "${base_branch}" - -rm test/results/bot/ansible-test-failure.json - -if find test/results/bot/ -mindepth 1 -name '.*' -prune -o -print -quit | grep -q .; then - echo "One or more of the above tests reported at least one failure." - exit 1 -fi diff --git a/test/utils/shippable/sanity.sh b/test/utils/shippable/sanity.sh new file mode 100755 index 00000000000..d62251b5a5e --- /dev/null +++ b/test/utils/shippable/sanity.sh @@ -0,0 +1,26 @@ +#!/bin/bash -eux + +set -o pipefail + +declare -a args +IFS='/:' read -ra args <<< "$1" + +group="${args[1]}" + +shippable.py + +if [ "${BASE_BRANCH:-}" ]; then + base_branch="origin/${BASE_BRANCH}" +else + base_branch="" +fi + +case "${group}" in + 1) options=(--skip-test pylint) ;; + 2) options=(--test pylint) ;; +esac + +# shellcheck disable=SC2086 +ansible-test sanity --color -v --junit ${COVERAGE:+"$COVERAGE"} ${CHANGED:+"$CHANGED"} \ + --docker --docker-keep-git --base-branch "${base_branch}" \ + "${options[@]}" diff --git a/tox.ini b/tox.ini index d86c38965a8..f460eb31394 100644 --- a/tox.ini +++ b/tox.ini @@ -12,7 +12,6 @@ commands = # ansible-test directly and use the --tox and --python options. # The commands below are provided as a convenience for those who # prefer to run tox directly instead of through ansible-test. - {toxinidir}/test/runner/ansible-test compile --python default -v {toxinidir}/test/runner/ansible-test sanity --python default -v {toxinidir}/test/runner/ansible-test units --python default -v passenv =