summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatt Clay <mclay@redhat.com>2021-01-10 22:46:21 -0800
committerGitHub <noreply@github.com>2021-01-11 00:46:21 -0600
commitcf21e699d49b929ea88a5e21f97c4a7f5cc760e9 (patch)
treed896b9a77ed82c04d784cf068575e2533f5fce6a
parent31ef9dffa1ba4ddd260d63603fecc0c3acf3e4e4 (diff)
downloadansible-cf21e699d49b929ea88a5e21f97c4a7f5cc760e9.tar.gz
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 fa48678a08115cc3cd8ef29777370f8db708d9b1) * Python 3.8 is now officially supported. * Python 3.9 is now skipped with a warning. (cherry picked from commit 37d09f24882c1f03be9900e610d53587cfa6bbd6) * Allow key None to prevent errors with import test. (cherry picked from commit dbc2c996ab361151fce8d1244f67413eb27aa50c) Backport of https://github.com/ansible/ansible/pull/73003 Co-authored-by: Felix Fontein <felix@fontein.de>
-rw-r--r--changelogs/fragments/ansible-test-pylint-plugin-name.yml2
-rw-r--r--changelogs/fragments/ansible-test-pylint-python-3.8-3.9.yml3
-rw-r--r--test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/filter/check_pylint.py21
-rw-r--r--test/integration/targets/ansible-test/ansible_collections/ns/col/plugins/modules/bad.py34
-rw-r--r--test/integration/targets/ansible-test/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py16
-rw-r--r--test/integration/targets/ansible-test/ansible_collections/ns/col/tests/sanity/ignore.txt6
-rwxr-xr-xtest/integration/targets/ansible-test/collection-tests/venv.sh4
-rw-r--r--test/lib/ansible_test/_data/requirements/constraints.txt6
-rw-r--r--test/lib/ansible_test/_data/requirements/sanity.pylint.txt2
-rw-r--r--test/lib/ansible_test/_data/sanity/pylint/plugins/unwanted.py (renamed from test/lib/ansible_test/_data/sanity/pylint/plugins/blacklist.py)108
-rw-r--r--test/lib/ansible_test/_internal/sanity/pylint.py8
-rw-r--r--test/sanity/ignore.txt4
12 files changed, 156 insertions, 58 deletions
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 0000000000..31239b5ceb
--- /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 0000000000..9668f7aa8b
--- /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 0000000000..359fbf071c
--- /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 (&lt;unknown&gt;, 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 (&lt;unknown&gt;, 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 0000000000..e79613bbbe
--- /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 0000000000..82215438fb
--- /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 e69de29bb2..079d01612a 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 6ff496b6a1..862c8ad989 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 f613ef0e60..e043bc52e9 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 1b800bd060..438ca51dad 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
index ac53aedaa6..7012feaa58 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 769a171728..324e587346 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 e1b2b007d6..f1b926e636 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