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

@@ -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)
+
+                # 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)
 
-                if doc:
-                    # doc may be None, such as when the module has been removed
-                    doc['filename'] = filename
+            raise ValueError('%s did not contain a DOCUMENTATION attribute' % 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)))
+        doc['filename'] = filename
         return doc, plainexamples, returndocs, metadata
 
     @staticmethod
-    def format_plugin_doc(plugin, loader, plugin_type, search_paths):
-        text = ''
-
-        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)
+    def format_plugin_doc(plugin, plugin_type, doc, plainexamples, returndocs, metadata):
+        # 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:
-                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
 

test/integration/targets/ansible-doc/aliases

@@ -0,0 +1 @@
+shippable/posix/group1

test/integration/targets/ansible-doc/inventory

@@ -0,0 +1 @@
+not_empty  # avoid empty empty hosts list warning without defining explicit localhost

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()

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()

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()

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()

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()

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()

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()

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()

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()

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')

test/integration/targets/ansible-doc/runme.sh

@@ -0,0 +1,4 @@
+#!/usr/bin/env bash
+
+set -eux
+ansible-playbook test.yml -i inventory "$@"

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'