From a30c55f68ac3d0a16077e984554c306295fa84cf Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Thu, 29 Apr 2021 15:35:39 -0700 Subject: [PATCH] Remove deprecated `common.removed` module_util. Tests have been updated to reflect its removal as well. --- .../fragments/ansible-test-removed-module.yml | 5 ++ .../dev_guide/testing_validate-modules.rst | 5 +- lib/ansible/module_utils/common/removed.py | 63 ------------- .../_data/sanity/import/importer.py | 10 +-- .../_data/sanity/pylint/plugins/unwanted.py | 2 - .../validate-modules/validate_modules/main.py | 90 ++++--------------- .../units/module_utils/common/test_removed.py | 62 ------------- 7 files changed, 24 insertions(+), 213 deletions(-) create mode 100644 changelogs/fragments/ansible-test-removed-module.yml delete mode 100644 lib/ansible/module_utils/common/removed.py delete mode 100644 test/units/module_utils/common/test_removed.py diff --git a/changelogs/fragments/ansible-test-removed-module.yml b/changelogs/fragments/ansible-test-removed-module.yml new file mode 100644 index 00000000000..a6d9947d928 --- /dev/null +++ b/changelogs/fragments/ansible-test-removed-module.yml @@ -0,0 +1,5 @@ +removed_features: + - The built-in module_util ``ansible.module_utils.common.removed`` was previously deprecated and has been removed. +minor_changes: + - ansible-test - The ``validate-modules`` sanity test codes ``last-line-main-call``, ``missing-if-name-main`` and ``missing-main-call`` have been removed. + - ansible-test - The ``validate-modules`` sanity test codes ``ansible-deprecated-module`` and ``collection-deprecated-module`` have been added. diff --git a/docs/docsite/rst/dev_guide/testing_validate-modules.rst b/docs/docsite/rst/dev_guide/testing_validate-modules.rst index 958c349c2e0..cba5d5d38bb 100644 --- a/docs/docsite/rst/dev_guide/testing_validate-modules.rst +++ b/docs/docsite/rst/dev_guide/testing_validate-modules.rst @@ -61,6 +61,8 @@ Codes ============================================================ ================== ==================== ========================================================================================= **Error Code** **Type** **Level** **Sample Message** ------------------------------------------------------------ ------------------ -------------------- ----------------------------------------------------------------------------------------- + ansible-deprecated-module Documentation Error A module is deprecated and supposed to be removed in the current or an earlier Ansible version + collection-deprecated-module Documentation Error A module is deprecated and supposed to be removed in the current or an earlier collection version ansible-deprecated-version Documentation Error A feature is deprecated and supposed to be removed in the current or an earlier Ansible version ansible-module-not-initialized Syntax Error Execution of the module did not result in initialization of AnsibleModule collection-deprecated-version Documentation Error A feature is deprecated and supposed to be removed in the current or an earlier collection version @@ -94,14 +96,11 @@ Codes invalid-module-schema Documentation Error ``AnsibleModule`` schema validation error invalid-removal-version Documentation Error The version at which a feature is supposed to be removed cannot be parsed (for collections, it must be a semantic version, see https://semver.org/) invalid-requires-extension Naming Error Module ``#AnsibleRequires -CSharpUtil`` should not end in .cs, Module ``#Requires`` should not end in .psm1 - last-line-main-call Syntax Error Call to ``main()`` not the last line (or ``removed_module()`` in the case of deprecated & docs only modules) missing-doc-fragment Documentation Error ``DOCUMENTATION`` fragment missing missing-existing-doc-fragment Documentation Warning Pre-existing ``DOCUMENTATION`` fragment missing missing-documentation Documentation Error No ``DOCUMENTATION`` provided missing-examples Documentation Error No ``EXAMPLES`` provided missing-gplv3-license Documentation Error GPLv3 license header not found - missing-if-name-main Syntax Error Next to last line is not ``if __name__ == "__main__":`` - missing-main-call Syntax Error Did not find a call to ``main()`` (or ``removed_module()`` in the case of deprecated & docs only modules) missing-module-utils-basic-import Imports Warning Did not find ``ansible.module_utils.basic`` import missing-module-utils-import-csharp-requirements Imports Error No ``Ansible.ModuleUtils`` or C# Ansible util requirements/imports found missing-powershell-interpreter Syntax Error Interpreter line is not ``#!powershell`` diff --git a/lib/ansible/module_utils/common/removed.py b/lib/ansible/module_utils/common/removed.py deleted file mode 100644 index 45725b0f830..00000000000 --- a/lib/ansible/module_utils/common/removed.py +++ /dev/null @@ -1,63 +0,0 @@ -# Copyright (c) 2018, Ansible Project -# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause) - -# Make coding more python3-ish -from __future__ import (absolute_import, division, print_function) -__metaclass__ = type - -import json -import sys - -from ansible.module_utils._text import to_native - - -def removed_module(removed_in, msg='This module has been removed. The module documentation for' - ' Ansible-%(version)s may contain hints for porting'): - """ - This function is deprecated and should not be used. Instead the module should just be - actually removed. This function is scheduled for removal for the 2.12 release. - - Returns module failure along with a message about the module being removed - - :arg removed_in: The version that the module was removed in - :kwarg msg: Message to use in the module's failure message. The default says that the module - has been removed and what version of the Ansible documentation to search for porting help. - - Remove the actual code and instead have boilerplate like this:: - - from ansible.module_utils.common.removed import removed_module - - if __name__ == '__main__': - removed_module("2.4") - """ - results = { - 'failed': True, - 'deprecations': [ - { - 'msg': 'The removed_module function is deprecated', - 'version': '2.12', - }, - ] - } - - # Convert numbers into strings - removed_in = to_native(removed_in) - - version = removed_in.split('.') - try: - numeric_minor = int(version[-1]) - except Exception: - last_version = None - else: - version = version[:-1] - version.append(to_native(numeric_minor - 1)) - last_version = '.'.join(version) - - if last_version is None: - results['warnings'] = ['removed modules should specify the version they were removed in'] - results['msg'] = 'This module has been removed' - else: - results['msg'] = msg % {'version': last_version} - - print('\n{0}\n'.format(json.dumps(results))) - sys.exit(1) diff --git a/test/lib/ansible_test/_data/sanity/import/importer.py b/test/lib/ansible_test/_data/sanity/import/importer.py index 0757b677ddc..f8be8e1363b 100755 --- a/test/lib/ansible_test/_data/sanity/import/importer.py +++ b/test/lib/ansible_test/_data/sanity/import/importer.py @@ -144,7 +144,7 @@ def main(): if not self.restrict_to_module_paths: return None # for non-modules, everything in the ansible namespace is allowed - if fullname in ('ansible.module_utils.basic', 'ansible.module_utils.common.removed'): + if fullname in ('ansible.module_utils.basic',): return self # intercept loading so we can modify the result if is_name_in_namepace(fullname, ['ansible.module_utils', self.name]): @@ -187,14 +187,6 @@ def main(): return module - if fullname == 'ansible.module_utils.common.removed': - module = self.__load_module(fullname) - - # no-op for removed_module since it is called in place of AnsibleModule instantiation - module.removed_module = lambda *args, **kwargs: None - - return module - raise ImportError('import of "%s" is not allowed in this context' % fullname) def __load_module(self, fullname): diff --git a/test/lib/ansible_test/_data/sanity/pylint/plugins/unwanted.py b/test/lib/ansible_test/_data/sanity/pylint/plugins/unwanted.py index 7012feaa585..5e818c26b34 100644 --- a/test/lib/ansible_test/_data/sanity/pylint/plugins/unwanted.py +++ b/test/lib/ansible_test/_data/sanity/pylint/plugins/unwanted.py @@ -126,14 +126,12 @@ class AnsibleUnwantedChecker(BaseChecker): 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), } diff --git a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py index 865a05681ab..6ec22755fb4 100644 --- a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py +++ b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py @@ -607,74 +607,6 @@ class ModuleValidator(Validator): return min(linenos) - def _find_main_call(self, look_for="main"): - """ Ensure that the module ends with: - if __name__ == '__main__': - main() - OR, in the case of modules that are in the docs-only deprecation phase - if __name__ == '__main__': - removed_module() - """ - lineno = False - if_bodies = [] - for child in self.ast.body: - if isinstance(child, ast.If): - try: - if child.test.left.id == '__name__': - if_bodies.extend(child.body) - except AttributeError: - pass - - bodies = self.ast.body - bodies.extend(if_bodies) - - for child in bodies: - - # validate that the next to last line is 'if __name__ == "__main__"' - if child.lineno == (self.length - 1): - - mainchecked = False - try: - if isinstance(child, ast.If) and \ - child.test.left.id == '__name__' and \ - len(child.test.ops) == 1 and \ - isinstance(child.test.ops[0], ast.Eq) and \ - child.test.comparators[0].s == '__main__': - mainchecked = True - except Exception: - pass - - if not mainchecked: - self.reporter.error( - path=self.object_path, - code='missing-if-name-main', - msg='Next to last line should be: if __name__ == "__main__":', - line=child.lineno - ) - - # validate that the final line is a call to main() - if isinstance(child, ast.Expr): - if isinstance(child.value, ast.Call): - if (isinstance(child.value.func, ast.Name) and - child.value.func.id == look_for): - lineno = child.lineno - if lineno < self.length - 1: - self.reporter.error( - path=self.object_path, - code='last-line-main-call', - msg=('Call to %s() not the last line' % look_for), - line=lineno - ) - - if not lineno: - self.reporter.error( - path=self.object_path, - code='missing-main-call', - msg=('Did not find a call to %s()' % look_for) - ) - - return lineno or 0 - def _find_has_import(self): for child in self.ast.body: found_try_except_import = False @@ -2209,10 +2141,26 @@ class ModuleValidator(Validator): strict_ansible_version = self._create_strict_version( '.'.join(ansible_version.split('.')[:2]), self.collection_name) end_of_deprecation_should_be_removed_only = strict_ansible_version >= removed_in + + if end_of_deprecation_should_be_removed_only: + self.reporter.error( + path=self.object_path, + code='ansible-deprecated-module', + msg='Module is marked for removal in version %s of Ansible when the current version is %s' % ( + version, ansible_version), + ) elif self.collection_version: strict_ansible_version = self.collection_version end_of_deprecation_should_be_removed_only = strict_ansible_version >= removed_in + if end_of_deprecation_should_be_removed_only: + self.reporter.error( + path=self.object_path, + code='collection-deprecated-module', + msg='Module is marked for removal in version %s of this collection when the current version is %s' % ( + version, self.collection_version_str), + ) + # handle deprecation by date if 'removed_at_date' in docs['deprecated']: try: @@ -2255,12 +2203,6 @@ class ModuleValidator(Validator): self._check_type_instead_of_isinstance( powershell=self._powershell_module() ) - if end_of_deprecation_should_be_removed_only: - # Ensure that `if __name__ == '__main__':` calls `removed_module()` which ensure that the module has no code in - main = self._find_main_call('removed_module') - # FIXME: Ensure that the version in the call to removed_module is less than +2. - # Otherwise it's time to remove the file (This may need to be done in another test to - # avoid breaking whenever the Ansible version bumps) class PythonPackageValidator(Validator): diff --git a/test/units/module_utils/common/test_removed.py b/test/units/module_utils/common/test_removed.py deleted file mode 100644 index 36c1c1e94c2..00000000000 --- a/test/units/module_utils/common/test_removed.py +++ /dev/null @@ -1,62 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright 2019, Andrew Klychkov @Andersson007 -# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause) - -from __future__ import absolute_import, division, print_function -__metaclass__ = type - -import pytest - -from ansible.module_utils.common.removed import removed_module - - -@pytest.mark.parametrize('input_data', [u'2.8', 2.8, 2, '', ]) -def test_removed_module_sys_exit(input_data): - """Test for removed_module function, sys.exit().""" - - with pytest.raises(SystemExit) as wrapped_e: - removed_module(input_data) - - assert wrapped_e.type == SystemExit - assert wrapped_e.value.code == 1 - - -@pytest.mark.parametrize( - 'input_data, expected_msg, expected_warn', - [ - ( - u'2.8', - u'This module has been removed. ' - 'The module documentation for Ansible-2.7 may contain hints for porting', - u'', - ), - ( - 2.8, - u'This module has been removed. ' - 'The module documentation for Ansible-2.7 may contain hints for porting', - u'', - ), - ( - 2, - u'This module has been removed. ' - 'The module documentation for Ansible-1 may contain hints for porting', - u'', - ), - ( - u'café', - u'This module has been removed', - u'"warnings": ["removed modules should specify the version they were removed in"]', - ), - ( - 0.1, - u'This module has been removed. ' - 'The module documentation for Ansible-0.0 may contain hints for porting', - u'', - ), - ] -) -def test_removed_module_msgs(input_data, expected_msg, expected_warn, capsys): - """Test for removed_module function, content of output messages.""" - - captured = capsys.readouterr() - assert expected_msg, expected_warn in captured.out