Update ansible-test pylint Python support. (#72997)

* Rename pylint plugin and add tests. (#70225)
* Update ansible-test pylint Python support. (#72972)
* Add integration tests for sanity test failures.
(cherry picked from commit fa48678a08)

* Python 3.8 is now officially supported.
* Python 3.9 is now skipped with a warning.
(cherry picked from commit 37d09f2488)

* Allow key None to prevent errors with import test.
(cherry picked from commit dbc2c996ab)

Backport of https://github.com/ansible/ansible/pull/73003

Co-authored-by: Felix Fontein <felix@fontein.de>

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.

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.

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()

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()

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

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

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 "${@}")

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

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

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:
+class UnwantedEntry:
-    """Defines a import blacklist entry."""
+    """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):
+class AnsibleUnwantedChecker(BaseChecker):
-    """Checker for blacklisted imports and functions."""
+    """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),
+        # boto=UnwantedEntry('boto3', modules_only=True),
-        # requests=BlacklistEntry('ansible.module_utils.urls', modules_only=True),
+        # requests=UnwantedEntry('ansible.module_utils.urls', modules_only=True),
-        # urllib=BlacklistEntry('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',
+        urllib2=UnwantedEntry('ansible.module_utils.urls',
-                               ignore_paths=(
+                              ignore_paths=(
-                                   '/lib/ansible/module_utils/urls.py',
+                                  '/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',
+        collections=UnwantedEntry('ansible.module_utils.common._collections_compat',
-                                   ignore_paths=(
+                                  ignore_paths=(
-                                       '/lib/ansible/module_utils/common/_collections_compat.py',
+                                      '/lib/ansible/module_utils/common/_collections_compat.py',
-                                   ),
+                                  ),
-                                   names=(
+                                  names=(
-                                       'MappingView',
+                                      'MappingView',
-                                       'ItemsView',
+                                      'ItemsView',
-                                       'KeysView',
+                                      'KeysView',
-                                       'ValuesView',
+                                      'ValuesView',
-                                       'Mapping', 'MutableMapping',
+                                      'Mapping', 'MutableMapping',
-                                       'Sequence', 'MutableSequence',
+                                      'Sequence', 'MutableSequence',
-                                       'Set', 'MutableSet',
+                                      'Set', 'MutableSet',
-                                       'Container',
+                                      'Container',
-                                       'Hashable',
+                                      'Hashable',
-                                       'Sized',
+                                      'Sized',
-                                       'Callable',
+                                      'Callable',
-                                       'Iterable',
+                                      'Iterable',
-                                       'Iterator',
+                                      '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',
+        'sys.exit': UnwantedEntry('exit_json or fail_json',
-                                   ignore_paths=(
+                                  ignore_paths=(
-                                       '/lib/ansible/module_utils/basic.py',
+                                      '/lib/ansible/module_utils/basic.py',
-                                       '/lib/ansible/modules/async_wrapper.py',
+                                      '/lib/ansible/modules/async_wrapper.py',
-                                       '/lib/ansible/module_utils/common/removed.py',
+                                      '/lib/ansible/module_utils/common/removed.py',
-                                   ),
+                                  ),
-                                   modules_only=True),
+                                  modules_only=True),
-
+
-        'builtins.print': BlacklistEntry('module.log or module.debug',
+        'builtins.print': UnwantedEntry('module.log or module.debug',
-                                         ignore_paths=(
+                                        ignore_paths=(
-                                             '/lib/ansible/module_utils/basic.py',
+                                            '/lib/ansible/module_utils/basic.py',
-                                             '/lib/ansible/module_utils/common/removed.py',
+                                            '/lib/ansible/module_utils/common/removed.py',
-                                         ),
+                                        ),
-                                         modules_only=True),
+                                        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))

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')]

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