From 7e2cc7db12aa95c4d07d24b42cf6c5bb2d8f41e4 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Tue, 28 Jul 2020 17:10:35 +0200 Subject: [PATCH] validate-modules: fix version_added validation for top-level, fix error codes (#70869) * Also validate top-level version_added. * Fix error code. * Produce same version_added validation error in schema than in code (and stop returning it twice). * Return correct error codes for invalid version_added for options and return values. * Add changelog. * Fix forgotten closing braket. * Accept 'historical' for some top-level version_added. --- ...le-test-validate-modules-version-added.yml | 3 +++ .../dev_guide/testing_validate-modules.rst | 3 ++- .../validate-modules/validate_modules/main.py | 26 ++++++++----------- .../validate_modules/schema.py | 24 +++++++++++------ 4 files changed, 32 insertions(+), 24 deletions(-) create mode 100644 changelogs/fragments/70869-ansible-test-validate-modules-version-added.yml diff --git a/changelogs/fragments/70869-ansible-test-validate-modules-version-added.yml b/changelogs/fragments/70869-ansible-test-validate-modules-version-added.yml new file mode 100644 index 00000000000..aa5c30072d3 --- /dev/null +++ b/changelogs/fragments/70869-ansible-test-validate-modules-version-added.yml @@ -0,0 +1,3 @@ +bugfixes: +- "ansible-test validate-modules - return correct error codes ``option-invalid-version-added`` resp. ``return-invalid-version-added`` instead of the wrong error ``deprecation-either-date-or-version`` when an invalid value of ``version_added`` is specified for an option or a return value (https://github.com/ansible/ansible/pull/70869)." +- "ansible-test validate-modules - ``version_added`` on module level was not validated for modules in collections (https://github.com/ansible/ansible/pull/70869)." diff --git a/docs/docsite/rst/dev_guide/testing_validate-modules.rst b/docs/docsite/rst/dev_guide/testing_validate-modules.rst index a876d7df2f4..5eb3bd0e2aa 100644 --- a/docs/docsite/rst/dev_guide/testing_validate-modules.rst +++ b/docs/docsite/rst/dev_guide/testing_validate-modules.rst @@ -120,7 +120,7 @@ Codes no-default-for-required-parameter Documentation Error Option is marked as required but specifies a default. Arguments with a default should not be marked as required nonexistent-parameter-documented Documentation Error Argument is listed in DOCUMENTATION.options, but not accepted by the module option-incorrect-version-added Documentation Error ``version_added`` for new option is incorrect - option-invalid-version-added Documentation Error ``version_added`` for new option is not a valid version number + option-invalid-version-added Documentation Error ``version_added`` for option is not a valid version number parameter-invalid Documentation Error Argument in argument_spec is not a valid python identifier parameter-invalid-elements Documentation Error Value for "elements" is valid only when value of "type" is ``list`` implied-parameter-type-mismatch Documentation Error Argument_spec implies ``type="str"`` but documentation defines it as different data type @@ -132,6 +132,7 @@ Codes parameter-state-invalid-choice Parameters Error Argument ``state`` includes ``get``, ``list`` or ``info`` as a choice. Functionality should be in an ``_info`` or (if further conditions apply) ``_facts`` module. python-syntax-error Syntax Error Python ``SyntaxError`` while parsing module return-syntax-error Documentation Error ``RETURN`` is not valid YAML, ``RETURN`` fragments missing or invalid + return-invalid-version-added Documentation Error ``version_added`` for return value is not a valid version number subdirectory-missing-init Naming Error Ansible module subdirectories must contain an ``__init__.py`` try-except-missing-has Imports Warning Try/Except ``HAS_`` expression missing undocumented-parameter Documentation Error Argument is listed in the argument_spec, but not documented in the module 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 00dcbf89a1a..3b61f1f329b 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 @@ -1160,16 +1160,18 @@ class ModuleValidator(Validator): try: collection_name = doc.get('version_added_collection') version_added = self._create_strict_version( - str(doc.get('version_added', '0.0') or '0.0'), + str(version_added_raw or '0.0'), collection_name=collection_name) except ValueError as e: - version_added = doc.get('version_added', '0.0') - if version_added != 'historical' or self._is_new_module(): - self.reporter.error( - path=self.object_path, - code='module-invalid-version-added', - msg='version_added is not a valid version number: %r. Error: %s' % (version_added, e) - ) + version_added = version_added_raw or '0.0' + if self._is_new_module() or version_added != 'historical': + # already reported during schema validation, except: + if version_added == 'historical': + self.reporter.error( + path=self.object_path, + code='module-invalid-version-added', + msg='version_added is not a valid version number: %r. Error: %s' % (version_added, e) + ) return if existing_doc and str(version_added_raw) != str(existing_doc.get('version_added')): @@ -2076,13 +2078,7 @@ class ModuleValidator(Validator): str(details.get('version_added', '0.0')), collection_name=collection_name) except ValueError as e: - self.reporter.error( - path=self.object_path, - code='option-invalid-version-added', - msg=('version_added for option (%s) is not a valid ' - 'version for %s. Currently %r. Error: %s' % - (option, collection_name, details.get('version_added', '0.0'), e)) - ) + # already reported during schema validation continue if collection_name != self.collection_name: diff --git a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/schema.py b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/schema.py index 5ca248124cf..42a2ada4afb 100644 --- a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/schema.py +++ b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/schema.py @@ -9,6 +9,7 @@ __metaclass__ = type import re from distutils.version import StrictVersion +from functools import partial from voluptuous import ALLOW_EXTRA, PREVENT_EXTRA, All, Any, Invalid, Length, Required, Schema, Self, ValueInvalid from ansible.module_utils.six import string_types @@ -226,7 +227,7 @@ json_value = Schema(Any( )) -def version_added(v): +def version_added(v, error_code='version-added-invalid', accept_historical=False): if 'version_added' in v: version_added = v.get('version_added') if isinstance(version_added, string_types): @@ -234,6 +235,8 @@ def version_added(v): # - or we have a float and we are in ansible/ansible, in which case we're # also happy. if v.get('version_added_collection') == 'ansible.builtin': + if version_added == 'historical' and accept_historical: + return v try: version = StrictVersion() version.parse(version_added) @@ -241,7 +244,7 @@ def version_added(v): raise _add_ansible_error_code( Invalid('version_added (%r) is not a valid ansible-base version: ' '%s' % (version_added, exc)), - error_code='deprecation-either-date-or-version') + error_code=error_code) else: try: version = SemanticVersion() @@ -251,7 +254,7 @@ def version_added(v): Invalid('version_added (%r) is not a valid collection version ' '(see specification at https://semver.org/): ' '%s' % (version_added, exc)), - error_code='deprecation-either-date-or-version') + error_code=error_code) elif 'version_added_collection' in v: # Must have been manual intervention, since version_added_collection is only # added automatically when version_added is present @@ -304,7 +307,7 @@ def list_dict_option_schema(for_collection): option_version_added = Schema( All({ 'suboptions': Any(None, *[{str_type: Self} for str_type in string_types]), - }, version_added), + }, partial(version_added, error_code='option-invalid-version-added')), extra=ALLOW_EXTRA ) @@ -343,7 +346,7 @@ def return_schema(for_collection): } ), Schema(return_contains), - Schema(version_added), + Schema(partial(version_added, error_code='option-invalid-version-added')), ), Schema(type(None)), ) @@ -371,7 +374,7 @@ def return_schema(for_collection): } ), Schema({any_string_types: return_contains}), - Schema({any_string_types: version_added}), + Schema({any_string_types: partial(version_added, error_code='option-invalid-version-added')}), ), Schema(type(None)), ) @@ -460,8 +463,13 @@ def doc_schema(module_name, for_collection=False, deprecated_module=False): doc_schema_dict.update(deprecation_required_scheme) return Schema( - doc_schema_dict, - extra=PREVENT_EXTRA + All( + Schema( + doc_schema_dict, + extra=PREVENT_EXTRA + ), + partial(version_added, error_code='module-invalid-version-added', accept_historical=not for_collection), + ) )