From 34589cee6df26c1f4cbb98637144eb8230f0cc0a Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Fri, 30 Jun 2017 06:48:32 -0700 Subject: [PATCH] Unittests for extracting metadata from plugins (#26218) * Unittests for extracting metadata from plugins * Port plugin_docs to use the generic extract_metadata function * Make the helper functions seek_end_of{string,dict} private --- lib/ansible/parsing/metadata.py | 34 ++--- lib/ansible/utils/plugin_docs.py | 7 +- test/units/parsing/test_metadata.py | 191 ++++++++++++++++++++++++++++ 3 files changed, 213 insertions(+), 19 deletions(-) create mode 100644 test/units/parsing/test_metadata.py diff --git a/lib/ansible/parsing/metadata.py b/lib/ansible/parsing/metadata.py index 691453c9bf1..8b2f85f6866 100644 --- a/lib/ansible/parsing/metadata.py +++ b/lib/ansible/parsing/metadata.py @@ -31,7 +31,7 @@ class ParseError(Exception): pass -def seek_end_of_dict(module_data, start_line, start_col, next_node_line, next_node_col): +def _seek_end_of_dict(module_data, start_line, start_col, next_node_line, next_node_col): """Look for the end of a dict in a set of lines We know the starting position of the dict and we know the start of the @@ -105,7 +105,7 @@ def seek_end_of_dict(module_data, start_line, start_col, next_node_line, next_no return end_line, end_col -def seek_end_of_string(module_data, start_line, start_col, next_node_line, next_node_col): +def _seek_end_of_string(module_data, start_line, start_col, next_node_line, next_node_col): """ This is much trickier than finding the end of a dict. A dict has only one ending character, "}". Strings have four potential ending characters. We @@ -181,26 +181,26 @@ def extract_metadata(module_data): if isinstance(child.value, ast.Dict): # Determine where the current metadata ends - end_line, end_col = seek_end_of_dict(module_data, - child.lineno - 1, - child.col_offset, - next_lineno, - next_col_offset) + end_line, end_col = _seek_end_of_dict(module_data, + child.lineno - 1, + child.col_offset, + next_lineno, + next_col_offset) elif isinstance(child.value, ast.Str): metadata = yaml.safe_load(child.value.s) - end_line, end_col = seek_end_of_string(module_data, - child.lineno - 1, - child.col_offset, - next_lineno, - next_col_offset) + end_line, end_col = _seek_end_of_string(module_data, + child.lineno - 1, + child.col_offset, + next_lineno, + next_col_offset) elif isinstance(child.value, ast.Bytes): metadata = yaml.safe_load(to_text(child.value.s, errors='surrogate_or_strict')) - end_line, end_col = seek_end_of_string(module_data, - child.lineno - 1, - child.col_offset, - next_lineno, - next_col_offset) + end_line, end_col = _seek_end_of_string(module_data, + child.lineno - 1, + child.col_offset, + next_lineno, + next_col_offset) else: # Example: # ANSIBLE_METADATA = 'junk' diff --git a/lib/ansible/utils/plugin_docs.py b/lib/ansible/utils/plugin_docs.py index 0c44514274e..b62505f0b32 100644 --- a/lib/ansible/utils/plugin_docs.py +++ b/lib/ansible/utils/plugin_docs.py @@ -26,6 +26,7 @@ import yaml from collections import MutableMapping, MutableSet, MutableSequence from ansible.module_utils.six import string_types +from ansible.parsing.metadata import extract_metadata from ansible.parsing.yaml.loader import AnsibleLoader from ansible.plugins import fragment_loader @@ -117,7 +118,8 @@ def get_docstring(filename, verbose=False): } try: - M = ast.parse(''.join(open(filename))) + b_module_data = open(filename, 'rb').read() + M = ast.parse(b_module_data) try: display.debug('Attempt first docstring is yaml docs') docstring = yaml.load(M.body[0].value.s) @@ -145,7 +147,7 @@ def get_docstring(filename, verbose=False): if isinstance(child.value, ast.Dict): data[varkey] = ast.literal_eval(child.value) else: - if theid in ['DOCUMENTATION', 'ANSIBLE_METADATA']: + if theid == 'DOCUMENTATION': # string should be yaml data[varkey] = AnsibleLoader(child.value.s, file_name=filename).get_single_data() else: @@ -153,6 +155,7 @@ def get_docstring(filename, verbose=False): data[varkey] = child.value.s display.debug('assigned :%s' % varkey) + data['metadata'] = extract_metadata(b_module_data)[0] # add fragments to documentation if data['doc']: add_fragments(data['doc'], filename) diff --git a/test/units/parsing/test_metadata.py b/test/units/parsing/test_metadata.py new file mode 100644 index 00000000000..ec05d6ff447 --- /dev/null +++ b/test/units/parsing/test_metadata.py @@ -0,0 +1,191 @@ +# coding: utf-8 +# (c) 2017, Toshio Kuratomi +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +# Make coding more python3-ish +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +from ansible.parsing import metadata as md + +import pytest + +LICENSE = b"""# some license text boilerplate +# That we have at the top of files +""" + +FUTURE_IMPORTS = b""" +from __future__ import (absolute_import, division, print_function) +""" + +REGULAR_IMPORTS = b""" +import test +from foo import bar +""" + +STANDARD_METADATA = b""" +ANSIBLE_METADATA = {'metadata_version': '1.0', + 'status': ['stableinterface'], + 'supported_by': 'core'} +""" + +STRING_STD_METADATA = b""" +ANSIBLE_METADATA = ''' +metadata_version: '1.0' +status: + - 'stableinterface' +supported_by: 'core' +''' +""" + +TRAILING_COMMENT_METADATA = b""" +ANSIBLE_METADATA = {'metadata_version': '1.0', + 'status': ['stableinterface'], + 'supported_by': 'core'} # { Testing } +""" + +MULTIPLE_STATEMENTS_METADATA = b""" +DOCUMENTATION = "" ; ANSIBLE_METADATA = {'metadata_version': '1.0', + 'status': ['stableinterface'], + 'supported_by': 'core'} ; RETURNS = "" +""" + +EMBEDDED_COMMENT_METADATA = b""" +ANSIBLE_METADATA = {'metadata_version': '1.0', + 'status': ['stableinterface'], + # { Testing } + 'supported_by': 'core'} +""" + +HASH_SYMBOL_METADATA = b""" +ANSIBLE_METADATA = {'metadata_version': '1.0 # 4', + 'status': ['stableinterface'], + 'supported_by': 'core # Testing '} +""" + +HASH_SYMBOL_METADATA = b""" +ANSIBLE_METADATA = {'metadata_version': '1.0 # 4', + 'status': ['stableinterface'], + 'supported_by': 'core # Testing '} +""" + +HASH_COMBO_METADATA = b""" +ANSIBLE_METADATA = {'metadata_version': '1.0 # 4', + 'status': ['stableinterface'], + # { Testing } + 'supported_by': 'core'} # { Testing } +""" + +METADATA = {'metadata_version': '1.0', 'status': ['stableinterface'], 'supported_by': 'core'} +HASH_SYMBOL_METADATA = {'metadata_version': '1.0 # 4', 'status': ['stableinterface'], 'supported_by': 'core'} + +METADATA_EXAMPLES = ( + # Standard import + (LICENSE + FUTURE_IMPORTS + STANDARD_METADATA + REGULAR_IMPORTS, + (METADATA, 5, 0, 7, 42, ['ANSIBLE_METADATA'])), + # Metadata at end of file + (LICENSE + FUTURE_IMPORTS + REGULAR_IMPORTS + STANDARD_METADATA.rstrip(), + (METADATA, 8, 0, 10, 42, ['ANSIBLE_METADATA'])), + # Metadata at beginning of file + (STANDARD_METADATA + LICENSE + REGULAR_IMPORTS, + (METADATA, 1, 0, 3, 42, ['ANSIBLE_METADATA'])), + + # Standard import with a trailing comment + (LICENSE + FUTURE_IMPORTS + TRAILING_COMMENT_METADATA + REGULAR_IMPORTS, + (METADATA, 5, 0, 7, 42, ['ANSIBLE_METADATA'])), + # Metadata at end of file with a trailing comment + (LICENSE + FUTURE_IMPORTS + REGULAR_IMPORTS + TRAILING_COMMENT_METADATA.rstrip(), + (METADATA, 8, 0, 10, 42, ['ANSIBLE_METADATA'])), + # Metadata at beginning of file with a trailing comment + (TRAILING_COMMENT_METADATA + LICENSE + REGULAR_IMPORTS, + (METADATA, 1, 0, 3, 42, ['ANSIBLE_METADATA'])), + + # FIXME: Current code cannot handle multiple statements on the same line. + # This is bad style so we're just going to ignore it for now + # Standard import with other statements on the same line + # (LICENSE + FUTURE_IMPORTS + MULTIPLE_STATEMENTS_METADATA + REGULAR_IMPORTS, + # (METADATA, 5, 0, 7, 42, ['ANSIBLE_METADATA'])), + # Metadata at end of file with other statements on the same line + # (LICENSE + FUTURE_IMPORTS + REGULAR_IMPORTS + MULTIPLE_STATEMENTS_METADATA.rstrip(), + # (METADATA, 8, 0, 10, 42, ['ANSIBLE_METADATA'])), + # Metadata at beginning of file with other statements on the same line + # (MULTIPLE_STATEMENTS_METADATA + LICENSE + REGULAR_IMPORTS, + # (METADATA, 1, 0, 3, 42, ['ANSIBLE_METADATA'])), + + # Standard import with comment inside the metadata + (LICENSE + FUTURE_IMPORTS + EMBEDDED_COMMENT_METADATA + REGULAR_IMPORTS, + (METADATA, 5, 0, 8, 42, ['ANSIBLE_METADATA'])), + # Metadata at end of file with comment inside the metadata + (LICENSE + FUTURE_IMPORTS + REGULAR_IMPORTS + EMBEDDED_COMMENT_METADATA.rstrip(), + (METADATA, 8, 0, 11, 42, ['ANSIBLE_METADATA'])), + # Metadata at beginning of file with comment inside the metadata + (EMBEDDED_COMMENT_METADATA + LICENSE + REGULAR_IMPORTS, + (METADATA, 1, 0, 4, 42, ['ANSIBLE_METADATA'])), + + # FIXME: Current code cannot handle hash symbols in the last element of + # the metadata. Fortunately, the metadata currently fully specifies all + # the strings inside of metadata and none of them can contain a hash. + # Need to fix this to future-proof it against strings containing hashes + # Standard import with hash symbol in metadata + # (LICENSE + FUTURE_IMPORTS + HASH_SYMBOL_METADATA + REGULAR_IMPORTS, + # (HASH_SYMBOL_METADATA, 5, 0, 7, 53, ['ANSIBLE_METADATA'])), + # Metadata at end of file with hash symbol in metadata + # (LICENSE + FUTURE_IMPORTS + REGULAR_IMPORTS + HASH_SYMBOL_HASH_SYMBOL_METADATA.rstrip(), + # (HASH_SYMBOL_METADATA, 8, 0, 10, 53, ['ANSIBLE_METADATA'])), + # Metadata at beginning of file with hash symbol in metadata + # (HASH_SYMBOL_HASH_SYMBOL_METADATA + LICENSE + REGULAR_IMPORTS, + # (HASH_SYMBOL_METADATA, 1, 0, 3, 53, ['ANSIBLE_METADATA'])), + + # Standard import with a bunch of hashes everywhere + (LICENSE + FUTURE_IMPORTS + HASH_COMBO_METADATA + REGULAR_IMPORTS, + (HASH_SYMBOL_METADATA, 5, 0, 8, 42, ['ANSIBLE_METADATA'])), + # Metadata at end of file with a bunch of hashes everywhere + (LICENSE + FUTURE_IMPORTS + REGULAR_IMPORTS + HASH_COMBO_METADATA.rstrip(), + (HASH_SYMBOL_METADATA, 8, 0, 11, 42, ['ANSIBLE_METADATA'])), + # Metadata at beginning of file with a bunch of hashes everywhere + (HASH_COMBO_METADATA + LICENSE + REGULAR_IMPORTS, + (HASH_SYMBOL_METADATA, 1, 0, 4, 42, ['ANSIBLE_METADATA'])), + + # Standard import with a junk ANSIBLE_METADATA as well + (LICENSE + FUTURE_IMPORTS + b"\nANSIBLE_METADAtA = 'junk'\n" + HASH_COMBO_METADATA + REGULAR_IMPORTS, + (HASH_SYMBOL_METADATA, 7, 0, 10, 42, ['ANSIBLE_METADATA'])), +) + +# FIXME: String/yaml metadata is not implemented yet. Need more test cases once it is implemented +STRING_METADATA_EXAMPLES = ( + # Standard import + (LICENSE + FUTURE_IMPORTS + STRING_STD_METADATA + REGULAR_IMPORTS, + (METADATA, 5, 0, 10, 3, ['ANSIBLE_METADATA'])), + # Metadata at end of file + (LICENSE + FUTURE_IMPORTS + REGULAR_IMPORTS + STRING_STD_METADATA.rstrip(), + (METADATA, 8, 0, 13, 3, ['ANSIBLE_METADATA'])), + # Metadata at beginning of file + (STRING_STD_METADATA + LICENSE + REGULAR_IMPORTS, + (METADATA, 1, 0, 6, 3, ['ANSIBLE_METADATA'])), +) + + +@pytest.mark.parametrize("code, expected", METADATA_EXAMPLES) +def test_extract_metadata(code, expected): + assert md.extract_metadata(code) == expected + + +@pytest.mark.parametrize("code, expected", STRING_METADATA_EXAMPLES) +def test_extract_string_metadata(code, expected): + # FIXME: String/yaml metadata is not implemented yet. + with pytest.raises(NotImplementedError): + assert md.extract_metadata(code) == expected