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/changelogs/fragments/ansible-test-pylint-python-3.8-3.9.yml b/changelogs/fragments/ansible-test-pylint-python-3.8-3.9.yml new file mode 100644 index 00000000000..9668f7aa8b4 --- /dev/null +++ b/changelogs/fragments/ansible-test-pylint-python-3.8-3.9.yml @@ -0,0 +1,3 @@ +minor_changes: + - ansible-test - The ``pylint`` sanity test is now supported on Python 3.8. + - ansible-test - The ``pylint`` sanity test is now skipped with a warning on Python 3.9 due to unresolved upstream regressions. diff --git a/test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/filter/check_pylint.py b/test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/filter/check_pylint.py new file mode 100644 index 00000000000..359fbf071c9 --- /dev/null +++ b/test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/filter/check_pylint.py @@ -0,0 +1,21 @@ +""" +These test cases verify ansible-test version constraints for pylint and its dependencies across Python versions. +The initial test cases were discovered while testing various Python versions against ansible/ansible. +""" +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +# Python 3.8 fails with astroid 2.2.5 but works on 2.3.3 +# syntax-error: Cannot import 'string' due to syntax error 'invalid syntax (<unknown>, line 109)' +# Python 3.9 fails with astroid 2.2.5 but works on 2.3.3 +# syntax-error: Cannot import 'string' due to syntax error 'invalid syntax (<unknown>, line 104)' +import string + +# Python 3.9 fails with pylint 2.3.1 or 2.4.4 with astroid 2.3.3 but works with pylint 2.5.0 and astroid 2.4.0 +# 'Call' object has no attribute 'value' +result = {None: None}[{}.get('something')] + +# pylint 2.3.1 and 2.4.4 report the following error but 2.5.0 and 2.6.0 do not +# blacklisted-name: Black listed name "foo" +# see: https://github.com/PyCQA/pylint/issues/3701 +foo = {}.keys() 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 index e69de29bb2d..079d01612ac 100644 --- 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 @@ -0,0 +1,6 @@ +plugins/filter/check_pylint.py pylint:blacklisted-name +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/requirements/constraints.txt b/test/lib/ansible_test/_data/requirements/constraints.txt index f613ef0e603..e043bc52e99 100644 --- a/test/lib/ansible_test/_data/requirements/constraints.txt +++ b/test/lib/ansible_test/_data/requirements/constraints.txt @@ -52,12 +52,12 @@ antsibull-changelog == 0.7.0 antsibull >= 0.21.0 # freeze pylint and its requirements for consistent test results -astroid == 2.2.5 +astroid == 2.3.3 isort == 4.3.15 -lazy-object-proxy == 1.3.1 +lazy-object-proxy == 1.4.3 mccabe == 0.6.1 pylint == 2.3.1 -typed-ast == 1.4.0 # 1.4.0 is required to compile on Python 3.8 +typed-ast == 1.4.1 wrapt == 1.11.1 # freeze pycodestyle for consistent test results diff --git a/test/lib/ansible_test/_data/requirements/sanity.pylint.txt b/test/lib/ansible_test/_data/requirements/sanity.pylint.txt index 1b800bd0607..438ca51dad4 100644 --- a/test/lib/ansible_test/_data/requirements/sanity.pylint.txt +++ b/test/lib/ansible_test/_data/requirements/sanity.pylint.txt @@ -1,3 +1,3 @@ -pylint ; python_version < '3.9' # installation fails on python 3.9.0b1 +pylint pyyaml # needed for collection_detail.py mccabe # pylint complexity testing 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'), - - '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), - - '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), + 'tempfile.mktemp': UnwantedEntry('tempfile.mkstemp'), + + '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': 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/lib/ansible_test/_internal/sanity/pylint.py b/test/lib/ansible_test/_internal/sanity/pylint.py index 769a171728a..324e587346b 100644 --- a/test/lib/ansible_test/_internal/sanity/pylint.py +++ b/test/lib/ansible_test/_internal/sanity/pylint.py @@ -64,6 +64,14 @@ class PylintTest(SanitySingleVersion): """Error code for ansible-test matching the format used by the underlying test program, or None if the program does not use error codes.""" return 'ansible-test' + @property + def supported_python_versions(self): # type: () -> t.Optional[t.Tuple[str, ...]] + """A tuple of supported Python versions or None if the test does not depend on specific Python versions.""" + # Python 3.9 is not supported on pylint < 2.5.0. + # Unfortunately pylint 2.5.0 and later include an unfixed regression. + # See: https://github.com/PyCQA/pylint/issues/3701 + return tuple(python_version for python_version in super(PylintTest, self).supported_python_versions if python_version not in ('3.9',)) + def filter_targets(self, targets): # type: (t.List[TestTarget]) -> t.List[TestTarget] """Return the given list of test targets, filtered to include only those relevant for the test.""" return [target for target in targets if os.path.splitext(target.path)[1] == '.py' or is_subdir(target.path, 'bin')] diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index e1b2b007d6c..f1b926e6362 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -211,6 +211,10 @@ test/integration/targets/ansible-runner/files/adhoc_example1.py future-import-bo test/integration/targets/ansible-runner/files/adhoc_example1.py metaclass-boilerplate test/integration/targets/ansible-runner/files/playbook_example1.py future-import-boilerplate test/integration/targets/ansible-runner/files/playbook_example1.py metaclass-boilerplate +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/filter/check_pylint.py pylint:blacklisted-name 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