From 064e8e1ef48a1833d98c67767740c981ef126455 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Tue, 10 Sep 2019 00:29:34 -0700 Subject: [PATCH] Fix ansible-doc traceback and sanity test. (#62040) * Fix ansible-doc traceback for removed modules. This avoids tracebacks with errors like the following when a module has been removed: module module_name missing documentation (or could not parse documentation): 'NoneType' object does not support item assignment * Fix ansible-doc sanity test warning handling. Warnings about removed modules/plugins on stderr are now properly ignored. Previously an ansible-doc error could result in unrelated errors going undetected because tests were stopped early and the underlying error was ignored. --- .../ansible-doc-removed-traceback.yml | 2 + .../fragments/ansible-test-ansible-doc.yml | 2 + lib/ansible/cli/doc.py | 5 ++- .../_internal/sanity/ansible_doc.py | 39 ++----------------- test/sanity/ignore.txt | 4 +- 5 files changed, 14 insertions(+), 38 deletions(-) create mode 100644 changelogs/fragments/ansible-doc-removed-traceback.yml create mode 100644 changelogs/fragments/ansible-test-ansible-doc.yml diff --git a/changelogs/fragments/ansible-doc-removed-traceback.yml b/changelogs/fragments/ansible-doc-removed-traceback.yml new file mode 100644 index 00000000000..01a223aacd7 --- /dev/null +++ b/changelogs/fragments/ansible-doc-removed-traceback.yml @@ -0,0 +1,2 @@ +bugfixes: + - ansible-doc now properly handles removed modules/plugins diff --git a/changelogs/fragments/ansible-test-ansible-doc.yml b/changelogs/fragments/ansible-test-ansible-doc.yml new file mode 100644 index 00000000000..df4e0d48427 --- /dev/null +++ b/changelogs/fragments/ansible-test-ansible-doc.yml @@ -0,0 +1,2 @@ +bugfixes: + - ansible-test now properly handles warnings for removed modules/plugins diff --git a/lib/ansible/cli/doc.py b/lib/ansible/cli/doc.py index af6e21d62e9..f3977fc9582 100644 --- a/lib/ansible/cli/doc.py +++ b/lib/ansible/cli/doc.py @@ -273,7 +273,10 @@ class DocCLI(CLI): if not any(filename.endswith(x) for x in C.BLACKLIST_EXTS): doc, plainexamples, returndocs, metadata = get_docstring(filename, fragment_loader, verbose=(context.CLIARGS['verbosity'] > 0)) - doc['filename'] = filename + + if doc: + # doc may be None, such as when the module has been removed + doc['filename'] = filename except Exception as e: display.vvv(traceback.format_exc()) diff --git a/test/lib/ansible_test/_internal/sanity/ansible_doc.py b/test/lib/ansible_test/_internal/sanity/ansible_doc.py index 589e007cefe..13427e8a9d4 100644 --- a/test/lib/ansible_test/_internal/sanity/ansible_doc.py +++ b/test/lib/ansible_test/_internal/sanity/ansible_doc.py @@ -116,14 +116,6 @@ class AnsibleDocTest(SanitySingleVersion): stderr = ex.stderr status = ex.status - if stderr: - errors = stderr.strip().splitlines() - messages = [self.parse_error(e, target_paths) for e in errors] - - if messages and all(messages): - error_messages += messages - continue - if status: summary = u'%s' % SubprocessError(cmd=cmd, status=status, stderr=stderr) return SanityFailure(self.name, summary=summary) @@ -131,6 +123,10 @@ class AnsibleDocTest(SanitySingleVersion): if stdout: display.info(stdout.strip(), verbosity=3) + if stderr: + # ignore removed module/plugin warnings + stderr = re.sub(r'\[WARNING\]: [^ ]+ [^ ]+ has been removed\n', '', stderr).strip() + if stderr: summary = u'Output on stderr from ansible-doc is considered an error.\n\n%s' % SubprocessError(cmd, stderr=stderr) return SanityFailure(self.name, summary=summary) @@ -144,30 +140,3 @@ class AnsibleDocTest(SanitySingleVersion): return SanityFailure(self.name, messages=error_messages) return SanitySuccess(self.name) - - @staticmethod - def parse_error(error, target_paths): - """ - :type error: str - :type target_paths: dict[str, dict[str, str]] - :rtype: SanityMessage | None - """ - # example error messages from lib/ansible/cli/doc.py: - # ERROR! module ping missing documentation (or could not parse documentation): expected string or buffer - # [ERROR]: module ping has a documentation error formatting or is missing documentation. - match = re.search(r'^[^ ]*ERROR[^ ]* (?P[^ ]+) (?P[^ ]+) (?P.*)$', error) - - if match: - groups = match.groupdict() - - error_type = groups['type'] - error_name = groups['name'] - error_text = groups['text'] - - if error_name in target_paths.get(error_type, {}): - return SanityMessage( - message=error_text, - path=target_paths[error_type][error_name], - ) - - return None diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index 0b3466d69b9..56c7dd4a6c4 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -5512,9 +5512,9 @@ lib/ansible/modules/system/user.py validate-modules:use-run-command-not-popen lib/ansible/modules/system/user.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/system/user.py validate-modules:doc-default-incompatible-type lib/ansible/modules/system/xfconf.py validate-modules:parameter-type-not-in-doc -lib/ansible/modules/utilities/helper/_accelerate.py ansible-doc lib/ansible/modules/utilities/logic/async_status.py use-argspec-type-path lib/ansible/modules/utilities/logic/async_status.py validate-modules!skip +lib/ansible/modules/utilities/logic/async_wrapper.py ansible-doc!skip # not an actual module lib/ansible/modules/utilities/logic/async_wrapper.py use-argspec-type-path lib/ansible/modules/utilities/logic/wait_for.py pylint:blacklisted-name lib/ansible/modules/web_infrastructure/ansible_tower/tower_credential.py validate-modules:doc-choices-do-not-match-spec @@ -5710,7 +5710,7 @@ lib/ansible/plugins/action/normal.py action-plugin-docs # default action plugin lib/ansible/plugins/action/nxos.py action-plugin-docs # base class for deprecated network platform modules using `connection: local` lib/ansible/plugins/action/sros.py action-plugin-docs # base class for deprecated network platform modules using `connection: local` lib/ansible/plugins/action/vyos.py action-plugin-docs # base class for deprecated network platform modules using `connection: local` -lib/ansible/plugins/cache/base.py ansible-doc # not a plugin, but a stub for backwards compatibility +lib/ansible/plugins/cache/base.py ansible-doc!skip # not a plugin, but a stub for backwards compatibility lib/ansible/plugins/callback/hipchat.py pylint:blacklisted-name lib/ansible/plugins/connection/lxc.py pylint:blacklisted-name lib/ansible/plugins/doc_fragments/a10.py future-import-boilerplate