From fa48678a08115cc3cd8ef29777370f8db708d9b1 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Mon, 22 Jun 2020 20:25:35 -0700 Subject: [PATCH] Rename pylint plugin and add tests. (#70225) * Renamed custom pylint plugin for unwanted names. * Add integration tests for sanity test failures. --- .../ansible-test-pylint-plugin-name.yml | 2 + .../ns/col/plugins/modules/bad.py | 34 ++++++ .../integration/targets/hello/files/bad.py | 16 +++ .../ns/col/tests/sanity/ignore.txt | 5 + .../ansible-test/collection-tests/venv.sh | 4 + .../plugins/{blacklist.py => unwanted.py} | 104 +++++++++--------- test/sanity/ignore.txt | 3 + 7 files changed, 116 insertions(+), 52 deletions(-) create mode 100644 changelogs/fragments/ansible-test-pylint-plugin-name.yml create mode 100644 test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/modules/bad.py create mode 100644 test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py create mode 100644 test/integration/targets/ansible-test/ansible_collections/ns/col/tests/sanity/ignore.txt rename test/lib/ansible_test/_data/sanity/pylint/plugins/{blacklist.py => unwanted.py} (66%) diff --git a/changelogs/fragments/ansible-test-pylint-plugin-name.yml b/changelogs/fragments/ansible-test-pylint-plugin-name.yml new file mode 100644 index 00000000000..31239b5ceb6 --- /dev/null +++ b/changelogs/fragments/ansible-test-pylint-plugin-name.yml @@ -0,0 +1,2 @@ +minor_changes: + - ansible-test - Changed the internal name of the custom plugin used to identify use of unwanted imports and functions. diff --git a/test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/modules/bad.py b/test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/modules/bad.py new file mode 100644 index 00000000000..e79613bbbe4 --- /dev/null +++ b/test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/modules/bad.py @@ -0,0 +1,34 @@ +#!/usr/bin/python +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +DOCUMENTATION = ''' +module: bad +short_description: Bad test module +description: Bad test module. +author: + - Ansible Core Team +''' + +EXAMPLES = ''' +- bad: +''' + +RETURN = '''''' + +from ansible.module_utils.basic import AnsibleModule +from ansible import constants # intentionally trigger pylint ansible-bad-module-import error + + +def main(): + module = AnsibleModule( + argument_spec=dict(), + ) + + module.exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py b/test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py new file mode 100644 index 00000000000..82215438fb1 --- /dev/null +++ b/test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py @@ -0,0 +1,16 @@ +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +import tempfile + +try: + import urllib2 # intentionally trigger pylint ansible-bad-import error +except ImportError: + urllib2 = None + +try: + from urllib2 import Request # intentionally trigger pylint ansible-bad-import-from error +except ImportError: + Request = None + +tempfile.mktemp() # intentionally trigger pylint ansible-bad-function error diff --git a/test/integration/targets/ansible-test/ansible_collections/ns/col/tests/sanity/ignore.txt b/test/integration/targets/ansible-test/ansible_collections/ns/col/tests/sanity/ignore.txt new file mode 100644 index 00000000000..6d7877ba1e7 --- /dev/null +++ b/test/integration/targets/ansible-test/ansible_collections/ns/col/tests/sanity/ignore.txt @@ -0,0 +1,5 @@ +plugins/modules/bad.py import +plugins/modules/bad.py pylint:ansible-bad-module-import +tests/integration/targets/hello/files/bad.py pylint:ansible-bad-function +tests/integration/targets/hello/files/bad.py pylint:ansible-bad-import +tests/integration/targets/hello/files/bad.py pylint:ansible-bad-import-from diff --git a/test/integration/targets/ansible-test/collection-tests/venv.sh b/test/integration/targets/ansible-test/collection-tests/venv.sh index 6ff496b6a17..862c8ad9899 100755 --- a/test/integration/targets/ansible-test/collection-tests/venv.sh +++ b/test/integration/targets/ansible-test/collection-tests/venv.sh @@ -5,6 +5,10 @@ set -eux -o pipefail cp -a "${TEST_DIR}/ansible_collections" "${WORK_DIR}" cd "${WORK_DIR}/ansible_collections/ns/col" +# rename the sanity ignore file to match the current ansible version and update import ignores with the python version +ansible_version="$(python -c 'import ansible.release; print(".".join(ansible.release.__version__.split(".")[:2]))')" +sed "s/ import$/ import-${ANSIBLE_TEST_PYTHON_VERSION}/;" < "tests/sanity/ignore.txt" > "tests/sanity/ignore-${ansible_version}.txt" + # common args for all tests # each test will be run in a separate venv to verify that requirements have been properly specified common=(--venv --python "${ANSIBLE_TEST_PYTHON_VERSION}" --color --truncate 0 "${@}") diff --git a/test/lib/ansible_test/_data/sanity/pylint/plugins/blacklist.py b/test/lib/ansible_test/_data/sanity/pylint/plugins/unwanted.py similarity index 66% rename from test/lib/ansible_test/_data/sanity/pylint/plugins/blacklist.py rename to test/lib/ansible_test/_data/sanity/pylint/plugins/unwanted.py index ac53aedaa65..7012feaa585 100644 --- a/test/lib/ansible_test/_data/sanity/pylint/plugins/blacklist.py +++ b/test/lib/ansible_test/_data/sanity/pylint/plugins/unwanted.py @@ -14,8 +14,8 @@ ANSIBLE_TEST_MODULES_PATH = os.environ['ANSIBLE_TEST_MODULES_PATH'] ANSIBLE_TEST_MODULE_UTILS_PATH = os.environ['ANSIBLE_TEST_MODULE_UTILS_PATH'] -class BlacklistEntry: - """Defines a import blacklist entry.""" +class UnwantedEntry: + """Defines an unwanted import.""" def __init__(self, alternative, modules_only=False, names=None, ignore_paths=None): """ :type alternative: str @@ -58,11 +58,11 @@ def is_module_path(path): return path.startswith(ANSIBLE_TEST_MODULES_PATH) or path.startswith(ANSIBLE_TEST_MODULE_UTILS_PATH) -class AnsibleBlacklistChecker(BaseChecker): - """Checker for blacklisted imports and functions.""" +class AnsibleUnwantedChecker(BaseChecker): + """Checker for unwanted imports and functions.""" __implements__ = (IAstroidChecker,) - name = 'blacklist' + name = 'unwanted' BAD_IMPORT = 'ansible-bad-import' BAD_IMPORT_FROM = 'ansible-bad-import-from' @@ -84,58 +84,58 @@ class AnsibleBlacklistChecker(BaseChecker): 'Identifies imports which should not be used.'), ) - blacklist_imports = dict( + unwanted_imports = dict( # Additional imports that we may want to start checking: - # boto=BlacklistEntry('boto3', modules_only=True), - # requests=BlacklistEntry('ansible.module_utils.urls', modules_only=True), - # urllib=BlacklistEntry('ansible.module_utils.urls', modules_only=True), + # boto=UnwantedEntry('boto3', modules_only=True), + # requests=UnwantedEntry('ansible.module_utils.urls', modules_only=True), + # urllib=UnwantedEntry('ansible.module_utils.urls', modules_only=True), # see https://docs.python.org/2/library/urllib2.html - urllib2=BlacklistEntry('ansible.module_utils.urls', - ignore_paths=( - '/lib/ansible/module_utils/urls.py', - )), + urllib2=UnwantedEntry('ansible.module_utils.urls', + ignore_paths=( + '/lib/ansible/module_utils/urls.py', + )), # see https://docs.python.org/3.7/library/collections.abc.html - collections=BlacklistEntry('ansible.module_utils.common._collections_compat', - ignore_paths=( - '/lib/ansible/module_utils/common/_collections_compat.py', - ), - names=( - 'MappingView', - 'ItemsView', - 'KeysView', - 'ValuesView', - 'Mapping', 'MutableMapping', - 'Sequence', 'MutableSequence', - 'Set', 'MutableSet', - 'Container', - 'Hashable', - 'Sized', - 'Callable', - 'Iterable', - 'Iterator', - )), + collections=UnwantedEntry('ansible.module_utils.common._collections_compat', + ignore_paths=( + '/lib/ansible/module_utils/common/_collections_compat.py', + ), + names=( + 'MappingView', + 'ItemsView', + 'KeysView', + 'ValuesView', + 'Mapping', 'MutableMapping', + 'Sequence', 'MutableSequence', + 'Set', 'MutableSet', + 'Container', + 'Hashable', + 'Sized', + 'Callable', + 'Iterable', + 'Iterator', + )), ) - blacklist_functions = { + unwanted_functions = { # see https://docs.python.org/2/library/tempfile.html#tempfile.mktemp - 'tempfile.mktemp': BlacklistEntry('tempfile.mkstemp'), + 'tempfile.mktemp': UnwantedEntry('tempfile.mkstemp'), - 'sys.exit': BlacklistEntry('exit_json or fail_json', - ignore_paths=( - '/lib/ansible/module_utils/basic.py', - '/lib/ansible/modules/async_wrapper.py', - '/lib/ansible/module_utils/common/removed.py', - ), - modules_only=True), + 'sys.exit': UnwantedEntry('exit_json or fail_json', + ignore_paths=( + '/lib/ansible/module_utils/basic.py', + '/lib/ansible/modules/async_wrapper.py', + '/lib/ansible/module_utils/common/removed.py', + ), + modules_only=True), - 'builtins.print': BlacklistEntry('module.log or module.debug', - ignore_paths=( - '/lib/ansible/module_utils/basic.py', - '/lib/ansible/module_utils/common/removed.py', - ), - modules_only=True), + 'builtins.print': UnwantedEntry('module.log or module.debug', + ignore_paths=( + '/lib/ansible/module_utils/basic.py', + '/lib/ansible/module_utils/common/removed.py', + ), + modules_only=True), } def visit_import(self, node): @@ -163,7 +163,7 @@ class AnsibleBlacklistChecker(BaseChecker): module = last_child.name - entry = self.blacklist_imports.get(module) + entry = self.unwanted_imports.get(module) if entry and entry.names: if entry.applies_to(self.linter.current_file, node.attrname): @@ -183,7 +183,7 @@ class AnsibleBlacklistChecker(BaseChecker): if not func: continue - entry = self.blacklist_functions.get(func) + entry = self.unwanted_functions.get(func) if entry and entry.applies_to(self.linter.current_file): self.add_message(self.BAD_FUNCTION, args=(entry.alternative, func), node=node) @@ -197,7 +197,7 @@ class AnsibleBlacklistChecker(BaseChecker): """ self._check_module_import(node, modname) - entry = self.blacklist_imports.get(modname) + entry = self.unwanted_imports.get(modname) if not entry: return @@ -213,7 +213,7 @@ class AnsibleBlacklistChecker(BaseChecker): """ self._check_module_import(node, modname) - entry = self.blacklist_imports.get(modname) + entry = self.unwanted_imports.get(modname) if not entry: return @@ -239,4 +239,4 @@ class AnsibleBlacklistChecker(BaseChecker): def register(linter): """required method to auto register this checker """ - linter.register_checker(AnsibleBlacklistChecker(linter)) + linter.register_checker(AnsibleUnwantedChecker(linter)) diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index ff4cfeac7d8..c927abfee0b 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -190,6 +190,9 @@ lib/ansible/plugins/strategy/__init__.py pylint:ansible-deprecated-version lib/ansible/plugins/strategy/__init__.py pylint:blacklisted-name lib/ansible/plugins/strategy/linear.py pylint:blacklisted-name lib/ansible/vars/hostvars.py pylint:blacklisted-name +test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py pylint:ansible-bad-import +test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py pylint:ansible-bad-import-from +test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py pylint:ansible-bad-function test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/modules/hello.py pylint:relative-beyond-top-level test/integration/targets/ansible-test/ansible_collections/ns/col/tests/unit/plugins/module_utils/test_my_util.py pylint:relative-beyond-top-level test/integration/targets/ansible-test/ansible_collections/ns/col/tests/unit/plugins/modules/test_hello.py pylint:relative-beyond-top-level