From a3b786309869fafea08adc7d332d85e03631f321 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Tue, 24 Jan 2023 13:25:32 -0800 Subject: [PATCH] validate-modules - Remove `__future__` limits (#79800) * validate-modules - Remove `__future__` limits Limits on specific `__future__` imports are handled by other sanity tests. * Add integration test for module/plugin imports. --- ...le-test-validate-modules-future-import.yml | 7 ++++ .../testing/sanity/validate-modules.rst | 1 - .../col/plugins/lookup/import_order_lookup.py | 16 +++++++++ .../ns/col/plugins/modules/import_order.py | 24 +++++++++++++ .../expected.txt | 2 ++ .../validate-modules/validate_modules/main.py | 35 ++++++------------- 6 files changed, 60 insertions(+), 25 deletions(-) create mode 100644 changelogs/fragments/ansible-test-validate-modules-future-import.yml create mode 100644 test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/lookup/import_order_lookup.py create mode 100644 test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/import_order.py diff --git a/changelogs/fragments/ansible-test-validate-modules-future-import.yml b/changelogs/fragments/ansible-test-validate-modules-future-import.yml new file mode 100644 index 00000000000..64c31512515 --- /dev/null +++ b/changelogs/fragments/ansible-test-validate-modules-future-import.yml @@ -0,0 +1,7 @@ +minor_changes: + - ansible-test - The ``validate-modules`` sanity test no longer limits the ``__future__`` imports that can be used. + Other sanity tests that check ``__future__`` imports remain unchanged. As a result, the error code + ``illegal-future-imports`` is no longer used. +bugfixes: + - ansible-test - The ``validate-modules`` sanity test now properly enforces documentation before imports for plugins. + Previously this was only enforced for modules due to a coding error. diff --git a/docs/docsite/rst/dev_guide/testing/sanity/validate-modules.rst b/docs/docsite/rst/dev_guide/testing/sanity/validate-modules.rst index dcb78c05270..e8a83b119ac 100644 --- a/docs/docsite/rst/dev_guide/testing/sanity/validate-modules.rst +++ b/docs/docsite/rst/dev_guide/testing/sanity/validate-modules.rst @@ -54,7 +54,6 @@ Codes doc-type-does-not-match-spec Documentation Error Argument_spec defines type different than documentation does documentation-error Documentation Error Unknown ``DOCUMENTATION`` error documentation-syntax-error Documentation Error Invalid ``DOCUMENTATION`` schema - illegal-future-imports Imports Error Only the following ``from __future__`` imports are allowed: ``absolute_import``, ``division``, and ``print_function``. import-before-documentation Imports Error Import found before documentation variables. All imports must appear below ``DOCUMENTATION``/``EXAMPLES``/``RETURN`` import-error Documentation Error ``Exception`` attempting to import module for ``argument_spec`` introspection import-placement Locations Warning Imports should be directly below ``DOCUMENTATION``/``EXAMPLES``/``RETURN`` diff --git a/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/lookup/import_order_lookup.py b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/lookup/import_order_lookup.py new file mode 100644 index 00000000000..5a1f0ece09f --- /dev/null +++ b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/lookup/import_order_lookup.py @@ -0,0 +1,16 @@ +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import annotations + +from ansible.plugins.lookup import LookupBase + +DOCUMENTATION = """ +name: import_order_lookup +short_description: Import order lookup +description: Import order lookup. +""" + + +class LookupModule(LookupBase): + def run(self, terms, variables=None, **kwargs): + return [] diff --git a/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/import_order.py b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/import_order.py new file mode 100644 index 00000000000..f4f3c9b81d8 --- /dev/null +++ b/test/integration/targets/ansible-test-sanity-validate-modules/ansible_collections/ns/col/plugins/modules/import_order.py @@ -0,0 +1,24 @@ +#!/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 + +from ansible.module_utils.basic import AnsibleModule + +DOCUMENTATION = ''' +module: import_order +short_description: Import order test module +description: Import order test module. +author: + - Ansible Core Team +''' + +EXAMPLES = '''#''' +RETURN = '''''' + + +if __name__ == '__main__': + module = AnsibleModule(argument_spec=dict()) + module.exit_json() diff --git a/test/integration/targets/ansible-test-sanity-validate-modules/expected.txt b/test/integration/targets/ansible-test-sanity-validate-modules/expected.txt index 95f12f390ad..c4a57c5e883 100644 --- a/test/integration/targets/ansible-test-sanity-validate-modules/expected.txt +++ b/test/integration/targets/ansible-test-sanity-validate-modules/expected.txt @@ -1,3 +1,5 @@ +plugins/lookup/import_order_lookup.py:5:0: import-before-documentation: Import found before documentation variables. All imports must appear below DOCUMENTATION/EXAMPLES/RETURN. +plugins/modules/import_order.py:8:0: import-before-documentation: Import found before documentation variables. All imports must appear below DOCUMENTATION/EXAMPLES/RETURN. plugins/modules/invalid_yaml_syntax.py:0:0: deprecation-mismatch: "meta/runtime.yml" and DOCUMENTATION.deprecation do not agree. plugins/modules/invalid_yaml_syntax.py:0:0: missing-documentation: No DOCUMENTATION provided plugins/modules/invalid_yaml_syntax.py:8:15: documentation-syntax-error: DOCUMENTATION is not valid YAML diff --git a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py index 77c380c7aad..cedf0acb2fd 100644 --- a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py +++ b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py @@ -299,8 +299,6 @@ class ModuleValidator(Validator): # win_dsc is a dynamic arg spec, the docs won't ever match PS_ARG_VALIDATE_REJECTLIST = frozenset(('win_dsc.ps1', )) - ACCEPTLIST_FUTURE_IMPORTS = frozenset(('absolute_import', 'division', 'print_function')) - def __init__(self, path, analyze_arg_spec=False, collection=None, collection_version=None, base_branch=None, git_cache=None, reporter=None, routing=None, plugin_type='module'): super(ModuleValidator, self).__init__(reporter=reporter or Reporter()) @@ -414,13 +412,10 @@ class ModuleValidator(Validator): if isinstance(child, ast.Expr) and isinstance(child.value, ast.Constant) and isinstance(child.value.value, str): continue - # allowed from __future__ imports + # allow __future__ imports (the specific allowed imports are checked by other sanity tests) if isinstance(child, ast.ImportFrom) and child.module == '__future__': - for future_import in child.names: - if future_import.name not in self.ACCEPTLIST_FUTURE_IMPORTS: - break - else: - continue + continue + return False return True except AttributeError: @@ -676,29 +671,21 @@ class ModuleValidator(Validator): ) def _ensure_imports_below_docs(self, doc_info, first_callable): - min_doc_line = min(doc_info[key]['lineno'] for key in doc_info) + doc_line_numbers = [lineno for lineno in (doc_info[key]['lineno'] for key in doc_info) if lineno > 0] + + min_doc_line = min(doc_line_numbers) if doc_line_numbers else None max_doc_line = max(doc_info[key]['end_lineno'] for key in doc_info) import_lines = [] for child in self.ast.body: if isinstance(child, (ast.Import, ast.ImportFrom)): + # allow __future__ imports (the specific allowed imports are checked by other sanity tests) if isinstance(child, ast.ImportFrom) and child.module == '__future__': - # allowed from __future__ imports - for future_import in child.names: - if future_import.name not in self.ACCEPTLIST_FUTURE_IMPORTS: - self.reporter.error( - path=self.object_path, - code='illegal-future-imports', - msg=('Only the following from __future__ imports are allowed: %s' - % ', '.join(self.ACCEPTLIST_FUTURE_IMPORTS)), - line=child.lineno - ) - break - else: # for-else. If we didn't find a problem nad break out of the loop, then this is a legal import - continue + continue + import_lines.append(child.lineno) - if child.lineno < min_doc_line: + if min_doc_line and child.lineno < min_doc_line: self.reporter.error( path=self.object_path, code='import-before-documentation', @@ -715,7 +702,7 @@ class ModuleValidator(Validator): for grandchild in bodies: if isinstance(grandchild, (ast.Import, ast.ImportFrom)): import_lines.append(grandchild.lineno) - if grandchild.lineno < min_doc_line: + if min_doc_line and grandchild.lineno < min_doc_line: self.reporter.error( path=self.object_path, code='import-before-documentation',