From 3b86dc3e12e334707251111b4188244db95a81ea Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Tue, 17 Sep 2019 15:45:30 -0700 Subject: [PATCH] WIP - Fix ansible-doc bugs and add integration tests. (#62461) * Add integration tests for ansible-doc. * Enable tests that now pass * Cleanup processing of plugin docs * Mostly separate the steps of processing plugin docs 1) Acquire source data 2) Transform and calculate additonal data 3) Format data for output 4) Output data format_plugin_doc() is still mixing transformation and formatting but that should be fixed in a devel-only change * Raise exceptions in _get_plugin_doc() on errors. * Remove check to exclude on blacklisted extensions. We already request only .py files * If there is no DOCUMENTATION entry in the plugin, raise an exception from _get_plugin_doc(). Everywhere we use _get_plugin_doc(), this is treated as an error * If there is no ANSIBLE_METADATA raise an exception as well as displaying of docs assumes that this has been set. * If there is neither DOCUMENTATION nor ANSIBLE_METADATA, warn about the lack of METADATA and error on the lack of DOCUMENTATION. Lack of DOCUMENTATION is more important so it is what the user should see. * Add a few special cases for backwards compat. These should probably be made errors in 2.10: * no docs but has metadata shows no documentation rather than an error * empty plugin file shows no doumentation rather than an error * Simplify backwards compatibility logic. --- lib/ansible/cli/doc.py | 146 +++++++++++------- test/integration/targets/ansible-doc/aliases | 1 + .../integration/targets/ansible-doc/inventory | 1 + .../targets/ansible-doc/library/test_docs.py | 39 +++++ .../library/test_docs_no_metadata.py | 35 +++++ .../library/test_docs_no_status.py | 38 +++++ .../library/test_docs_non_iterable_status.py | 39 +++++ .../library/test_docs_removed_status.py | 39 +++++ .../targets/ansible-doc/library/test_empty.py | 0 .../ansible-doc/library/test_no_docs.py | 23 +++ .../library/test_no_docs_no_metadata.py | 18 +++ .../library/test_no_docs_no_status.py | 22 +++ .../test_no_docs_non_iterable_status.py | 23 +++ .../library/test_no_docs_removed_status.py | 15 ++ test/integration/targets/ansible-doc/runme.sh | 4 + test/integration/targets/ansible-doc/test.yml | 105 +++++++++++++ 16 files changed, 491 insertions(+), 57 deletions(-) create mode 100644 test/integration/targets/ansible-doc/aliases create mode 100644 test/integration/targets/ansible-doc/inventory create mode 100644 test/integration/targets/ansible-doc/library/test_docs.py create mode 100644 test/integration/targets/ansible-doc/library/test_docs_no_metadata.py create mode 100644 test/integration/targets/ansible-doc/library/test_docs_no_status.py create mode 100644 test/integration/targets/ansible-doc/library/test_docs_non_iterable_status.py create mode 100644 test/integration/targets/ansible-doc/library/test_docs_removed_status.py create mode 100644 test/integration/targets/ansible-doc/library/test_empty.py create mode 100644 test/integration/targets/ansible-doc/library/test_no_docs.py create mode 100644 test/integration/targets/ansible-doc/library/test_no_docs_no_metadata.py create mode 100644 test/integration/targets/ansible-doc/library/test_no_docs_no_status.py create mode 100644 test/integration/targets/ansible-doc/library/test_no_docs_non_iterable_status.py create mode 100644 test/integration/targets/ansible-doc/library/test_no_docs_removed_status.py create mode 100755 test/integration/targets/ansible-doc/runme.sh create mode 100644 test/integration/targets/ansible-doc/test.yml diff --git a/lib/ansible/cli/doc.py b/lib/ansible/cli/doc.py index f3977fc9582..f7878ff4e92 100644 --- a/lib/ansible/cli/doc.py +++ b/lib/ansible/cli/doc.py @@ -20,7 +20,7 @@ from ansible.cli import CLI from ansible.cli.arguments import option_helpers as opt_help from ansible.errors import AnsibleError, AnsibleOptionsError from ansible.module_utils._text import to_native -from ansible.module_utils.common._collections_compat import Sequence +from ansible.module_utils.common._collections_compat import Container, Sequence from ansible.module_utils.six import string_types from ansible.parsing.metadata import extract_metadata from ansible.parsing.plugin_docs import read_docstub @@ -36,6 +36,14 @@ def jdump(text): display.display(json.dumps(text, sort_keys=True, indent=4)) +class RemovedPlugin(Exception): + pass + + +class PluginNotFound(Exception): + pass + + class DocCLI(CLI): ''' displays information on modules installed in Ansible libraries. It displays a terse listing of plugins and their short descriptions, @@ -180,28 +188,52 @@ class DocCLI(CLI): if len(context.CLIARGS['args']) == 0: raise AnsibleOptionsError("Incorrect options passed") - # process command line list + # get the docs for plugins in the command line list + plugin_docs = {} + for plugin in context.CLIARGS['args']: + try: + doc, plainexamples, returndocs, metadata = DocCLI._get_plugin_doc(plugin, loader, search_paths) + except PluginNotFound: + display.warning("%s %s not found in:\n%s\n" % (plugin_type, plugin, search_paths)) + continue + except RemovedPlugin: + display.warning("%s %s has been removed\n" % (plugin_type, plugin)) + continue + except Exception as e: + display.vvv(traceback.format_exc()) + raise AnsibleError("%s %s missing documentation (or could not parse" + " documentation): %s\n" % + (plugin_type, plugin, to_native(e))) + + if not doc: + # The doc section existed but was empty + continue + + plugin_docs[plugin] = {'doc': doc, 'examples': plainexamples, + 'return': returndocs, 'metadata': metadata} + if do_json: - dump = {} - for plugin in context.CLIARGS['args']: - doc, plainexamples, returndocs, metadata = DocCLI._get_plugin_doc(plugin, loader, plugin_type, search_paths) + # Some changes to how json docs are formatted + for plugin, doc_data in plugin_docs.items(): try: - returndocs = yaml.load(returndocs) + doc_data['return'] = yaml.load(doc_data['return']) except Exception: pass - if doc: - dump[plugin] = {'doc': doc, 'examples': plainexamples, 'return': returndocs, 'metadata': metadata} - jdump(dump) - else: - text = '' - for plugin in context.CLIARGS['args']: - textret = DocCLI.format_plugin_doc(plugin, loader, plugin_type, search_paths) + jdump(plugin_docs) + + else: + # Some changes to how plain text docs are formatted + text = [] + for plugin, doc_data in plugin_docs.items(): + textret = DocCLI.format_plugin_doc(plugin, plugin_type, + doc_data['doc'], doc_data['examples'], + doc_data['return'], doc_data['metadata']) if textret: - text += textret + text.append(textret) if text: - DocCLI.pager(text) + DocCLI.pager(''.join(text)) return 0 @@ -261,58 +293,58 @@ class DocCLI(CLI): return clean_ns @staticmethod - def _get_plugin_doc(plugin, loader, plugin_type, search_paths): + def _get_plugin_doc(plugin, loader, search_paths): + # if the plugin lives in a non-python file (eg, win_X.ps1), require the corresponding python file for docs + filename = loader.find_plugin(plugin, mod_type='.py', ignore_deprecated=True, check_aliases=True) + if filename is None: + raise PluginNotFound('%s was not found in %s' % (plugin, search_paths)) - doc = plainexamples = returndocs = metadata = {} - try: - # if the plugin lives in a non-python file (eg, win_X.ps1), require the corresponding python file for docs - filename = loader.find_plugin(plugin, mod_type='.py', ignore_deprecated=True, check_aliases=True) - if filename is None: - display.warning("%s %s not found in:\n%s\n" % (plugin_type, plugin, search_paths)) - return + doc, plainexamples, returndocs, metadata = get_docstring(filename, fragment_loader, verbose=(context.CLIARGS['verbosity'] > 0)) - 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)) + # If the plugin existed but did not have a DOCUMENTATION element and was not removed, it's + # an error + if doc is None: + # doc may be None when the module has been removed. Calling code may choose to + # handle that but we can't. + if 'status' in metadata and isinstance(metadata['status'], Container): + if 'removed' in metadata['status']: + raise RemovedPlugin('%s has been removed' % plugin) - if doc: - # doc may be None, such as when the module has been removed - doc['filename'] = filename + # Backwards compat: no documentation but valid metadata (or no metadata, which results in using the default metadata). + # Probably should make this an error in 2.10 + return {}, {}, {}, metadata + else: + # If metadata is invalid, warn but don't error + display.warning(u'%s has an invalid ANSIBLE_METADATA field' % plugin) - except Exception as e: - display.vvv(traceback.format_exc()) - raise AnsibleError("%s %s missing documentation (or could not parse documentation): %s\n" % (plugin_type, plugin, to_native(e))) + raise ValueError('%s did not contain a DOCUMENTATION attribute' % plugin) + + doc['filename'] = filename return doc, plainexamples, returndocs, metadata @staticmethod - def format_plugin_doc(plugin, loader, plugin_type, search_paths): - text = '' + def format_plugin_doc(plugin, plugin_type, doc, plainexamples, returndocs, metadata): + # assign from other sections + doc['plainexamples'] = plainexamples + doc['returndocs'] = returndocs + doc['metadata'] = metadata - doc, plainexamples, returndocs, metadata = DocCLI._get_plugin_doc(plugin, loader, plugin_type, search_paths) - if doc is not None: - - # assign from other sections - doc['plainexamples'] = plainexamples - doc['returndocs'] = returndocs - doc['metadata'] = metadata - - # generate extra data - if plugin_type == 'module': - # is there corresponding action plugin? - if plugin in action_loader: - doc['action'] = True - else: - doc['action'] = False - doc['now_date'] = datetime.date.today().strftime('%Y-%m-%d') - if 'docuri' in doc: - doc['docuri'] = doc[plugin_type].replace('_', '-') - - if context.CLIARGS['show_snippet'] and plugin_type == 'module': - text += DocCLI.get_snippet_text(doc) + # generate extra data + if plugin_type == 'module': + # is there corresponding action plugin? + if plugin in action_loader: + doc['action'] = True else: - text += DocCLI.get_man_text(doc) + doc['action'] = False - elif 'removed' in metadata['status']: - display.warning("%s %s has been removed\n" % (plugin_type, plugin)) + doc['now_date'] = datetime.date.today().strftime('%Y-%m-%d') + if 'docuri' in doc: + doc['docuri'] = doc[plugin_type].replace('_', '-') + + if context.CLIARGS['show_snippet'] and plugin_type == 'module': + text = DocCLI.get_snippet_text(doc) + else: + text = DocCLI.get_man_text(doc) return text diff --git a/test/integration/targets/ansible-doc/aliases b/test/integration/targets/ansible-doc/aliases new file mode 100644 index 00000000000..a6dafcf8cd8 --- /dev/null +++ b/test/integration/targets/ansible-doc/aliases @@ -0,0 +1 @@ +shippable/posix/group1 diff --git a/test/integration/targets/ansible-doc/inventory b/test/integration/targets/ansible-doc/inventory new file mode 100644 index 00000000000..ab9b62c8bb3 --- /dev/null +++ b/test/integration/targets/ansible-doc/inventory @@ -0,0 +1 @@ +not_empty # avoid empty empty hosts list warning without defining explicit localhost diff --git a/test/integration/targets/ansible-doc/library/test_docs.py b/test/integration/targets/ansible-doc/library/test_docs.py new file mode 100644 index 00000000000..39ae3728e62 --- /dev/null +++ b/test/integration/targets/ansible-doc/library/test_docs.py @@ -0,0 +1,39 @@ +#!/usr/bin/python +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +ANSIBLE_METADATA = {'metadata_version': '1.1', + 'status': ['stableinterface'], + 'supported_by': 'core'} + +DOCUMENTATION = ''' +--- +module: test_docs +short_description: Test module +description: + - Test module +author: + - Ansible Core Team +''' + +EXAMPLES = ''' +''' + +RETURN = ''' +''' + + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + module = AnsibleModule( + argument_spec=dict(), + ) + + module.exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/library/test_docs_no_metadata.py b/test/integration/targets/ansible-doc/library/test_docs_no_metadata.py new file mode 100644 index 00000000000..4ea86f023e0 --- /dev/null +++ b/test/integration/targets/ansible-doc/library/test_docs_no_metadata.py @@ -0,0 +1,35 @@ +#!/usr/bin/python +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +DOCUMENTATION = ''' +--- +module: test_docs_no_metadata +short_description: Test module +description: + - Test module +author: + - Ansible Core Team +''' + +EXAMPLES = ''' +''' + +RETURN = ''' +''' + + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + module = AnsibleModule( + argument_spec=dict(), + ) + + module.exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/library/test_docs_no_status.py b/test/integration/targets/ansible-doc/library/test_docs_no_status.py new file mode 100644 index 00000000000..1b0db4e9632 --- /dev/null +++ b/test/integration/targets/ansible-doc/library/test_docs_no_status.py @@ -0,0 +1,38 @@ +#!/usr/bin/python +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +ANSIBLE_METADATA = {'metadata_version': '1.1', + 'supported_by': 'core'} + +DOCUMENTATION = ''' +--- +module: test_docs_no_status +short_description: Test module +description: + - Test module +author: + - Ansible Core Team +''' + +EXAMPLES = ''' +''' + +RETURN = ''' +''' + + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + module = AnsibleModule( + argument_spec=dict(), + ) + + module.exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/library/test_docs_non_iterable_status.py b/test/integration/targets/ansible-doc/library/test_docs_non_iterable_status.py new file mode 100644 index 00000000000..63d080f6532 --- /dev/null +++ b/test/integration/targets/ansible-doc/library/test_docs_non_iterable_status.py @@ -0,0 +1,39 @@ +#!/usr/bin/python +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +ANSIBLE_METADATA = {'metadata_version': '1.1', + 'status': 1, + 'supported_by': 'core'} + +DOCUMENTATION = ''' +--- +module: test_docs_non_iterable_status +short_description: Test module +description: + - Test module +author: + - Ansible Core Team +''' + +EXAMPLES = ''' +''' + +RETURN = ''' +''' + + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + module = AnsibleModule( + argument_spec=dict(), + ) + + module.exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/library/test_docs_removed_status.py b/test/integration/targets/ansible-doc/library/test_docs_removed_status.py new file mode 100644 index 00000000000..cb48c1694c5 --- /dev/null +++ b/test/integration/targets/ansible-doc/library/test_docs_removed_status.py @@ -0,0 +1,39 @@ +#!/usr/bin/python +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +ANSIBLE_METADATA = {'metadata_version': '1.1', + 'status': ['removed'], + 'supported_by': 'core'} + +DOCUMENTATION = ''' +--- +module: test_docs_removed_status +short_description: Test module +description: + - Test module +author: + - Ansible Core Team +''' + +EXAMPLES = ''' +''' + +RETURN = ''' +''' + + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + module = AnsibleModule( + argument_spec=dict(), + ) + + module.exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/library/test_empty.py b/test/integration/targets/ansible-doc/library/test_empty.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/integration/targets/ansible-doc/library/test_no_docs.py b/test/integration/targets/ansible-doc/library/test_no_docs.py new file mode 100644 index 00000000000..5503aedb8cd --- /dev/null +++ b/test/integration/targets/ansible-doc/library/test_no_docs.py @@ -0,0 +1,23 @@ +#!/usr/bin/python +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +ANSIBLE_METADATA = {'metadata_version': '1.1', + 'status': ['stableinterface'], + 'supported_by': 'core'} + + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + module = AnsibleModule( + argument_spec=dict(), + ) + + module.exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/library/test_no_docs_no_metadata.py b/test/integration/targets/ansible-doc/library/test_no_docs_no_metadata.py new file mode 100644 index 00000000000..48872684739 --- /dev/null +++ b/test/integration/targets/ansible-doc/library/test_no_docs_no_metadata.py @@ -0,0 +1,18 @@ +#!/usr/bin/python +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + module = AnsibleModule( + argument_spec=dict(), + ) + + module.exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/library/test_no_docs_no_status.py b/test/integration/targets/ansible-doc/library/test_no_docs_no_status.py new file mode 100644 index 00000000000..f90c5c71222 --- /dev/null +++ b/test/integration/targets/ansible-doc/library/test_no_docs_no_status.py @@ -0,0 +1,22 @@ +#!/usr/bin/python +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +ANSIBLE_METADATA = {'metadata_version': '1.1', + 'supported_by': 'core'} + + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + module = AnsibleModule( + argument_spec=dict(), + ) + + module.exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/library/test_no_docs_non_iterable_status.py b/test/integration/targets/ansible-doc/library/test_no_docs_non_iterable_status.py new file mode 100644 index 00000000000..44fbedee71c --- /dev/null +++ b/test/integration/targets/ansible-doc/library/test_no_docs_non_iterable_status.py @@ -0,0 +1,23 @@ +#!/usr/bin/python +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +ANSIBLE_METADATA = {'metadata_version': '1.1', + 'status': 1, + 'supported_by': 'core'} + + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + module = AnsibleModule( + argument_spec=dict(), + ) + + module.exit_json() + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansible-doc/library/test_no_docs_removed_status.py b/test/integration/targets/ansible-doc/library/test_no_docs_removed_status.py new file mode 100644 index 00000000000..0bcacb61d4d --- /dev/null +++ b/test/integration/targets/ansible-doc/library/test_no_docs_removed_status.py @@ -0,0 +1,15 @@ +#!/usr/bin/python +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +ANSIBLE_METADATA = {'metadata_version': '1.1', + 'status': ['removed'], + 'supported_by': 'core'} + + +from ansible.module_utils.common.removed import removed_module + + +if __name__ == '__main__': + removed_module(removed_in='2.9') diff --git a/test/integration/targets/ansible-doc/runme.sh b/test/integration/targets/ansible-doc/runme.sh new file mode 100755 index 00000000000..341c08d88bb --- /dev/null +++ b/test/integration/targets/ansible-doc/runme.sh @@ -0,0 +1,4 @@ +#!/usr/bin/env bash + +set -eux +ansible-playbook test.yml -i inventory "$@" diff --git a/test/integration/targets/ansible-doc/test.yml b/test/integration/targets/ansible-doc/test.yml new file mode 100644 index 00000000000..4398752c407 --- /dev/null +++ b/test/integration/targets/ansible-doc/test.yml @@ -0,0 +1,105 @@ +- hosts: localhost + gather_facts: no + environment: + ANSIBLE_LIBRARY: "{{ playbook_dir }}/library" + tasks: + - name: non-existent module + command: ansible-doc test_does_not_exist + register: result + - assert: + that: + - '"[WARNING]: module test_does_not_exist not found in:" in result.stderr' + + - name: documented module + command: ansible-doc test_docs + register: result + - assert: + that: + - '"WARNING" not in result.stderr' + - '"TEST_DOCS " in result.stdout' + - '"AUTHOR: Ansible Core Team" in result.stdout' + + - name: documented module without metadata + command: ansible-doc test_docs_no_metadata + register: result + - assert: + that: + - '"WARNING" not in result.stderr' + - '"TEST_DOCS_NO_METADATA " in result.stdout' + - '"AUTHOR: Ansible Core Team" in result.stdout' + + - name: documented module with no status in metadata + command: ansible-doc test_docs_no_status + register: result + - assert: + that: + - '"WARNING" not in result.stderr' + - '"TEST_DOCS_NO_STATUS " in result.stdout' + - '"AUTHOR: Ansible Core Team" in result.stdout' + + - name: documented module with non-iterable status in metadata + command: ansible-doc test_docs_non_iterable_status + register: result + - assert: + that: + - '"WARNING" not in result.stderr' + - '"TEST_DOCS_NON_ITERABLE_STATUS " in result.stdout' + - '"AUTHOR: Ansible Core Team" in result.stdout' + + - name: documented module with removed status + command: ansible-doc test_docs_removed_status + register: result + - assert: + that: + - '"WARNING" not in result.stderr' + - '"TEST_DOCS_REMOVED_STATUS " in result.stdout' + - '"AUTHOR: Ansible Core Team" in result.stdout' + + - name: empty module + command: ansible-doc test_empty + register: result + - assert: + that: + - 'result.stdout == ""' + - 'result.stderr == ""' + + - name: module with no documentation + command: ansible-doc test_no_docs + register: result + - assert: + that: + - 'result.stdout == ""' + - 'result.stderr == ""' + + - name: module with no documentation and no metadata + command: ansible-doc test_no_docs_no_metadata + register: result + - assert: + that: + - 'result.stdout == ""' + - 'result.stderr == ""' + + - name: module with no documentation and no status in metadata + command: ansible-doc test_no_docs_no_status + ignore_errors: yes + register: result + - assert: + that: + - 'result is failed' + - '"ERROR! module test_no_docs_no_status missing documentation (or could not parse documentation): test_no_docs_no_status did not contain a DOCUMENTATION attribute" in result.stderr' + + - name: module with no documentation and non-iterable status in metadata + command: ansible-doc test_no_docs_non_iterable_status + ignore_errors: yes + register: result + - assert: + that: + - 'result is failed' + - '"ERROR! module test_no_docs_non_iterable_status missing documentation (or could not parse documentation): test_no_docs_non_iterable_status did not contain a DOCUMENTATION attribute" in result.stderr' + + - name: module with no documentation and removed status + command: ansible-doc test_no_docs_removed_status + register: result + - assert: + that: + - '"[WARNING]: module test_no_docs_removed_status has been removed" in result.stderr'