diff options
author | Matt Clay <matt@mystile.com> | 2018-02-21 20:09:13 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-02-21 20:09:13 -0800 |
commit | 891f4f3b2d1b2fcab1b64e0ea942d186f98b7406 (patch) | |
tree | 46c34272bb37e8e1427919c0759e149a683fcc90 /test/sanity | |
parent | a4df4d33ac72e048694f662a358d96b7f487bf83 (diff) | |
download | ansible-891f4f3b2d1b2fcab1b64e0ea942d186f98b7406.tar.gz |
Upgrade more code-smell tests. (#36560)
* Enhance no-dict-* code-smell tests.
* Enhance no-basestring code-smell test.
* Enhance no-get-exception code-smell test.
* Enhance empty-init code-smell test.
* Enhance required-and-default-attribute test.
* Remove unused code-smell test.
Diffstat (limited to 'test/sanity')
23 files changed, 261 insertions, 161 deletions
diff --git a/test/sanity/code-smell/empty-init.json b/test/sanity/code-smell/empty-init.json new file mode 100644 index 0000000000..43d0bd3501 --- /dev/null +++ b/test/sanity/code-smell/empty-init.json @@ -0,0 +1,11 @@ +{ + "prefixes": [ + "lib/ansible/modules/", + "lib/ansible/module_utils/", + "test/units/" + ], + "extensions": [ + ".py" + ], + "output": "path-message" +} diff --git a/test/sanity/code-smell/empty-init.py b/test/sanity/code-smell/empty-init.py new file mode 100755 index 0000000000..f55817dc67 --- /dev/null +++ b/test/sanity/code-smell/empty-init.py @@ -0,0 +1,30 @@ +#!/usr/bin/env python + +import os +import re +import sys + + +def main(): + skip = set([ + 'test/sanity/code-smell/%s' % os.path.basename(__file__), + # facts is grandfathered in but will break namespacing + # the only way to fix it is to deprecate and eventually remove it + # six will break namespacing but because it is bundled we should not be overriding it + 'lib/ansible/module_utils/facts/__init__.py', + 'lib/ansible/module_utils/six/__init__.py', + ]) + + for path in sys.argv[1:]: + 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) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/empty-init.sh b/test/sanity/code-smell/empty-init.sh deleted file mode 100755 index 3ae810f5f9..0000000000 --- a/test/sanity/code-smell/empty-init.sh +++ /dev/null @@ -1,20 +0,0 @@ -#!/bin/sh - -found='' - -for path in lib/ansible/modules/ lib/ansible/module_utils test/units/; do - # facts is grandfathered in but will break namespacing. Only way to fix it - # is to deprecate and eventually remove. - # six will break namespacing but because it is bundled we should not be overriding it - files=$(find "${path}" -name __init__.py -size '+0' | sed '\!lib/ansible/module_utils/\(six\|facts\)/__init__.py!d') - - if [ "${files}" ]; then - echo "${files}" - found=1 - fi -done - -if [ "${found}" ]; then - echo "One or more __init__.py file(s) listed above are non-empty." - exit 1 -fi diff --git a/test/sanity/code-smell/inappropriately-private.sh b/test/sanity/code-smell/inappropriately-private.sh deleted file mode 100755 index 9ee3342179..0000000000 --- a/test/sanity/code-smell/inappropriately-private.sh +++ /dev/null @@ -1,18 +0,0 @@ -#!/bin/sh - -# -# Test that we do not access private attributes of other objects. -# -# * private attributes of ourself are okay: self._private. -# * Private attributes of other objects are not: self.other._private -# - -# Currently the code has many places where we're violating this test so we need -# to clean up the code before we can enable this. Maybe we'll need to -# selectively blacklist modules so that we can work on this a piece at a time. -# -# Also need to implement whitelist for certain things like bundled libraries -# that violate this. -# -# 23-10-2015: Count was 508 lines -grep -Pri '(?<!self)\._(?!_)' "$1" | grep -v modules diff --git a/test/sanity/code-smell/no-basestring.json b/test/sanity/code-smell/no-basestring.json new file mode 100644 index 0000000000..776590b74d --- /dev/null +++ b/test/sanity/code-smell/no-basestring.json @@ -0,0 +1,6 @@ +{ + "extensions": [ + ".py" + ], + "output": "path-line-column-message" +} diff --git a/test/sanity/code-smell/no-basestring.py b/test/sanity/code-smell/no-basestring.py new file mode 100755 index 0000000000..e1743807b5 --- /dev/null +++ b/test/sanity/code-smell/no-basestring.py @@ -0,0 +1,28 @@ +#!/usr/bin/env python + +import os +import re +import sys + + +def main(): + skip = set([ + 'test/sanity/code-smell/%s' % os.path.basename(__file__), + 'lib/ansible/module_utils/six/__init__.py', + ]) + + for path in sys.argv[1:]: + if path in skip: + continue + + with open(path, 'r') as path_fd: + for line, text in enumerate(path_fd.readlines()): + match = re.search(r'(isinstance.*basestring)', text) + + if match: + print('%s:%d:%d: do not use `isinstance(s, basestring)`' % ( + path, line + 1, match.start(1) + 1)) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/no-basestring.sh b/test/sanity/code-smell/no-basestring.sh deleted file mode 100755 index 1a2ae23613..0000000000 --- a/test/sanity/code-smell/no-basestring.sh +++ /dev/null @@ -1,18 +0,0 @@ -#!/bin/sh - -BASESTRING_USERS=$(grep -r basestring . \ - --exclude-dir .git \ - --exclude-dir .tox \ - | grep isinstance \ - | grep -v \ - -e test/results/ \ - -e docs/docsite/_build/ \ - -e docs/docsite/rst/dev_guide/testing/sanity/ \ - -e lib/ansible/module_utils/six/__init__.py \ - -e '^[^:]*:#' - ) - -if [ "${BASESTRING_USERS}" ]; then - echo "${BASESTRING_USERS}" - exit 1 -fi diff --git a/test/sanity/code-smell/no-dict-iteritems.json b/test/sanity/code-smell/no-dict-iteritems.json new file mode 100644 index 0000000000..776590b74d --- /dev/null +++ b/test/sanity/code-smell/no-dict-iteritems.json @@ -0,0 +1,6 @@ +{ + "extensions": [ + ".py" + ], + "output": "path-line-column-message" +} diff --git a/test/sanity/code-smell/no-dict-iteritems.py b/test/sanity/code-smell/no-dict-iteritems.py new file mode 100755 index 0000000000..95b3d79b1d --- /dev/null +++ b/test/sanity/code-smell/no-dict-iteritems.py @@ -0,0 +1,28 @@ +#!/usr/bin/env python + +import os +import re +import sys + + +def main(): + skip = set([ + 'test/sanity/code-smell/%s' % os.path.basename(__file__), + 'lib/ansible/module_utils/six/__init__.py', + ]) + + for path in sys.argv[1:]: + if path in skip: + continue + + with open(path, 'r') as path_fd: + for line, text in enumerate(path_fd.readlines()): + match = re.search(r'(?<! six)\.(iteritems)', text) + + if match: + print('%s:%d:%d: use `dict.items` or `ansible.module_utils.six.iteritems` instead of `dict.iteritems`' % ( + path, line + 1, match.start(1) + 1)) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/no-dict-iteritems.sh b/test/sanity/code-smell/no-dict-iteritems.sh deleted file mode 100755 index 1dbf563630..0000000000 --- a/test/sanity/code-smell/no-dict-iteritems.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/bin/sh - -ITERITEMS_USERS=$(grep -rI '\.iteritems' . \ - --exclude-dir .git \ - --exclude-dir .tox \ - --exclude-dir docsite \ - | grep -v \ - -e 'six\.iteritems' \ - -e lib/ansible/module_utils/six/__init__.py \ - -e test/sanity/code-smell/no-dict-iteritems.sh \ - ) - -if [ "${ITERITEMS_USERS}" ]; then - echo 'iteritems has been removed in python3. Alternatives:' - echo ' for KEY, VALUE in DICT.items():' - echo ' from ansible.module_utils.six import iteritems ; for KEY, VALUE in iteritems(DICT):' - echo "${ITERITEMS_USERS}" - exit 1 -fi diff --git a/test/sanity/code-smell/no-dict-iterkeys.json b/test/sanity/code-smell/no-dict-iterkeys.json new file mode 100644 index 0000000000..776590b74d --- /dev/null +++ b/test/sanity/code-smell/no-dict-iterkeys.json @@ -0,0 +1,6 @@ +{ + "extensions": [ + ".py" + ], + "output": "path-line-column-message" +} diff --git a/test/sanity/code-smell/no-dict-iterkeys.py b/test/sanity/code-smell/no-dict-iterkeys.py new file mode 100755 index 0000000000..5309ab7ceb --- /dev/null +++ b/test/sanity/code-smell/no-dict-iterkeys.py @@ -0,0 +1,28 @@ +#!/usr/bin/env python + +import os +import re +import sys + + +def main(): + skip = set([ + 'test/sanity/code-smell/%s' % os.path.basename(__file__), + 'lib/ansible/module_utils/six/__init__.py', + ]) + + for path in sys.argv[1:]: + if path in skip: + continue + + with open(path, 'r') as path_fd: + for line, text in enumerate(path_fd.readlines()): + match = re.search(r'\.(iterkeys)', text) + + if match: + print('%s:%d:%d: use `dict.keys` or `for key in dict:` instead of `dict.iterkeys`' % ( + path, line + 1, match.start(1) + 1)) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/no-dict-iterkeys.sh b/test/sanity/code-smell/no-dict-iterkeys.sh deleted file mode 100755 index 5cd3e8d4e9..0000000000 --- a/test/sanity/code-smell/no-dict-iterkeys.sh +++ /dev/null @@ -1,22 +0,0 @@ -#!/bin/sh - -ITERKEYS_USERS=$(grep -r -I iterkeys . \ - --exclude-dir .git \ - --exclude-dir .tox \ - --exclude-dir .idea \ - --exclude-dir docsite \ - --exclude-dir results \ - | grep -v \ - -e 'metadata-.*.json:' \ - -e lib/ansible.egg-info/ \ - -e lib/ansible/module_utils/six/__init__.py \ - -e docs/docsite/rst/dev_guide/testing/sanity/ \ - -e test/sanity/code-smell/no-dict-iterkeys.sh \ - -e '^[^:]*:#' - ) - -if [ "${ITERKEYS_USERS}" ]; then - echo 'iterkeys has been removed in python3. Use "for KEY in DICT:" instead' - echo "${ITERKEYS_USERS}" - exit 1 -fi diff --git a/test/sanity/code-smell/no-dict-itervalues.json b/test/sanity/code-smell/no-dict-itervalues.json new file mode 100644 index 0000000000..776590b74d --- /dev/null +++ b/test/sanity/code-smell/no-dict-itervalues.json @@ -0,0 +1,6 @@ +{ + "extensions": [ + ".py" + ], + "output": "path-line-column-message" +} diff --git a/test/sanity/code-smell/no-dict-itervalues.py b/test/sanity/code-smell/no-dict-itervalues.py new file mode 100755 index 0000000000..e503ffb67e --- /dev/null +++ b/test/sanity/code-smell/no-dict-itervalues.py @@ -0,0 +1,28 @@ +#!/usr/bin/env python + +import os +import re +import sys + + +def main(): + skip = set([ + 'test/sanity/code-smell/%s' % os.path.basename(__file__), + 'lib/ansible/module_utils/six/__init__.py', + ]) + + for path in sys.argv[1:]: + if path in skip: + continue + + with open(path, 'r') as path_fd: + for line, text in enumerate(path_fd.readlines()): + match = re.search(r'(?<! six)\.(itervalues)', text) + + if match: + print('%s:%d:%d: use `dict.values` or `ansible.module_utils.six.itervalues` instead of `dict.itervalues`' % ( + path, line + 1, match.start(1) + 1)) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/no-dict-itervalues.sh b/test/sanity/code-smell/no-dict-itervalues.sh deleted file mode 100755 index 0970bb4893..0000000000 --- a/test/sanity/code-smell/no-dict-itervalues.sh +++ /dev/null @@ -1,19 +0,0 @@ -#!/bin/sh - -ITERVALUES_USERS=$(grep -rI '\.itervalues' . \ - --exclude-dir .git \ - --exclude-dir .tox \ - --exclude-dir docsite \ - | grep -v \ - -e 'six\.itervalues' \ - -e lib/ansible/module_utils/six/__init__.py \ - -e test/sanity/code-smell/no-dict-itervalues.sh \ - ) - -if [ "${ITERVALUES_USERS}" ]; then - echo 'itervalues has been removed in python3. Alternatives:' - echo ' for VALUE in DICT.values():' - echo ' from ansible.module_utils.six import itervalues ; for VALUE in itervalues(DICT):' - echo "${ITERVALUES_USERS}" - exit 1 -fi diff --git a/test/sanity/code-smell/no-get-exception.json b/test/sanity/code-smell/no-get-exception.json new file mode 100644 index 0000000000..776590b74d --- /dev/null +++ b/test/sanity/code-smell/no-get-exception.json @@ -0,0 +1,6 @@ +{ + "extensions": [ + ".py" + ], + "output": "path-line-column-message" +} diff --git a/test/sanity/code-smell/no-get-exception.py b/test/sanity/code-smell/no-get-exception.py new file mode 100755 index 0000000000..d09fa23b35 --- /dev/null +++ b/test/sanity/code-smell/no-get-exception.py @@ -0,0 +1,42 @@ +#!/usr/bin/env python + +import os +import re +import sys + + +def main(): + skip = set([ + 'test/sanity/code-smell/%s' % os.path.basename(__file__), + 'lib/ansible/module_utils/pycompat24.py', + 'lib/ansible/module_utils/six/__init__.py', + # the following files should be fixed and removed from this list + 'lib/ansible/modules/network/cloudengine/ce_file_copy.py', + 'lib/ansible/modules/network/panos/panos_dag_tags.py', + 'lib/ansible/modules/network/panos/panos_match_rule.py', + 'lib/ansible/modules/network/panos/panos_op.py', + 'lib/ansible/modules/system/sefcontext.py', + ]) + + basic_allow_once = True + + for path in sys.argv[1:]: + if path in skip: + continue + + with open(path, 'r') as path_fd: + for line, text in enumerate(path_fd.readlines()): + match = re.search(r'(get_exception)', text) + + if match: + if path == 'lib/ansible/module_utils/basic.py' and basic_allow_once: + # basic.py is allowed to import get_exception for backwards compatibility but should not call it anywhere + basic_allow_once = False + continue + + print('%s:%d:%d: do not use `get_exception`' % ( + path, line + 1, match.start(1) + 1)) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/no-get-exception.sh b/test/sanity/code-smell/no-get-exception.sh deleted file mode 100755 index ba6448ec25..0000000000 --- a/test/sanity/code-smell/no-get-exception.sh +++ /dev/null @@ -1,34 +0,0 @@ -#!/bin/sh - -# We're getting rid of get_exception in our code so that we can deprecate it. -# get_exception is no longer needed as we no longer support python-2.4 on the controller. - -# We eventually want pycompat24 and basic.py to be the only things on this list -get_exception=$(find . -path ./test/runner/.tox -prune \ - -o -path ./lib/ansible/module_utils/pycompat24.py -prune \ - -o -path ./lib/ansible/module_utils/basic.py -prune \ - -o -path ./lib/ansible/modules/network/panos -prune \ - -o -path ./lib/ansible/modules/network/junos/junos_facts.py -prune \ - -o -path ./lib/ansible/modules/network/junos/junos_package.py -prune \ - -o -path ./lib/ansible/modules/network/cloudengine/ce_file_copy.py -prune \ - -o -path ./lib/ansible/modules/system -prune \ - -o -name '*.py' -type f -exec grep -H 'get_exception' '{}' '+') - -basic_failed=0 - -if test "$(grep -c get_exception ./lib/ansible/module_utils/basic.py)" -gt 1 ; then - printf "\n== basic.py contains get_exception calls ==\n\n" - printf " basic.py is allowed to import get_exception for backwards compat but\n" - printf " should not call it anywhere\n" - basic_failed=1 -fi - -failures=0 -if test -n "$get_exception" ; then - printf "\n== Contains get_exception() calls ==\n" - printf " %s\n" "$get_exception" - failures=$(printf "%s" "$get_exception"| wc -l) - failures=$((failures + 2)) -fi - -exit $((basic_failed + failures)) diff --git a/test/sanity/code-smell/required-and-default-attributes.json b/test/sanity/code-smell/required-and-default-attributes.json new file mode 100644 index 0000000000..dd9ac7b1f9 --- /dev/null +++ b/test/sanity/code-smell/required-and-default-attributes.json @@ -0,0 +1,9 @@ +{ + "prefixes": [ + "lib/ansible/" + ], + "extensions": [ + ".py" + ], + "output": "path-line-column-message" +} diff --git a/test/sanity/code-smell/required-and-default-attributes.py b/test/sanity/code-smell/required-and-default-attributes.py new file mode 100755 index 0000000000..5c6f86c5b9 --- /dev/null +++ b/test/sanity/code-smell/required-and-default-attributes.py @@ -0,0 +1,27 @@ +#!/usr/bin/env python + +import os +import re +import sys + + +def main(): + skip = set([ + 'test/sanity/code-smell/%s' % os.path.basename(__file__), + ]) + + for path in sys.argv[1:]: + if path in skip: + continue + + with open(path, 'r') as path_fd: + for line, text in enumerate(path_fd.readlines()): + match = re.search(r'(FieldAttribute.*(default|required).*(default|required))', text) + + if match: + print('%s:%d:%d: use only one of `default` or `required` with `FieldAttribute`' % ( + path, line + 1, match.start(1) + 1)) + + +if __name__ == '__main__': + main() diff --git a/test/sanity/code-smell/required-and-default-attributes.sh b/test/sanity/code-smell/required-and-default-attributes.sh deleted file mode 100755 index 9822a15597..0000000000 --- a/test/sanity/code-smell/required-and-default-attributes.sh +++ /dev/null @@ -1,10 +0,0 @@ -#!/bin/sh - -BASEDIR=${1-"lib/ansible"} -cd "$BASEDIR" -grep -r FieldAttribute . |grep 'default' | grep 'required' -if test $? -eq 0 ; then - exit 1 -fi -exit 0 - diff --git a/test/sanity/code-smell/skip.txt b/test/sanity/code-smell/skip.txt index bfb8b181a2..e69de29bb2 100644 --- a/test/sanity/code-smell/skip.txt +++ b/test/sanity/code-smell/skip.txt @@ -1 +0,0 @@ -inappropriately-private.sh |