Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul additional sanity tests. #36803

Merged
merged 11 commits into from
Feb 27, 2018
18 changes: 16 additions & 2 deletions test/runner/lib/sanity/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import json
import os
import re
import sys

from lib.util import (
ApplicationError,
Expand Down Expand Up @@ -233,6 +234,8 @@ def test(self, args, targets):
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<path>[^:]*):(?P<line>[0-9]+):(?P<column>[0-9]+): (?P<message>.*)$'
Expand All @@ -243,18 +246,29 @@ def test(self, args, targets):

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
Expand Down
9 changes: 7 additions & 2 deletions test/runner/lib/sanity/compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from lib.util import (
SubprocessError,
run_command,
display,
)

from lib.config import (
Expand Down Expand Up @@ -49,10 +50,14 @@ def test(self, args, targets, python_version):
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
Expand Down
10 changes: 8 additions & 2 deletions test/runner/lib/sanity/import.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
run_command,
intercept_command,
remove_tree,
display,
)

from lib.ansible_util import (
Expand Down Expand Up @@ -88,12 +89,17 @@ def test(self, args, targets, python_version):
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)
Expand Down
9 changes: 7 additions & 2 deletions test/runner/lib/sanity/yamllint.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from lib.util import (
SubprocessError,
run_command,
display,
)

from lib.config import (
Expand Down Expand Up @@ -66,10 +67,14 @@ def test_paths(self, args, paths):
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
Expand Down
4 changes: 4 additions & 0 deletions test/sanity/code-smell/azure-requirements.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"always": true,
"output": "path-message"
}
20 changes: 14 additions & 6 deletions test/sanity/code-smell/azure-requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
4 changes: 4 additions & 0 deletions test/sanity/code-smell/configure-remoting-ps1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"always": true,
"output": "path-message"
}
31 changes: 31 additions & 0 deletions test/sanity/code-smell/configure-remoting-ps1.py
Original file line number Diff line number Diff line change
@@ -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()
9 changes: 0 additions & 9 deletions test/sanity/code-smell/configure-remoting-ps1.sh

This file was deleted.

4 changes: 2 additions & 2 deletions test/sanity/code-smell/empty-init.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
"lib/ansible/module_utils/",
"test/units/"
],
"extensions": [
".py"
"files": [
"__init__.py"
],
"output": "path-message"
}
3 changes: 0 additions & 3 deletions test/sanity/code-smell/empty-init.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
4 changes: 4 additions & 0 deletions test/sanity/code-smell/integration-aliases.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"always": true,
"output": "path-message"
}
44 changes: 1 addition & 43 deletions test/sanity/code-smell/integration-aliases.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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__':
Expand Down
6 changes: 0 additions & 6 deletions test/sanity/code-smell/no-assert.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,16 @@


def main():
failed = False

for path in sys.argv[1:] or sys.stdin.read().splitlines():
with open(path, 'r') as f:
for i, line in enumerate(f.readlines()):
matches = ASSERT_RE.findall(line)

if matches:
failed = True
lineno = i + 1
colno = line.index('assert') + 1
print('%s:%d:%d: raise AssertionError instead of: %s' % (path, lineno, colno, matches[0][colno - 1:]))

if failed:
sys.exit(1)


if __name__ == '__main__':
main()
4 changes: 4 additions & 0 deletions test/sanity/code-smell/no-illegal-filenames.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"always": true,
"output": "path-message"
}
26 changes: 11 additions & 15 deletions test/sanity/code-smell/no-illegal-filenames.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,45 +54,41 @@


def check_path(path, dir=False):
errors = []
type_name = 'directory' if dir else 'file'
parent, file_name = os.path.split(path)
name, ext = os.path.splitext(file_name)

if name.upper() in ILLEGAL_NAMES:
errors.append("Illegal %s name %s: %s" % (type_name, name.upper(), path))
print("%s: illegal %s name %s" % (path, type_name, name.upper()))

if file_name[-1] in ILLEGAL_END_CHARS:
errors.append("Illegal %s name end-char '%s': %s" % (type_name, file_name[-1], path))
print("%s: illegal %s name end-char '%s'" % (path, type_name, file_name[-1]))

bfile = to_bytes(file_name, encoding='utf-8')
for char in ILLEGAL_CHARS:
if char in bfile:
bpath = to_bytes(path, encoding='utf-8')
errors.append("Illegal char %s in %s name: %s" % (char, type_name, bpath))
return errors
print("%s: illegal char '%s' in %s name" % (bpath, char, type_name))


def main():
errors = []
pattern = re.compile("^./test/integration/targets/.*/backup")
pattern = re.compile("^test/integration/targets/.*/backup")

for root, dirs, files in os.walk('.'):
if root == '.':
root = ''
elif root.startswith('./'):
root = root[2:]

# ignore test/integration/targets/*/backup
if pattern.match(root):
continue

for dir_name in dirs:
errors += check_path(os.path.abspath(os.path.join(root, dir_name)), dir=True)
check_path(os.path.join(root, dir_name), dir=True)

for file_name in files:
errors += check_path(os.path.abspath(os.path.join(root, file_name)), dir=False)

if len(errors) > 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__':
Expand Down