From bfab54e8f4db4c3b5e8918501e362a4b941130c7 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 18 May 2015 11:00:34 -0500 Subject: [PATCH 01/89] Initial commit --- ansible_testing/__init__.py | 0 ansible_testing/modules.py | 164 ++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100644 ansible_testing/__init__.py create mode 100644 ansible_testing/modules.py diff --git a/ansible_testing/__init__.py b/ansible_testing/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py new file mode 100644 index 00000000000..5451c46c6aa --- /dev/null +++ b/ansible_testing/modules.py @@ -0,0 +1,164 @@ +#!/usr/bin/env python + +from __future__ import print_function + +import os +import ast +import sys +import argparse + +from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES + + +class ModuleValidator(object): + def __init__(self, path): + self.path = path + self.basename = os.path.basename(self.path) + self.name, _ = os.path.splitext(self.basename) + + self.errors = [] + self.warnings = [] + + with open(path) as f: + text = f.read() + self.length = len(text.splitlines()) + self.ast = ast.parse(text) + + def _just_docs(self): + for child in self.ast.body: + if not isinstance(child, ast.Assign): + return False + return True + + def _find_module_utils(self): + linenos = [] + for child in self.ast.body: + found_module_utils_import = False + if isinstance(child, ast.ImportFrom): + if child.module.startswith('ansible.module_utils.'): + found_module_utils_import = True + linenos.append(child.lineno) + + if not child.names: + self.errors.append('%s: not a "from" import"' % + child.module) + + found_alias = False + for name in child.names: + if isinstance(name, ast.alias): + found_alias = True + if name.asname or name.name != '*': + self.errors.append('%s: did not import "*"' % + child.module) + + if found_module_utils_import and not found_alias: + self.errors.append('%s: did not import "*"' % child.module) + + if not linenos: + self.errors.append('Did not find a module_utils import') + + return linenos + + def _find_main_call(self): + lineno = False + if_bodies = [] + for child in self.ast.body: + if isinstance(child, ast.If): + try: + if child.test.left.id == '__name__': + if_bodies.extend(child.body) + except AttributeError: + pass + + bodies = self.ast.body + bodies.extend(if_bodies) + + for child in bodies: + if isinstance(child, ast.Expr): + if isinstance(child.value, ast.Call): + if (isinstance(child.value.func, ast.Name) and + child.value.func.id == 'main'): + lineno = child.lineno + if lineno != self.length: + self.warnings.append('Call to main() not the last ' + 'line') + + if not lineno: + self.errors.append('Did not find a call to main') + + return lineno or 0 + + def _find_has_import(self): + for child in self.ast.body: + found_try_except_import = False + found_has = False + if isinstance(child, ast.TryExcept): + bodies = child.body + bodies.extend([h.body for h in child.handlers]) + for grandchild in bodies: + if isinstance(grandchild, ast.Import): + found_try_except_import = True + elif isinstance(grandchild, ast.Assign): + for target in grandchild.targets: + if target.id.lower().startswith('has_'): + found_has = True + if found_try_except_import and not found_has: + self.warnings.append('Found Try/Except block without HAS_ ' + 'assginment') + + def validate(self): + if set([self.basename, self.name]) & set(BLACKLIST_MODULES): + return + + doc, examples, ret = get_docstring(self.path) + if not bool(doc): + self.errors.append('Invalid or no DOCUMENTATION provided') + if not bool(examples): + self.errors.append('No EXAMPLES provided') + if not bool(ret): + self.warnings.append('No RETURN provided') + + if not self._just_docs(): + module_utils = self._find_module_utils() + main = self._find_main_call() + for mu in module_utils: + if mu < main - 10: + self.errors.append('module_utils import not near main()') + + self._find_has_import() + + def report(self, warnings=False): + if self.errors or (warnings and self.warnings): + print('=' * 76) + print(self.basename) + print('=' * 76) + + for error in self.errors: + print('ERROR: %s' % error) + if warnings: + for warning in self.warnings: + print('WARNING: %s' % warning) + + if self.errors or (warnings and self.warnings): + print() + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument('modules', help='Path to modules') + parser.add_argument('-w', '--warnings', help='Show warnings', + action='store_true') + args = parser.parse_args() + + for root, dirs, files in os.walk(args.modules): + for filename in files: + path = os.path.join(root, filename) + if not path.endswith('.py') or filename == '__init__.py': + continue + mv = ModuleValidator(os.path.abspath(path)) + mv.validate() + mv.report(args.warnings) + + +if __name__ == '__main__': + main() From b608194e5980efc004bec109d837bc9318fce42a Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 18 May 2015 11:49:02 -0500 Subject: [PATCH 02/89] Make call to main() not at bottom an error --- ansible_testing/modules.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 5451c46c6aa..05527b34554 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -4,7 +4,6 @@ from __future__ import print_function import os import ast -import sys import argparse from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES @@ -79,8 +78,8 @@ class ModuleValidator(object): if (isinstance(child.value.func, ast.Name) and child.value.func.id == 'main'): lineno = child.lineno - if lineno != self.length: - self.warnings.append('Call to main() not the last ' + if lineno < self.length - 1: + self.errors.append('Call to main() not the last ' 'line') if not lineno: From 7a8862975e56f071be44d2c21f5ff46fdd6cd576 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 18 May 2015 11:59:53 -0500 Subject: [PATCH 03/89] Fix try/except HAS_ logic --- ansible_testing/modules.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 05527b34554..77d05ae893d 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -93,11 +93,12 @@ class ModuleValidator(object): found_has = False if isinstance(child, ast.TryExcept): bodies = child.body - bodies.extend([h.body for h in child.handlers]) + for handler in child.handlers: + bodies.extend(handler.body) for grandchild in bodies: if isinstance(grandchild, ast.Import): found_try_except_import = True - elif isinstance(grandchild, ast.Assign): + if isinstance(grandchild, ast.Assign): for target in grandchild.targets: if target.id.lower().startswith('has_'): found_has = True From b6c1bcb64d4edaca448a184c86e8407b7d821767 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Mon, 18 May 2015 11:37:01 -0700 Subject: [PATCH 04/89] Some checks that the modules are also python modules * Pull some logic into a Validator base class * Add a PythonPackageValidator that checks directories are python packages * Handle files that have python syntax errors * Report modules that do not have a .py extension --- ansible_testing/modules.py | 135 +++++++++++++++++++++++++++++++------ 1 file changed, 115 insertions(+), 20 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 77d05ae893d..9fae72a2ad0 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -3,25 +3,91 @@ from __future__ import print_function import os +import abc import ast import argparse +from fnmatch import fnmatch from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES -class ModuleValidator(object): +BLACKLIST_DIRS = frozenset(('.git',)) + +class Validator(object): + """Validator instances are intended to be run on a single object. if you + are scanning multiple objects for problems, you'll want to have a separate + Validator for each one.""" + __metaclass__ = abc.ABCMeta + + def __init__(self): + self.reset() + + def reset(self): + """Reset the test results""" + self.errors = [] + self.warnings = [] + + @abc.abstractproperty + def object_name(self): + """Name of the object we validated""" + pass + + @abc.abstractmethod + def validate(self, reset=True): + """Run this method to generate the test results""" + if reset: + self.reset() + + def report(self, warnings=False): + """Print out the test results""" + if self.errors or (warnings and self.warnings): + print('=' * 76) + print(self.object_name) + print('=' * 76) + + for error in self.errors: + print('ERROR: %s' % error) + if warnings: + for warning in self.warnings: + print('WARNING: %s' % warning) + + if self.errors or (warnings and self.warnings): + print() + + +class ModuleValidator(Validator): + BLACKLIST_PATTERNS = ('.git*', '*.pyc', '*.pyo', '.*') + BLACKLIST_FILES = frozenset(('.git', '.gitignore', '.travis.yml', '.gitattributes', '.gitmodules', 'COPYING', 'CONTRIBUTING.md', 'README.md', '__init__.py')) + BLACKLIST = BLACKLIST_FILES.union(BLACKLIST_MODULES) + def __init__(self, path): + super(ModuleValidator, self).__init__() + self.path = path self.basename = os.path.basename(self.path) self.name, _ = os.path.splitext(self.basename) - self.errors = [] - self.warnings = [] - with open(path) as f: text = f.read() self.length = len(text.splitlines()) - self.ast = ast.parse(text) + try: + self.ast = ast.parse(text) + except: + self.ast = None + + @property + def object_name(self): + return self.basename + + def _python_module(self): + if self.path.endswith('.py'): + return True + return False + + def _powershell_module(self): + if self.path.endswith('.ps1'): + return True + return False def _just_docs(self): for child in self.ast.body: @@ -107,7 +173,23 @@ class ModuleValidator(object): 'assginment') def validate(self): - if set([self.basename, self.name]) & set(BLACKLIST_MODULES): + super(ModuleValidator, self).validate() + + # Blacklists -- these files are not checked + if not frozenset((self.basename, self.name)).isdisjoint(self.BLACKLIST): + return + for pat in self.BLACKLIST_PATTERNS: + if fnmatch(self.basename, pat): + return + + if self._powershell_module(): + self.warnings.append('Cannot check powershell modules at this time. Skipping') + return + if not self._python_module(): + self.errors.append('Official Ansible modules must have a .py extension') + return + if self.ast is None: + self.errors.append('Python SyntaxError while parsing module') return doc, examples, ret = get_docstring(self.path) @@ -127,20 +209,24 @@ class ModuleValidator(object): self._find_has_import() - def report(self, warnings=False): - if self.errors or (warnings and self.warnings): - print('=' * 76) - print(self.basename) - print('=' * 76) - for error in self.errors: - print('ERROR: %s' % error) - if warnings: - for warning in self.warnings: - print('WARNING: %s' % warning) +class PythonPackageValidator(Validator): + def __init__(self, path): + super(PythonPackageValidator, self).__init__() - if self.errors or (warnings and self.warnings): - print() + self.path = path + self.basename = os.path.basename(path) + + @property + def object_name(self): + return self.basename + + def validate(self): + super(PythonPackageValidator, self).validate() + + init_file = os.path.join(self.path, '__init__.py') + if not os.path.exists(init_file): + self.errors.append('Ansible module subdirectories must contain an __init__.py') def main(): @@ -151,10 +237,19 @@ def main(): args = parser.parse_args() for root, dirs, files in os.walk(args.modules): + basedir = root[len(args.modules)+1:].split('/', 1)[0] + if basedir in BLACKLIST_DIRS: + continue + for dirname in dirs: + if root == args.modules and dirname in BLACKLIST_DIRS: + continue + path = os.path.join(root, dirname) + pv = PythonPackageValidator(os.path.abspath(path)) + pv.validate() + pv.report(args.warnings) + for filename in files: path = os.path.join(root, filename) - if not path.endswith('.py') or filename == '__init__.py': - continue mv = ModuleValidator(os.path.abspath(path)) mv.validate() mv.report(args.warnings) From f0413bfd454bab183d3637bcf4c95e7484b894c5 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 18 May 2015 14:11:15 -0500 Subject: [PATCH 05/89] pep8 cleanup --- ansible_testing/modules.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 9fae72a2ad0..e86e01ff828 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -13,6 +13,7 @@ from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES BLACKLIST_DIRS = frozenset(('.git',)) + class Validator(object): """Validator instances are intended to be run on a single object. if you are scanning multiple objects for problems, you'll want to have a separate @@ -57,7 +58,10 @@ class Validator(object): class ModuleValidator(Validator): BLACKLIST_PATTERNS = ('.git*', '*.pyc', '*.pyo', '.*') - BLACKLIST_FILES = frozenset(('.git', '.gitignore', '.travis.yml', '.gitattributes', '.gitmodules', 'COPYING', 'CONTRIBUTING.md', 'README.md', '__init__.py')) + BLACKLIST_FILES = frozenset(('.git', '.gitignore', '.travis.yml', + '.gitattributes', '.gitmodules', 'COPYING', + 'CONTRIBUTING.md', 'README.md', + '__init__.py')) BLACKLIST = BLACKLIST_FILES.union(BLACKLIST_MODULES) def __init__(self, path): @@ -176,17 +180,20 @@ class ModuleValidator(Validator): super(ModuleValidator, self).validate() # Blacklists -- these files are not checked - if not frozenset((self.basename, self.name)).isdisjoint(self.BLACKLIST): + if not frozenset((self.basename, + self.name)).isdisjoint(self.BLACKLIST): return for pat in self.BLACKLIST_PATTERNS: if fnmatch(self.basename, pat): return if self._powershell_module(): - self.warnings.append('Cannot check powershell modules at this time. Skipping') + self.warnings.append('Cannot check powershell modules at this ' + 'time. Skipping') return if not self._python_module(): - self.errors.append('Official Ansible modules must have a .py extension') + self.errors.append('Official Ansible modules must have a .py ' + 'extension') return if self.ast is None: self.errors.append('Python SyntaxError while parsing module') @@ -226,7 +233,8 @@ class PythonPackageValidator(Validator): init_file = os.path.join(self.path, '__init__.py') if not os.path.exists(init_file): - self.errors.append('Ansible module subdirectories must contain an __init__.py') + self.errors.append('Ansible module subdirectories must contain an ' + '__init__.py') def main(): From 46670598aac52af2ce3972830412a439991613f5 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 18 May 2015 15:30:58 -0500 Subject: [PATCH 06/89] Add interpreter check. Fixes #1 --- ansible_testing/modules.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index e86e01ff828..cca54d26324 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -72,10 +72,10 @@ class ModuleValidator(Validator): self.name, _ = os.path.splitext(self.basename) with open(path) as f: - text = f.read() - self.length = len(text.splitlines()) + self.text = f.read() + self.length = len(self.text.splitlines()) try: - self.ast = ast.parse(text) + self.ast = ast.parse(self.text) except: self.ast = None @@ -99,6 +99,10 @@ class ModuleValidator(Validator): return False return True + def _check_interpreter(self): + if not self.text.startswith('#!/usr/bin/python'): + self.errors.append('Interpreter line is not "#!/usr/bin/python"') + def _find_module_utils(self): linenos = [] for child in self.ast.body: @@ -208,6 +212,7 @@ class ModuleValidator(Validator): self.warnings.append('No RETURN provided') if not self._just_docs(): + self._check_interpreter() module_utils = self._find_module_utils() main = self._find_main_call() for mu in module_utils: From 074e4ad47f9589954fe1284cce80f9018abc9886 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 18 May 2015 15:31:27 -0500 Subject: [PATCH 07/89] rstrip modules path, to prevent some strange scenario with .git --- ansible_testing/modules.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index cca54d26324..3c357319312 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -249,6 +249,8 @@ def main(): action='store_true') args = parser.parse_args() + args.modules = args.modules.rstrip('/') + for root, dirs, files in os.walk(args.modules): basedir = root[len(args.modules)+1:].split('/', 1)[0] if basedir in BLACKLIST_DIRS: From af6dde6eae898407668029f899b9de1a60d025b3 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 18 May 2015 15:41:16 -0500 Subject: [PATCH 08/89] Add warning for json import. Fixes #2 --- ansible_testing/modules.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 3c357319312..a685fcd9091 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -103,6 +103,15 @@ class ModuleValidator(Validator): if not self.text.startswith('#!/usr/bin/python'): self.errors.append('Interpreter line is not "#!/usr/bin/python"') + def _find_json_import(self): + for child in self.ast.body: + if isinstance(child, ast.Import): + for name in child.names: + if name.name == 'json': + self.warnings.append('JSON import found, ' + 'already provided by ' + 'ansible.module_utils.basic') + def _find_module_utils(self): linenos = [] for child in self.ast.body: @@ -213,6 +222,7 @@ class ModuleValidator(Validator): if not self._just_docs(): self._check_interpreter() + self._find_json_import() module_utils = self._find_module_utils() main = self._find_main_call() for mu in module_utils: From 58703e47acb8d8f2f9eef96c92310576af85a505 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 18 May 2015 15:52:30 -0500 Subject: [PATCH 09/89] Make module_utils imports not at bottom a warning --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index a685fcd9091..527bff816dd 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -227,7 +227,7 @@ class ModuleValidator(Validator): main = self._find_main_call() for mu in module_utils: if mu < main - 10: - self.errors.append('module_utils import not near main()') + self.errors.warnings('module_utils import not near main()') self._find_has_import() From b121d202f5f89d324fa64d5875b60958559939a4 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 19 May 2015 11:15:10 -0500 Subject: [PATCH 10/89] Um, that was dumb, and apparently not tested, good job self. --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 527bff816dd..2afa3706608 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -227,7 +227,7 @@ class ModuleValidator(Validator): main = self._find_main_call() for mu in module_utils: if mu < main - 10: - self.errors.warnings('module_utils import not near main()') + self.warnings.append('module_utils import not near main()') self._find_has_import() From 823e3c72d3faf9dc8cf76be0c5b1d8961901024e Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 19 May 2015 11:19:52 -0500 Subject: [PATCH 11/89] Track errors/warnings and exit with a code equal to the total --- ansible_testing/modules.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 2afa3706608..29235d73ef8 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -5,7 +5,9 @@ from __future__ import print_function import os import abc import ast +import sys import argparse + from fnmatch import fnmatch from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES @@ -46,15 +48,21 @@ class Validator(object): print(self.object_name) print('=' * 76) + ret = [] + for error in self.errors: print('ERROR: %s' % error) + ret.append(1) if warnings: for warning in self.warnings: print('WARNING: %s' % warning) + ret.append(1) if self.errors or (warnings and self.warnings): print() + return len(ret) + class ModuleValidator(Validator): BLACKLIST_PATTERNS = ('.git*', '*.pyc', '*.pyo', '.*') @@ -261,6 +269,8 @@ def main(): args.modules = args.modules.rstrip('/') + exit = [] + for root, dirs, files in os.walk(args.modules): basedir = root[len(args.modules)+1:].split('/', 1)[0] if basedir in BLACKLIST_DIRS: @@ -271,13 +281,15 @@ def main(): path = os.path.join(root, dirname) pv = PythonPackageValidator(os.path.abspath(path)) pv.validate() - pv.report(args.warnings) + exit.append(pv.report(args.warnings)) for filename in files: path = os.path.join(root, filename) mv = ModuleValidator(os.path.abspath(path)) mv.validate() - mv.report(args.warnings) + exit.append(mv.report(args.warnings)) + + sys.exit(sum(exit)) if __name__ == '__main__': From 4c8c0b035f452d4da1b6181096dccd5930757cf7 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 19 May 2015 11:25:11 -0500 Subject: [PATCH 12/89] Check for sys.exit. Fixes #5 --- ansible_testing/modules.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 29235d73ef8..2e72d1dbc3b 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -111,6 +111,10 @@ class ModuleValidator(Validator): if not self.text.startswith('#!/usr/bin/python'): self.errors.append('Interpreter line is not "#!/usr/bin/python"') + def _check_for_sys_exit(self): + if 'sys.exit(' in self.text: + self.errors.append('sys.exit() call found') + def _find_json_import(self): for child in self.ast.body: if isinstance(child, ast.Import): @@ -230,6 +234,7 @@ class ModuleValidator(Validator): if not self._just_docs(): self._check_interpreter() + self._check_for_sys_exit() self._find_json_import() module_utils = self._find_module_utils() main = self._find_main_call() From 8ff644680d75cbd3ca498404ba2cbb986164c2f5 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 19 May 2015 11:28:32 -0500 Subject: [PATCH 13/89] Check for missing GPLv3 license header in module. Fixes #4 --- ansible_testing/modules.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 2e72d1dbc3b..cfd5a40afb9 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -115,6 +115,11 @@ class ModuleValidator(Validator): if 'sys.exit(' in self.text: self.errors.append('sys.exit() call found') + def _check_for_gpl3_header(self): + if ('GNU General Public License' not in self.text and + 'version 3' not in self.text): + self.errors.append('GPLv3 license header not found') + def _find_json_import(self): for child in self.ast.body: if isinstance(child, ast.Import): @@ -235,6 +240,7 @@ class ModuleValidator(Validator): if not self._just_docs(): self._check_interpreter() self._check_for_sys_exit() + self._check_for_gpl3_header() self._find_json_import() module_utils = self._find_module_utils() main = self._find_main_call() From 90c469d8ec664256d1284db29397aa0d81e60c9c Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 19 May 2015 13:25:14 -0500 Subject: [PATCH 14/89] Require some module_utils imports to be at the bottom --- ansible_testing/modules.py | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index cfd5a40afb9..e81069bdcfb 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -72,6 +72,17 @@ class ModuleValidator(Validator): '__init__.py')) BLACKLIST = BLACKLIST_FILES.union(BLACKLIST_MODULES) + BOTTOM_IMPORTS = frozenset(( + 'ansible.module_utils.basic', + 'ansible.module_utils.urls', + 'ansible.module_utils.facts', + 'ansible.module_utils.splitter', + 'ansible.module_utils.known_hosts', + )) + BOTTOM_IMPORTS_BLACKLIST = frozenset(( + 'command.py', + )) + def __init__(self, path): super(ModuleValidator, self).__init__() @@ -107,6 +118,9 @@ class ModuleValidator(Validator): return False return True + def _is_bottom_import_blacklisted(self): + return self.object_name in self.BOTTOM_IMPORTS_BLACKLIST + def _check_interpreter(self): if not self.text.startswith('#!/usr/bin/python'): self.errors.append('Interpreter line is not "#!/usr/bin/python"') @@ -129,13 +143,23 @@ class ModuleValidator(Validator): 'already provided by ' 'ansible.module_utils.basic') - def _find_module_utils(self): + def _find_module_utils(self, main): linenos = [] for child in self.ast.body: found_module_utils_import = False if isinstance(child, ast.ImportFrom): if child.module.startswith('ansible.module_utils.'): found_module_utils_import = True + + if child.module in self.BOTTOM_IMPORTS: + if (child.lineno < main - 10 and + not self._is_bottom_import_blacklisted()): + self.errors.append('%s import not near main()' % + child.module) + else: + self.warnings.append('%s import not near main()' % + child.module) + linenos.append(child.lineno) if not child.names: @@ -242,11 +266,8 @@ class ModuleValidator(Validator): self._check_for_sys_exit() self._check_for_gpl3_header() self._find_json_import() - module_utils = self._find_module_utils() main = self._find_main_call() - for mu in module_utils: - if mu < main - 10: - self.warnings.append('module_utils import not near main()') + module_utils = self._find_module_utils(main) self._find_has_import() From efd8787e0bffa512366f7ba8e52c1c5d4c19ea98 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 19 May 2015 13:35:50 -0500 Subject: [PATCH 15/89] flake8 cleanup --- ansible_testing/modules.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index e81069bdcfb..ed2ed9e0c7d 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -267,8 +267,7 @@ class ModuleValidator(Validator): self._check_for_gpl3_header() self._find_json_import() main = self._find_main_call() - module_utils = self._find_module_utils(main) - + self._find_module_utils(main) self._find_has_import() From 0386aa264388d64f0b70f991fc95a0bb27b21948 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 19 May 2015 13:36:03 -0500 Subject: [PATCH 16/89] Allow running against a single file --- ansible_testing/modules.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index ed2ed9e0c7d..12ac730059a 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -293,7 +293,7 @@ class PythonPackageValidator(Validator): def main(): parser = argparse.ArgumentParser() - parser.add_argument('modules', help='Path to modules') + parser.add_argument('modules', help='Path to module or module directory') parser.add_argument('-w', '--warnings', help='Show warnings', action='store_true') args = parser.parse_args() @@ -302,6 +302,13 @@ def main(): exit = [] + # Allow testing against a single file + if os.path.isfile(args.modules): + mv = ModuleValidator(os.path.abspath(args.modules)) + mv.validate() + exit.append(mv.report(args.warnings)) + sys.exit(sum(exit)) + for root, dirs, files in os.walk(args.modules): basedir = root[len(args.modules)+1:].split('/', 1)[0] if basedir in BLACKLIST_DIRS: From 4f9b6899fe1221aca2a9ccc8a3d5dab88f1c8685 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 19 May 2015 16:13:37 -0500 Subject: [PATCH 17/89] Add some basic support for powershell modules --- ansible_testing/modules.py | 68 ++++++++++++++++++++++++++++---------- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 12ac730059a..f9984c3f911 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -10,6 +10,7 @@ import argparse from fnmatch import fnmatch +from ansible.module_common import REPLACER_WINDOWS from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES @@ -83,6 +84,11 @@ class ModuleValidator(Validator): 'command.py', )) + PS_DOC_BLACKLIST = frozenset(( + 'slurp.ps1', + 'setup.ps1' + )) + def __init__(self, path): super(ModuleValidator, self).__init__() @@ -121,7 +127,12 @@ class ModuleValidator(Validator): def _is_bottom_import_blacklisted(self): return self.object_name in self.BOTTOM_IMPORTS_BLACKLIST - def _check_interpreter(self): + def _check_interpreter(self, powershell=False): + if powershell: + if not self.text.startswith('#!powershell\n'): + self.errors.append('Interpreter line is not "#!powershell"') + return + if not self.text.startswith('#!/usr/bin/python'): self.errors.append('Interpreter line is not "#!/usr/bin/python"') @@ -230,6 +241,20 @@ class ModuleValidator(Validator): self.warnings.append('Found Try/Except block without HAS_ ' 'assginment') + def _find_ps_replacers(self): + if 'WANT_JSON' not in self.text: + self.errors.append('WANT_JSON not found in module') + + if REPLACER_WINDOWS not in self.text: + self.errors.append('"%s" not found in module' % REPLACER_WINDOWS) + + def _find_ps_docs_py_file(self): + if self.object_name in self.PS_DOC_BLACKLIST: + return + py_path = self.path.replace('.ps1', '.py') + if not os.path.isfile(py_path): + self.errors.append('Missing python documentation file') + def validate(self): super(ModuleValidator, self).validate() @@ -241,35 +266,44 @@ class ModuleValidator(Validator): if fnmatch(self.basename, pat): return - if self._powershell_module(): - self.warnings.append('Cannot check powershell modules at this ' - 'time. Skipping') - return - if not self._python_module(): +# if self._powershell_module(): +# self.warnings.append('Cannot check powershell modules at this ' +# 'time. Skipping') +# return + if not self._python_module() and not self._powershell_module(): self.errors.append('Official Ansible modules must have a .py ' - 'extension') + 'extension for python modules or a .ps1 ' + 'for powershell modules') return - if self.ast is None: + + if self._python_module() and self.ast is None: self.errors.append('Python SyntaxError while parsing module') return - doc, examples, ret = get_docstring(self.path) - if not bool(doc): - self.errors.append('Invalid or no DOCUMENTATION provided') - if not bool(examples): - self.errors.append('No EXAMPLES provided') - if not bool(ret): - self.warnings.append('No RETURN provided') + if self._python_module(): + doc, examples, ret = get_docstring(self.path) + if not bool(doc): + self.errors.append('Invalid or no DOCUMENTATION provided') + if not bool(examples): + self.errors.append('No EXAMPLES provided') + if not bool(ret): + self.warnings.append('No RETURN provided') - if not self._just_docs(): + if self._python_module() and not self._just_docs(): self._check_interpreter() self._check_for_sys_exit() - self._check_for_gpl3_header() self._find_json_import() main = self._find_main_call() self._find_module_utils(main) self._find_has_import() + if self._powershell_module(): + self._find_ps_replacers() + self._find_ps_docs_py_file() + + self._check_for_gpl3_header() + self._check_interpreter(powershell=self._powershell_module()) + class PythonPackageValidator(Validator): def __init__(self, path): From dcb17e18002fc82894de3bb5fbe551debdb453d0 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 26 May 2015 13:42:36 -0500 Subject: [PATCH 18/89] Only run the interpreter check once --- ansible_testing/modules.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index f9984c3f911..c70b274114d 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -290,7 +290,6 @@ class ModuleValidator(Validator): self.warnings.append('No RETURN provided') if self._python_module() and not self._just_docs(): - self._check_interpreter() self._check_for_sys_exit() self._find_json_import() main = self._find_main_call() From b794d929917ed5eb8d3e8979b2f0356eda3864a9 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 30 Jun 2015 13:50:01 -0500 Subject: [PATCH 19/89] Update BLACKLISTs --- ansible_testing/modules.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index c70b274114d..69737449757 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -66,11 +66,10 @@ class Validator(object): class ModuleValidator(Validator): - BLACKLIST_PATTERNS = ('.git*', '*.pyc', '*.pyo', '.*') + BLACKLIST_PATTERNS = ('.git*', '*.pyc', '*.pyo', '.*', '*.md') BLACKLIST_FILES = frozenset(('.git', '.gitignore', '.travis.yml', '.gitattributes', '.gitmodules', 'COPYING', - 'CONTRIBUTING.md', 'README.md', - '__init__.py')) + '__init__.py', 'VERSION', 'test-docs.sh')) BLACKLIST = BLACKLIST_FILES.union(BLACKLIST_MODULES) BOTTOM_IMPORTS = frozenset(( From da3ce668fa223f351fa0329f3b5f8d813f74115d Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 30 Jun 2015 13:51:01 -0500 Subject: [PATCH 20/89] Check for tabbed indentation --- ansible_testing/modules.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 69737449757..f16a9f53466 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -3,6 +3,7 @@ from __future__ import print_function import os +import re import abc import ast import sys @@ -15,6 +16,7 @@ from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES BLACKLIST_DIRS = frozenset(('.git',)) +INDENT_REGEX = re.compile(r'(^[ \t]*)', flags=re.M) class Validator(object): @@ -144,6 +146,13 @@ class ModuleValidator(Validator): 'version 3' not in self.text): self.errors.append('GPLv3 license header not found') + def _check_for_tabs(self): + indent = INDENT_REGEX.findall(self.text) + for i in indent: + if '\t' in i: + self.errors.append('indentation contains tabs') + break + def _find_json_import(self): for child in self.ast.body: if isinstance(child, ast.Import): @@ -294,6 +303,7 @@ class ModuleValidator(Validator): main = self._find_main_call() self._find_module_utils(main) self._find_has_import() + self._check_for_tabs() if self._powershell_module(): self._find_ps_replacers() From 117ecc1e9ba58deef96301331d5464d189321395 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 30 Jun 2015 13:51:18 -0500 Subject: [PATCH 21/89] Update import for REPLACER_WINDOWS --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index f16a9f53466..77e8ad28c76 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -11,7 +11,7 @@ import argparse from fnmatch import fnmatch -from ansible.module_common import REPLACER_WINDOWS +from ansible.executor.module_common import REPLACER_WINDOWS from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES From d488bd57ccbfdbb02963cdd32aab426554d759a6 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 30 Jun 2015 13:51:37 -0500 Subject: [PATCH 22/89] Don't check docs only files for a proper interpreter line --- ansible_testing/modules.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 77e8ad28c76..56eaefc5800 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -310,7 +310,8 @@ class ModuleValidator(Validator): self._find_ps_docs_py_file() self._check_for_gpl3_header() - self._check_interpreter(powershell=self._powershell_module()) + if not self._just_docs(): + self._check_interpreter(powershell=self._powershell_module()) class PythonPackageValidator(Validator): From 48ce4b7d70b7e3695af090e4c4bdb5d7436e54f7 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 30 Jun 2015 14:17:26 -0500 Subject: [PATCH 23/89] Don't trace if we check a non python module for just docs --- ansible_testing/modules.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 56eaefc5800..b7e060dddc2 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -120,10 +120,13 @@ class ModuleValidator(Validator): return False def _just_docs(self): - for child in self.ast.body: - if not isinstance(child, ast.Assign): - return False - return True + try: + for child in self.ast.body: + if not isinstance(child, ast.Assign): + return False + return True + except AttributeError: + return False def _is_bottom_import_blacklisted(self): return self.object_name in self.BOTTOM_IMPORTS_BLACKLIST From 6b02c1c2619a8b7edd770e0106d05f6a5be0b33e Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 30 Jun 2015 14:26:27 -0500 Subject: [PATCH 24/89] Print the modules path, so it's easier to go find that module --- ansible_testing/modules.py | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index b7e060dddc2..e3c423387b5 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -48,7 +48,7 @@ class Validator(object): """Print out the test results""" if self.errors or (warnings and self.warnings): print('=' * 76) - print(self.object_name) + print(self.object_path) print('=' * 76) ret = [] @@ -90,9 +90,10 @@ class ModuleValidator(Validator): 'setup.ps1' )) - def __init__(self, path): + def __init__(self, path, root=None): super(ModuleValidator, self).__init__() + self._root = root self.path = path self.basename = os.path.basename(self.path) self.name, _ = os.path.splitext(self.basename) @@ -109,6 +110,12 @@ class ModuleValidator(Validator): def object_name(self): return self.basename + @property + def object_path(self): + if self._root: + return self.path.replace(self._root, '').lstrip('/') + return self.object_name + def _python_module(self): if self.path.endswith('.py'): return True @@ -318,9 +325,10 @@ class ModuleValidator(Validator): class PythonPackageValidator(Validator): - def __init__(self, path): + def __init__(self, path, root=None): super(PythonPackageValidator, self).__init__() + self._root = root self.path = path self.basename = os.path.basename(path) @@ -328,6 +336,12 @@ class PythonPackageValidator(Validator): def object_name(self): return self.basename + @property + def object_path(self): + if self._root: + return self.path.replace(self._root, '').lstrip('/') + return self.object_name + def validate(self): super(PythonPackageValidator, self).validate() @@ -344,13 +358,13 @@ def main(): action='store_true') args = parser.parse_args() - args.modules = args.modules.rstrip('/') + args.modules = os.path.abspath(args.modules.rstrip('/')) exit = [] # Allow testing against a single file if os.path.isfile(args.modules): - mv = ModuleValidator(os.path.abspath(args.modules)) + mv = ModuleValidator(os.path.abspath(args.modules), root=args.modules) mv.validate() exit.append(mv.report(args.warnings)) sys.exit(sum(exit)) @@ -363,13 +377,14 @@ def main(): if root == args.modules and dirname in BLACKLIST_DIRS: continue path = os.path.join(root, dirname) - pv = PythonPackageValidator(os.path.abspath(path)) + pv = PythonPackageValidator(os.path.abspath(path), + root=args.modules) pv.validate() exit.append(pv.report(args.warnings)) for filename in files: path = os.path.join(root, filename) - mv = ModuleValidator(os.path.abspath(path)) + mv = ModuleValidator(os.path.abspath(path), root=args.modules) mv.validate() exit.append(mv.report(args.warnings)) From cb87eeccad53c0e283173fe97d7a41d6d1322bb2 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 30 Jun 2015 16:41:35 -0500 Subject: [PATCH 25/89] Catch the traceback from get_docstring so we can output it in the correct spot --- ansible_testing/modules.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index e3c423387b5..8293c4677f2 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -14,6 +14,11 @@ from fnmatch import fnmatch from ansible.executor.module_common import REPLACER_WINDOWS from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES +try: + from cStringIO import StringIO +except ImportError: + from StringIO import StringIO + BLACKLIST_DIRS = frozenset(('.git',)) INDENT_REGEX = re.compile(r'(^[ \t]*)', flags=re.M) @@ -32,12 +37,18 @@ class Validator(object): """Reset the test results""" self.errors = [] self.warnings = [] + self.traces = [] @abc.abstractproperty def object_name(self): """Name of the object we validated""" pass + @abc.abstractproperty + def object_path(self): + """Path of the object we validated""" + pass + @abc.abstractmethod def validate(self, reset=True): """Run this method to generate the test results""" @@ -53,6 +64,8 @@ class Validator(object): ret = [] + for trace in self.traces: + print(trace.replace(self._root, '').lstrip('/')) for error in self.errors: print('ERROR: %s' % error) ret.append(1) @@ -299,7 +312,15 @@ class ModuleValidator(Validator): return if self._python_module(): + sys_stdout = sys.stdout + sys_stderr = sys.stderr + sys.stdout = sys.stderr = StringIO() doc, examples, ret = get_docstring(self.path) + trace = sys.stdout.getvalue() + sys.stdout = sys_stdout + sys.stderr = sys.stderr + if trace: + self.traces.append(trace) if not bool(doc): self.errors.append('Invalid or no DOCUMENTATION provided') if not bool(examples): From 95c9e11cbad75458c24b1cf36b1176b303135879 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 5 Oct 2015 11:10:30 -0500 Subject: [PATCH 26/89] Give line no and column for indentation --- ansible_testing/modules.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 8293c4677f2..640c00885eb 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -21,7 +21,7 @@ except ImportError: BLACKLIST_DIRS = frozenset(('.git',)) -INDENT_REGEX = re.compile(r'(^[ \t]*)', flags=re.M) +INDENT_REGEX = re.compile(r'(^[ \t]*)') class Validator(object): @@ -170,11 +170,13 @@ class ModuleValidator(Validator): self.errors.append('GPLv3 license header not found') def _check_for_tabs(self): - indent = INDENT_REGEX.findall(self.text) - for i in indent: - if '\t' in i: - self.errors.append('indentation contains tabs') - break + for line_no, line in enumerate(self.text.splitlines()): + indent = INDENT_REGEX.search(line) + for i in indent.groups(): + if '\t' in i: + index = line.index(i) + self.errors.append('indentation contains tabs. line %d ' + 'column %d' % (line_no, index)) def _find_json_import(self): for child in self.ast.body: From cf9b22103d882e99da99d198cca4f77bf2afe409 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 5 Oct 2015 11:16:44 -0500 Subject: [PATCH 27/89] Warnings should not increment the exit status --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 640c00885eb..32bebf573cd 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -72,7 +72,7 @@ class Validator(object): if warnings: for warning in self.warnings: print('WARNING: %s' % warning) - ret.append(1) + # ret.append(1) # Don't incrememt exit status for warnings if self.errors or (warnings and self.warnings): print() From 3760ae3bfe01ed1f20c4f2fffe89a4aa16731fd9 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 5 Oct 2015 11:47:20 -0500 Subject: [PATCH 28/89] Add option for regex pattern exclusions for file paths --- ansible_testing/modules.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 32bebf573cd..67af30ea734 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -373,12 +373,28 @@ class PythonPackageValidator(Validator): self.errors.append('Ansible module subdirectories must contain an ' '__init__.py') +def re_compile(value): + """ + Argparse expects things to raise TypeError, re.compile raises an re.error + exception + + This function is a shorthand to convert the re.error exception to a + TypeError + """ + + try: + return re.compile(value) + except re.error as e: + raise TypeError(e) + def main(): parser = argparse.ArgumentParser() parser.add_argument('modules', help='Path to module or module directory') parser.add_argument('-w', '--warnings', help='Show warnings', action='store_true') + parser.add_argument('--exclude', help='Regex exclusion pattern', + type=re_compile) args = parser.parse_args() args.modules = os.path.abspath(args.modules.rstrip('/')) @@ -387,7 +403,10 @@ def main(): # Allow testing against a single file if os.path.isfile(args.modules): - mv = ModuleValidator(os.path.abspath(args.modules), root=args.modules) + path = os.path.abspath(args.modules) + if args.exclude and args.exclude.search(path): + sys.exit(0) + mv = ModuleValidator(path, root=args.modules) mv.validate() exit.append(mv.report(args.warnings)) sys.exit(sum(exit)) @@ -400,6 +419,8 @@ def main(): if root == args.modules and dirname in BLACKLIST_DIRS: continue path = os.path.join(root, dirname) + if args.exclude and args.exclude.search(path): + continue pv = PythonPackageValidator(os.path.abspath(path), root=args.modules) pv.validate() @@ -407,6 +428,8 @@ def main(): for filename in files: path = os.path.join(root, filename) + if args.exclude and args.exclude.search(path): + continue mv = ModuleValidator(os.path.abspath(path), root=args.modules) mv.validate() exit.append(mv.report(args.warnings)) From 2ce2b7a41629842801e5ce84578769ce5ea9a371 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 7 Oct 2015 11:50:44 -0500 Subject: [PATCH 29/89] Detect duplicate globals from basic.py --- ansible_testing/modules.py | 15 ++++++++++++++- ansible_testing/utils.py | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 ansible_testing/utils.py diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 67af30ea734..e94cf607a40 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -10,9 +10,11 @@ import sys import argparse from fnmatch import fnmatch +from utils import find_globals from ansible.executor.module_common import REPLACER_WINDOWS from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES +from ansible.module_utils import basic as module_utils_basic try: from cStringIO import StringIO @@ -22,6 +24,7 @@ except ImportError: BLACKLIST_DIRS = frozenset(('.git',)) INDENT_REGEX = re.compile(r'(^[ \t]*)') +BASIC_RESERVED = frozenset((r for r in dir(module_utils_basic) if r[0] != '_')) class Validator(object): @@ -288,6 +291,14 @@ class ModuleValidator(Validator): if not os.path.isfile(py_path): self.errors.append('Missing python documentation file') + def _find_redeclarations(self): + g = set() + find_globals(g, self.ast.body) + redeclared = BASIC_RESERVED.intersection(g) + if redeclared: + self.warnings.append('Redeclared basic.py variable or ' + 'function: %s' % ', '.join(redeclared)) + def validate(self): super(ModuleValidator, self).validate() @@ -320,7 +331,7 @@ class ModuleValidator(Validator): doc, examples, ret = get_docstring(self.path) trace = sys.stdout.getvalue() sys.stdout = sys_stdout - sys.stderr = sys.stderr + sys.stderr = sys_stderr if trace: self.traces.append(trace) if not bool(doc): @@ -337,6 +348,7 @@ class ModuleValidator(Validator): self._find_module_utils(main) self._find_has_import() self._check_for_tabs() + self._find_redeclarations() if self._powershell_module(): self._find_ps_replacers() @@ -373,6 +385,7 @@ class PythonPackageValidator(Validator): self.errors.append('Ansible module subdirectories must contain an ' '__init__.py') + def re_compile(value): """ Argparse expects things to raise TypeError, re.compile raises an re.error diff --git a/ansible_testing/utils.py b/ansible_testing/utils.py new file mode 100644 index 00000000000..dc5a84e95af --- /dev/null +++ b/ansible_testing/utils.py @@ -0,0 +1,24 @@ +import ast + + +def find_globals(g, tree): + """Uses AST to find globals in an ast tree""" + for child in tree: + if hasattr(child, 'body') and isinstance(child.body, list): + find_globals(g, child.body) + elif isinstance(child, (ast.FunctionDef, ast.ClassDef)): + g.add(child.name) + continue + elif isinstance(child, ast.Assign): + try: + g.add(child.targets[0].id) + except (IndexError, AttributeError): + pass + elif isinstance(child, ast.Import): + g.add(child.names[0].name) + elif isinstance(child, ast.ImportFrom): + for name in child.names: + g_name = name.asname or name.name + if g_name == '*': + continue + g.add(g_name) From 4d24f3ba610d6101857ddc9a9fb501d8639dd7b0 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 7 Oct 2015 15:23:00 -0500 Subject: [PATCH 30/89] Don't abspath things, and don't do root based magic --- ansible_testing/modules.py | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index e94cf607a40..7acfbbb4d94 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -68,7 +68,8 @@ class Validator(object): ret = [] for trace in self.traces: - print(trace.replace(self._root, '').lstrip('/')) + #print(trace.replace(self._root, '').lstrip('/')) + print(trace) for error in self.errors: print('ERROR: %s' % error) ret.append(1) @@ -106,10 +107,9 @@ class ModuleValidator(Validator): 'setup.ps1' )) - def __init__(self, path, root=None): + def __init__(self, path): super(ModuleValidator, self).__init__() - self._root = root self.path = path self.basename = os.path.basename(self.path) self.name, _ = os.path.splitext(self.basename) @@ -128,9 +128,7 @@ class ModuleValidator(Validator): @property def object_path(self): - if self._root: - return self.path.replace(self._root, '').lstrip('/') - return self.object_name + return self.path def _python_module(self): if self.path.endswith('.py'): @@ -360,10 +358,9 @@ class ModuleValidator(Validator): class PythonPackageValidator(Validator): - def __init__(self, path, root=None): + def __init__(self, path): super(PythonPackageValidator, self).__init__() - self._root = root self.path = path self.basename = os.path.basename(path) @@ -373,9 +370,7 @@ class PythonPackageValidator(Validator): @property def object_path(self): - if self._root: - return self.path.replace(self._root, '').lstrip('/') - return self.object_name + return self.path def validate(self): super(PythonPackageValidator, self).validate() @@ -410,16 +405,16 @@ def main(): type=re_compile) args = parser.parse_args() - args.modules = os.path.abspath(args.modules.rstrip('/')) + args.modules = args.modules.rstrip('/') exit = [] # Allow testing against a single file if os.path.isfile(args.modules): - path = os.path.abspath(args.modules) + path = args.modules if args.exclude and args.exclude.search(path): sys.exit(0) - mv = ModuleValidator(path, root=args.modules) + mv = ModuleValidator(path) mv.validate() exit.append(mv.report(args.warnings)) sys.exit(sum(exit)) @@ -434,8 +429,7 @@ def main(): path = os.path.join(root, dirname) if args.exclude and args.exclude.search(path): continue - pv = PythonPackageValidator(os.path.abspath(path), - root=args.modules) + pv = PythonPackageValidator(path) pv.validate() exit.append(pv.report(args.warnings)) @@ -443,7 +437,7 @@ def main(): path = os.path.join(root, filename) if args.exclude and args.exclude.search(path): continue - mv = ModuleValidator(os.path.abspath(path), root=args.modules) + mv = ModuleValidator(path) mv.validate() exit.append(mv.report(args.warnings)) From cbe7052ebe3cd8f704841a1c6c60887326fa8771 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 8 Oct 2015 10:06:30 -0500 Subject: [PATCH 31/89] get_docstring has changed output, rework code to get exception raised by get_docstring --- ansible_testing/modules.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 7acfbbb4d94..56c906158e3 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -8,6 +8,7 @@ import abc import ast import sys import argparse +import traceback from fnmatch import fnmatch from utils import find_globals @@ -16,10 +17,8 @@ from ansible.executor.module_common import REPLACER_WINDOWS from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES from ansible.module_utils import basic as module_utils_basic -try: - from cStringIO import StringIO -except ImportError: - from StringIO import StringIO +# We only use StringIO, since we cannot setattr on cStringIO +from StringIO import StringIO BLACKLIST_DIRS = frozenset(('.git',)) @@ -68,7 +67,6 @@ class Validator(object): ret = [] for trace in self.traces: - #print(trace.replace(self._root, '').lstrip('/')) print(trace) for error in self.errors: print('ERROR: %s' % error) @@ -326,10 +324,17 @@ class ModuleValidator(Validator): sys_stdout = sys.stdout sys_stderr = sys.stderr sys.stdout = sys.stderr = StringIO() - doc, examples, ret = get_docstring(self.path) - trace = sys.stdout.getvalue() - sys.stdout = sys_stdout - sys.stderr = sys_stderr + setattr(sys.stdout, 'encoding', sys_stdout.encoding) + setattr(sys.stderr, 'encoding', sys_stderr.encoding) + try: + doc, examples, ret = get_docstring(self.path, verbose=True) + trace = None + except: + doc, examples, ret = get_docstring(self.path) + trace = traceback.format_exc() + finally: + sys.stdout = sys_stdout + sys.stderr = sys_stderr if trace: self.traces.append(trace) if not bool(doc): From 1464f246a3d061c3e4064f7b97bd3f2a87131f47 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 8 Oct 2015 10:29:10 -0500 Subject: [PATCH 32/89] Add a version --- ansible_testing/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ansible_testing/__init__.py b/ansible_testing/__init__.py index e69de29bb2d..06cf07d6899 100644 --- a/ansible_testing/__init__.py +++ b/ansible_testing/__init__.py @@ -0,0 +1 @@ +__version__ = '0.0.1b' From 86c50839bd113f728277c11b92884828388cd35d Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 8 Oct 2015 13:50:21 -0500 Subject: [PATCH 33/89] We are using enumerate to get line numbers, so we need to add 1 since it is 0 offset --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 56c906158e3..b621f06d432 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -175,7 +175,7 @@ class ModuleValidator(Validator): if '\t' in i: index = line.index(i) self.errors.append('indentation contains tabs. line %d ' - 'column %d' % (line_no, index)) + 'column %d' % (line_no + 1, index)) def _find_json_import(self): for child in self.ast.body: From e3dbe85f255f5b279efdc8a331176ae89884a429 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 18 Nov 2015 16:13:05 -0800 Subject: [PATCH 34/89] Check the version_added value --- ansible_testing/modules.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index b621f06d432..4bb86858ee0 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -11,10 +11,14 @@ import argparse import traceback from fnmatch import fnmatch +from distutils.version import StrictVersion + from utils import find_globals from ansible.executor.module_common import REPLACER_WINDOWS from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES + +from ansible import __version__ as ansible_version from ansible.module_utils import basic as module_utils_basic # We only use StringIO, since we cannot setattr on cStringIO @@ -295,6 +299,22 @@ class ModuleValidator(Validator): self.warnings.append('Redeclared basic.py variable or ' 'function: %s' % ', '.join(redeclared)) + def _check_version_added(self, doc): + try: + version_added = StrictVersion(doc.get('version_added', 0)) + except ValueError: + self.errors.append('version_added is not a valid version ' + 'number: %s' % version_added) + return + + strict_ansible_version = StrictVersion(ansible_version) + should_be = '.'.join(ansible_version.split('.')[:2]) + + if (version_added < strict_ansible_version or + strict_ansible_version < version_added): + self.errors.append('version_added should be %s. Currently %s' % + (should_be, version_added)) + def validate(self): super(ModuleValidator, self).validate() @@ -339,6 +359,8 @@ class ModuleValidator(Validator): self.traces.append(trace) if not bool(doc): self.errors.append('Invalid or no DOCUMENTATION provided') + else: + self._check_version_added(doc) if not bool(examples): self.errors.append('No EXAMPLES provided') if not bool(ret): From 2218e763f8025b2158e6bd72f032af82f5d5c421 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 18 Nov 2015 16:18:21 -0800 Subject: [PATCH 35/89] Get the index of the tab, not the line itself --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 4bb86858ee0..34d409d63a7 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -177,7 +177,7 @@ class ModuleValidator(Validator): indent = INDENT_REGEX.search(line) for i in indent.groups(): if '\t' in i: - index = line.index(i) + index = line.index('\t') self.errors.append('indentation contains tabs. line %d ' 'column %d' % (line_no + 1, index)) From 72690c89bdb8a2d73541249f623a48f1d46366eb Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 24 Nov 2015 10:25:57 -0600 Subject: [PATCH 36/89] Try to parse RETURN as YAML --- ansible_testing/modules.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 34d409d63a7..033eadc4d8c 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -7,6 +7,7 @@ import re import abc import ast import sys +import yaml import argparse import traceback @@ -365,6 +366,12 @@ class ModuleValidator(Validator): self.errors.append('No EXAMPLES provided') if not bool(ret): self.warnings.append('No RETURN provided') + else: + try: + yaml.safe_load(ret) + except: + self.errors.append('RETURN is not valid YAML') + self.traces.append(traceback.format_exc()) if self._python_module() and not self._just_docs(): self._check_for_sys_exit() From e9de4d136fc436590ecd226c34e39fe331b35d91 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 24 Nov 2015 10:26:30 -0600 Subject: [PATCH 37/89] Better tab checking, since we care about any tabs in the line, not just in initial indentation --- ansible_testing/modules.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 033eadc4d8c..8bc344c34b3 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -27,7 +27,7 @@ from StringIO import StringIO BLACKLIST_DIRS = frozenset(('.git',)) -INDENT_REGEX = re.compile(r'(^[ \t]*)') +INDENT_REGEX = re.compile(r'([\t]*)') BASIC_RESERVED = frozenset((r for r in dir(module_utils_basic) if r[0] != '_')) @@ -176,11 +176,10 @@ class ModuleValidator(Validator): def _check_for_tabs(self): for line_no, line in enumerate(self.text.splitlines()): indent = INDENT_REGEX.search(line) - for i in indent.groups(): - if '\t' in i: - index = line.index('\t') - self.errors.append('indentation contains tabs. line %d ' - 'column %d' % (line_no + 1, index)) + if indent and '\t' in line: + index = line.index('\t') + self.errors.append('indentation contains tabs. line %d ' + 'column %d' % (line_no + 1, index)) def _find_json_import(self): for child in self.ast.body: From 8c6c2caf615f6f8e2702fc167ccd4573255c54a6 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 24 Nov 2015 10:27:16 -0600 Subject: [PATCH 38/89] Still scan modules with no extension assuming they are python. Fixes #8 --- ansible_testing/modules.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 8bc344c34b3..705fda3ba7c 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -117,6 +117,8 @@ class ModuleValidator(Validator): self.basename = os.path.basename(self.path) self.name, _ = os.path.splitext(self.basename) + self._python_module_override = False + with open(path) as f: self.text = f.read() self.length = len(self.text.splitlines()) @@ -134,7 +136,7 @@ class ModuleValidator(Validator): return self.path def _python_module(self): - if self.path.endswith('.py'): + if self.path.endswith('.py') or self._python_module_override: return True return False @@ -334,7 +336,7 @@ class ModuleValidator(Validator): self.errors.append('Official Ansible modules must have a .py ' 'extension for python modules or a .ps1 ' 'for powershell modules') - return + self._python_module_override = True if self._python_module() and self.ast is None: self.errors.append('Python SyntaxError while parsing module') From 99158a6a8927451e78c420c836e3ce629726b052 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 24 Nov 2015 10:27:45 -0600 Subject: [PATCH 39/89] If no version_added is found, we should assume the string 0.0 instead of the int 0 --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 705fda3ba7c..f0acaa380b8 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -303,7 +303,7 @@ class ModuleValidator(Validator): def _check_version_added(self, doc): try: - version_added = StrictVersion(doc.get('version_added', 0)) + version_added = StrictVersion(doc.get('version_added', '0.0')) except ValueError: self.errors.append('version_added is not a valid version ' 'number: %s' % version_added) From 1d2cb3968fd9e0a830f7606a7dbbc86fa23d6c5a Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 24 Nov 2015 11:56:50 -0600 Subject: [PATCH 40/89] Still look for EXAMPLES and RETURN if DOCUMENTATION is invalid. Fixes #7 --- ansible_testing/modules.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index f0acaa380b8..29e078a3301 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -293,6 +293,22 @@ class ModuleValidator(Validator): if not os.path.isfile(py_path): self.errors.append('Missing python documentation file') + def _get_docs(self): + docs = None + examples = None + ret = None + for child in self.ast.body: + if isinstance(child, ast.Assign): + for grandchild in child.targets: + if grandchild.id == 'DOCUMENTATION': + docs = child.value.s + elif grandchild.id == 'EXAMPLES': + examples = child.value.s[1:] + elif grandchild.id == 'RETURN': + ret = child.value.s + + return docs, examples, ret + def _find_redeclarations(self): g = set() find_globals(g, self.ast.body) @@ -352,7 +368,8 @@ class ModuleValidator(Validator): doc, examples, ret = get_docstring(self.path, verbose=True) trace = None except: - doc, examples, ret = get_docstring(self.path) + doc = None + _, examples, ret = self._get_docs() trace = traceback.format_exc() finally: sys.stdout = sys_stdout From 61a49e0420d470394ecc01c81b6e3e1fd7099262 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 22 Dec 2015 10:48:36 -0600 Subject: [PATCH 41/89] Only check version correctness on new modules. Fixes #11 --- ansible_testing/modules.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 29e078a3301..d67beb7fd8d 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -16,6 +16,7 @@ from distutils.version import StrictVersion from utils import find_globals +from ansible.plugins import module_loader from ansible.executor.module_common import REPLACER_WINDOWS from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES @@ -157,6 +158,9 @@ class ModuleValidator(Validator): def _is_bottom_import_blacklisted(self): return self.object_name in self.BOTTOM_IMPORTS_BLACKLIST + def _is_new_module(self): + return not module_loader.has_plugin(self.name) + def _check_interpreter(self, powershell=False): if powershell: if not self.text.startswith('#!powershell\n'): @@ -318,6 +322,9 @@ class ModuleValidator(Validator): 'function: %s' % ', '.join(redeclared)) def _check_version_added(self, doc): + if not self._is_new_module(): + return + try: version_added = StrictVersion(doc.get('version_added', '0.0')) except ValueError: From 69ec1e8e6ab36d245ecc9cd6bd3de1e1d2057797 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 22 Dec 2015 10:52:03 -0600 Subject: [PATCH 42/89] Check for requests imports. Fixes #12 --- ansible_testing/modules.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index d67beb7fd8d..1196d699e9b 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -196,6 +196,16 @@ class ModuleValidator(Validator): 'already provided by ' 'ansible.module_utils.basic') + def _find_requests_import(self): + for child in self.ast.body: + if isinstance(child, ast.Import): + for name in child.names: + if name.name == 'requests': + self.errors.append('requests import found, ' + 'should use ' + 'ansible.module_utils.urls ' + 'instead') + def _find_module_utils(self, main): linenos = [] for child in self.ast.body: @@ -401,6 +411,7 @@ class ModuleValidator(Validator): if self._python_module() and not self._just_docs(): self._check_for_sys_exit() self._find_json_import() + self._find_requests_import() main = self._find_main_call() self._find_module_utils(main) self._find_has_import() From bc51bb97ddbb58b66de42ed6c32b9d814309cffd Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 22 Dec 2015 10:53:41 -0600 Subject: [PATCH 43/89] No RETURN should be an error for new modules --- ansible_testing/modules.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 1196d699e9b..928ad176d3a 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -400,7 +400,10 @@ class ModuleValidator(Validator): if not bool(examples): self.errors.append('No EXAMPLES provided') if not bool(ret): - self.warnings.append('No RETURN provided') + if self._is_new_module(): + self.errors.append('No RETURN provided') + else: + self.warnings.append('No RETURN provided') else: try: yaml.safe_load(ret) From e37ba8dd2984a94c67bcf1ac68fff7b40a050c3a Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 22 Dec 2015 10:55:26 -0600 Subject: [PATCH 44/89] Fix bottom import check weirdness --- ansible_testing/modules.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 928ad176d3a..102b2034157 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -101,6 +101,7 @@ class ModuleValidator(Validator): 'ansible.module_utils.facts', 'ansible.module_utils.splitter', 'ansible.module_utils.known_hosts', + 'ansible.module_utils.rax', )) BOTTOM_IMPORTS_BLACKLIST = frozenset(( 'command.py', @@ -217,11 +218,8 @@ class ModuleValidator(Validator): if child.module in self.BOTTOM_IMPORTS: if (child.lineno < main - 10 and not self._is_bottom_import_blacklisted()): - self.errors.append('%s import not near main()' % - child.module) - else: - self.warnings.append('%s import not near main()' % - child.module) + self.errors.append('%s import not near call to ' + 'main()' % child.module) linenos.append(child.lineno) From 8daaa75027c817ec93f22f1dcff6e8e6aab0abc3 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 22 Dec 2015 11:15:34 -0600 Subject: [PATCH 45/89] Note that sys.exit should be exit_json or fail_json --- ansible_testing/modules.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 102b2034157..7e8a546d3f2 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -173,7 +173,8 @@ class ModuleValidator(Validator): def _check_for_sys_exit(self): if 'sys.exit(' in self.text: - self.errors.append('sys.exit() call found') + self.errors.append('sys.exit() call found. Should be ' + 'exit_json/fail_json') def _check_for_gpl3_header(self): if ('GNU General Public License' not in self.text and From ccd49201b132f58f3e0a390c4175d5861806be3b Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 22 Dec 2015 11:15:54 -0600 Subject: [PATCH 46/89] Cast version to string for comparisons --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 7e8a546d3f2..cc68e5df80c 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -335,7 +335,7 @@ class ModuleValidator(Validator): return try: - version_added = StrictVersion(doc.get('version_added', '0.0')) + version_added = StrictVersion(str(doc.get('version_added', '0.0'))) except ValueError: self.errors.append('version_added is not a valid version ' 'number: %s' % version_added) From f0e769e12597359fa41f04baa8a69853fd9f1ebf Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 23 Dec 2015 10:45:51 -0600 Subject: [PATCH 47/89] s/Regex/RegEx/ --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index cc68e5df80c..c475c5d0abe 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -473,7 +473,7 @@ def main(): parser.add_argument('modules', help='Path to module or module directory') parser.add_argument('-w', '--warnings', help='Show warnings', action='store_true') - parser.add_argument('--exclude', help='Regex exclusion pattern', + parser.add_argument('--exclude', help='RegEx exclusion pattern', type=re_compile) args = parser.parse_args() From ab749282592edd158e9bc3293d0b42aa888ea68e Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 18 Jan 2016 18:27:56 -0600 Subject: [PATCH 48/89] Fix local variable 'version_added' referenced before assignment. Fixes #15 --- ansible_testing/modules.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index c475c5d0abe..8bd2ea7cd90 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -337,8 +337,9 @@ class ModuleValidator(Validator): try: version_added = StrictVersion(str(doc.get('version_added', '0.0'))) except ValueError: + version_added = doc.get('version_added', '0.0') self.errors.append('version_added is not a valid version ' - 'number: %s' % version_added) + 'number: %r' % version_added) return strict_ansible_version = StrictVersion(ansible_version) From fca2f088ea5e5ef83e5b9c24103bed6fbd1bacaf Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 12 Feb 2016 14:29:17 -0600 Subject: [PATCH 49/89] Verify that new arguments have a correct version added. Fixes #16 --- ansible_testing/modules.py | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 8bd2ea7cd90..8438a450db5 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -350,6 +350,41 @@ class ModuleValidator(Validator): self.errors.append('version_added should be %s. Currently %s' % (should_be, version_added)) + def _check_for_new_args(self, doc): + if self._is_new_module(): + return + + existing = module_loader.find_plugin(self.name) + existing_doc, _, _ = get_docstring(existing, verbose=True) + existing_options = existing_doc.get('options', {}) + + options = doc.get('options', {}) + + strict_ansible_version = StrictVersion(ansible_version) + should_be = '.'.join(ansible_version.split('.')[:2]) + + for option, details in options.iteritems(): + new = not bool(existing_options.get(option)) + if not new: + continue + + try: + version_added = StrictVersion( + str(details.get('version_added', '0.0')) + ) + except ValueError: + version_added = details.get('version_added', '0.0') + self.errors.append('version_added for new option (%s) ' + 'is not a valid version number: %r' % + (option, version_added)) + continue + + if (version_added < strict_ansible_version or + strict_ansible_version < version_added): + self.errors.append('version_added for new option (%s) should ' + 'be %s. Currently %s' % + (option, should_be, version_added)) + def validate(self): super(ModuleValidator, self).validate() @@ -378,7 +413,7 @@ class ModuleValidator(Validator): if self._python_module(): sys_stdout = sys.stdout sys_stderr = sys.stderr - sys.stdout = sys.stderr = StringIO() + sys.stdout = sys.stderr = buf = StringIO() setattr(sys.stdout, 'encoding', sys_stdout.encoding) setattr(sys.stderr, 'encoding', sys_stderr.encoding) try: @@ -397,6 +432,7 @@ class ModuleValidator(Validator): self.errors.append('Invalid or no DOCUMENTATION provided') else: self._check_version_added(doc) + self._check_for_new_args(doc) if not bool(examples): self.errors.append('No EXAMPLES provided') if not bool(ret): From ef55f9a24337ed0beaf258563184de92411e4b75 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 16 Feb 2016 14:07:54 -0600 Subject: [PATCH 50/89] Clarify that RETURN is a form of documentation --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 8438a450db5..925a4d6a8b4 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -437,7 +437,7 @@ class ModuleValidator(Validator): self.errors.append('No EXAMPLES provided') if not bool(ret): if self._is_new_module(): - self.errors.append('No RETURN provided') + self.errors.append('No RETURN documentation provided') else: self.warnings.append('No RETURN provided') else: From 8ca303032f72460ba9c55cb4afa13188f275c775 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 16 Feb 2016 14:23:29 -0600 Subject: [PATCH 51/89] Clean up and re-order imports --- ansible_testing/modules.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 925a4d6a8b4..db77674dfd9 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -2,29 +2,29 @@ from __future__ import print_function -import os -import re import abc +import argparse import ast +import os +import re import sys -import yaml -import argparse import traceback -from fnmatch import fnmatch -from distutils.version import StrictVersion - -from utils import find_globals +# We only use StringIO, since we cannot setattr on cStringIO +from StringIO import StringIO -from ansible.plugins import module_loader -from ansible.executor.module_common import REPLACER_WINDOWS -from ansible.utils.module_docs import get_docstring, BLACKLIST_MODULES +from distutils.version import StrictVersion +from fnmatch import fnmatch from ansible import __version__ as ansible_version +from ansible.executor.module_common import REPLACER_WINDOWS from ansible.module_utils import basic as module_utils_basic +from ansible.plugins import module_loader +from ansible.utils.module_docs import BLACKLIST_MODULES, get_docstring -# We only use StringIO, since we cannot setattr on cStringIO -from StringIO import StringIO +from utils import find_globals + +import yaml BLACKLIST_DIRS = frozenset(('.git',)) From 87808797ea39302b8ace3e3b1f06a2a93fec0f90 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 16 Feb 2016 14:25:43 -0600 Subject: [PATCH 52/89] Fix flake8 error --- ansible_testing/modules.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index db77674dfd9..01e508bcf5c 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -414,6 +414,8 @@ class ModuleValidator(Validator): sys_stdout = sys.stdout sys_stderr = sys.stderr sys.stdout = sys.stderr = buf = StringIO() + # instead of adding noqa to the above, do something with buf + assert buf setattr(sys.stdout, 'encoding', sys_stdout.encoding) setattr(sys.stderr, 'encoding', sys_stderr.encoding) try: From cd88cb753c5c1eaf654a43177e009a1d62449e33 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 16 Feb 2016 15:49:20 -0600 Subject: [PATCH 53/89] Make sure we find the .py file when looking for a module to compare docs with --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 01e508bcf5c..469fad8f5d6 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -354,7 +354,7 @@ class ModuleValidator(Validator): if self._is_new_module(): return - existing = module_loader.find_plugin(self.name) + existing = module_loader.find_plugin(self.name, mod_type='.py') existing_doc, _, _ = get_docstring(existing, verbose=True) existing_options = existing_doc.get('options', {}) From 6bcc5e6f6a30ada0cb3b63667826563a0d857a41 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 16 Feb 2016 16:05:40 -0600 Subject: [PATCH 54/89] Add *.txt and test dir to exclusions --- ansible_testing/modules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 469fad8f5d6..18bd281d075 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -27,7 +27,7 @@ from utils import find_globals import yaml -BLACKLIST_DIRS = frozenset(('.git',)) +BLACKLIST_DIRS = frozenset(('.git', 'test')) INDENT_REGEX = re.compile(r'([\t]*)') BASIC_RESERVED = frozenset((r for r in dir(module_utils_basic) if r[0] != '_')) @@ -89,7 +89,7 @@ class Validator(object): class ModuleValidator(Validator): - BLACKLIST_PATTERNS = ('.git*', '*.pyc', '*.pyo', '.*', '*.md') + BLACKLIST_PATTERNS = ('.git*', '*.pyc', '*.pyo', '.*', '*.md', '*.txt') BLACKLIST_FILES = frozenset(('.git', '.gitignore', '.travis.yml', '.gitattributes', '.gitmodules', 'COPYING', '__init__.py', 'VERSION', 'test-docs.sh')) From ddf0474a76a97111bae4efe4d31fd78d18b57862 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Sat, 20 Feb 2016 10:30:13 -0600 Subject: [PATCH 55/89] Manipulate YAMLError for docs. Fixes #14 * reference the section the error came from * offset the line number to reference the actual line in the file --- ansible_testing/modules.py | 127 ++++++++++++++++++++++++------------- 1 file changed, 83 insertions(+), 44 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 18bd281d075..6a7928bf778 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -73,6 +73,7 @@ class Validator(object): ret = [] for trace in self.traces: + print('TRACE:') print(trace) for error in self.errors: print('ERROR: %s' % error) @@ -307,20 +308,94 @@ class ModuleValidator(Validator): self.errors.append('Missing python documentation file') def _get_docs(self): - docs = None - examples = None - ret = None + docs = { + 'DOCUMENTATION': { + 'value': None, + 'lineno': 0 + }, + 'EXAMPLES': { + 'value': None, + 'lineno': 0 + }, + 'RETURN': { + 'value': None, + 'lineno': 0 + } + } for child in self.ast.body: if isinstance(child, ast.Assign): for grandchild in child.targets: if grandchild.id == 'DOCUMENTATION': - docs = child.value.s + docs['DOCUMENTATION']['value'] = child.value.s + docs['DOCUMENTATION']['lineno'] = child.lineno elif grandchild.id == 'EXAMPLES': + docs['EXAMPLES']['value'] = child.value.s[1:] + docs['EXAMPLES']['lineno'] = child.lineno examples = child.value.s[1:] elif grandchild.id == 'RETURN': - ret = child.value.s - - return docs, examples, ret + docs['RETURN']['value'] = child.value.s + docs['RETURN']['lineno'] = child.lineno + + return docs + + def _validate_docs(self): + sys_stdout = sys.stdout + sys_stderr = sys.stderr + sys.stdout = sys.stderr = buf = StringIO() + # instead of adding noqa to the above, do something with buf + assert buf + setattr(sys.stdout, 'encoding', sys_stdout.encoding) + setattr(sys.stderr, 'encoding', sys_stderr.encoding) + doc_info = self._get_docs() + try: + doc, examples, ret = get_docstring(self.path, verbose=True) + trace = None + except yaml.YAMLError as e: + doc = None + examples = doc_info['EXAMPLES']['value'] + ret = doc_info['RETURN']['value'] + trace = e + finally: + sys.stdout = sys_stdout + sys.stderr = sys_stderr + if trace: + # This offsets the error line number to where the + # DOCUMENTATION starts so we can just go to that line in the + # module + trace.problem_mark.line += ( + doc_info['DOCUMENTATION']['lineno'] - 1 + ) + trace.problem_mark.name = '%s.DOCUMENTATION' % self.name + self.traces.append(trace) + self.errors.append('DOCUMENTATION is not valid YAML. Line %d ' + 'column %d' % + (trace.problem_mark.line + 1, + trace.problem_mark.column + 1)) + if not bool(doc): + self.errors.append('No DOCUMENTATION provided') + else: + self._check_version_added(doc) + self._check_for_new_args(doc) + if not bool(examples): + self.errors.append('No EXAMPLES provided') + if not bool(ret): + if self._is_new_module(): + self.errors.append('No RETURN documentation provided') + else: + self.warnings.append('No RETURN provided') + else: + try: + yaml.safe_load(ret) + except yaml.YAMLError as e: + e.problem_mark.line += ( + doc_info['RETURN']['lineno'] - 1 + ) + e.problem_mark.name = '%s.RETURN' % self.name + self.errors.append('RETURN is not valid YAML. Line %d ' + 'column %d' % + (e.problem_mark.line + 1, + e.problem_mark.column + 1)) + self.traces.append(e) def _find_redeclarations(self): g = set() @@ -411,43 +486,7 @@ class ModuleValidator(Validator): return if self._python_module(): - sys_stdout = sys.stdout - sys_stderr = sys.stderr - sys.stdout = sys.stderr = buf = StringIO() - # instead of adding noqa to the above, do something with buf - assert buf - setattr(sys.stdout, 'encoding', sys_stdout.encoding) - setattr(sys.stderr, 'encoding', sys_stderr.encoding) - try: - doc, examples, ret = get_docstring(self.path, verbose=True) - trace = None - except: - doc = None - _, examples, ret = self._get_docs() - trace = traceback.format_exc() - finally: - sys.stdout = sys_stdout - sys.stderr = sys_stderr - if trace: - self.traces.append(trace) - if not bool(doc): - self.errors.append('Invalid or no DOCUMENTATION provided') - else: - self._check_version_added(doc) - self._check_for_new_args(doc) - if not bool(examples): - self.errors.append('No EXAMPLES provided') - if not bool(ret): - if self._is_new_module(): - self.errors.append('No RETURN documentation provided') - else: - self.warnings.append('No RETURN provided') - else: - try: - yaml.safe_load(ret) - except: - self.errors.append('RETURN is not valid YAML') - self.traces.append(traceback.format_exc()) + self._validate_docs() if self._python_module() and not self._just_docs(): self._check_for_sys_exit() From 60a21659872d08eba73fca83e5bb0dae9ca12aee Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Sat, 20 Feb 2016 11:37:55 -0600 Subject: [PATCH 56/89] Improved documentation error handling --- ansible_testing/modules.py | 85 +++++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 29 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 6a7928bf778..01fddbc9569 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -74,7 +74,7 @@ class Validator(object): for trace in self.traces: print('TRACE:') - print(trace) + print('\n '.join((' %s' % trace).splitlines())) for error in self.errors: print('ERROR: %s' % error) ret.append(1) @@ -339,53 +339,59 @@ class ModuleValidator(Validator): return docs def _validate_docs(self): - sys_stdout = sys.stdout - sys_stderr = sys.stderr - sys.stdout = sys.stderr = buf = StringIO() - # instead of adding noqa to the above, do something with buf - assert buf - setattr(sys.stdout, 'encoding', sys_stdout.encoding) - setattr(sys.stderr, 'encoding', sys_stderr.encoding) doc_info = self._get_docs() try: - doc, examples, ret = get_docstring(self.path, verbose=True) - trace = None + doc = yaml.safe_load(doc_info['DOCUMENTATION']['value']) except yaml.YAMLError as e: doc = None - examples = doc_info['EXAMPLES']['value'] - ret = doc_info['RETURN']['value'] - trace = e - finally: - sys.stdout = sys_stdout - sys.stderr = sys_stderr - if trace: # This offsets the error line number to where the # DOCUMENTATION starts so we can just go to that line in the # module - trace.problem_mark.line += ( + e.problem_mark.line += ( doc_info['DOCUMENTATION']['lineno'] - 1 ) - trace.problem_mark.name = '%s.DOCUMENTATION' % self.name - self.traces.append(trace) + e.problem_mark.name = '%s.DOCUMENTATION' % self.name + self.traces.append(e) self.errors.append('DOCUMENTATION is not valid YAML. Line %d ' 'column %d' % - (trace.problem_mark.line + 1, - trace.problem_mark.column + 1)) - if not bool(doc): + (e.problem_mark.line + 1, + e.problem_mark.column + 1)) + except AttributeError: self.errors.append('No DOCUMENTATION provided') else: + sys_stdout = sys.stdout + sys_stderr = sys.stderr + sys.stdout = sys.stderr = buf = StringIO() + # instead of adding noqa to the above, do something with buf + assert buf + setattr(sys.stdout, 'encoding', sys_stdout.encoding) + setattr(sys.stderr, 'encoding', sys_stderr.encoding) + try: + get_docstring(self.path, verbose=True) + except AssertionError: + fragment = doc['extends_documentation_fragment'] + self.errors.append('DOCUMENTATION fragment missing: %s' % fragment) + except Exception as e: + self.traces.append(e) + self.errors.append('Unknown DOCUMENTATION error, see TRACE') + finally: + sys.stdout = sys_stdout + sys.stderr = sys_stderr + self._check_version_added(doc) self._check_for_new_args(doc) - if not bool(examples): + + if not bool(doc_info['EXAMPLES']['value']): self.errors.append('No EXAMPLES provided') - if not bool(ret): + + if not bool(doc_info['RETURN']['value']): if self._is_new_module(): self.errors.append('No RETURN documentation provided') else: self.warnings.append('No RETURN provided') else: try: - yaml.safe_load(ret) + yaml.safe_load(doc_info['RETURN']['value']) except yaml.YAMLError as e: e.problem_mark.line += ( doc_info['RETURN']['lineno'] - 1 @@ -429,9 +435,30 @@ class ModuleValidator(Validator): if self._is_new_module(): return - existing = module_loader.find_plugin(self.name, mod_type='.py') - existing_doc, _, _ = get_docstring(existing, verbose=True) - existing_options = existing_doc.get('options', {}) + sys_stdout = sys.stdout + sys_stderr = sys.stderr + sys.stdout = sys.stderr = buf = StringIO() + # instead of adding noqa to the above, do something with buf + assert buf + setattr(sys.stdout, 'encoding', sys_stdout.encoding) + setattr(sys.stderr, 'encoding', sys_stderr.encoding) + try: + existing = module_loader.find_plugin(self.name, mod_type='.py') + existing_doc, _, _ = get_docstring(existing, verbose=True) + existing_options = existing_doc.get('options', {}) + except AssertionError: + fragment = doc['extends_documentation_fragment'] + self.errors.append('Existing DOCUMENTATION fragment missing: %s' % + fragment) + return + except Exception as e: + self.traces.append(e) + self.errors.append('Unknown existing DOCUMENTATION error, see ' + 'TRACE') + return + finally: + sys.stdout = sys_stdout + sys.stderr = sys_stderr options = doc.get('options', {}) From 75b299e6de74942be6ef9ec8e3d14e7001bccf42 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Sat, 20 Feb 2016 11:54:05 -0600 Subject: [PATCH 57/89] Add CaptureStd context manager for capturing stdout and stderr --- ansible_testing/modules.py | 73 ++++++++++++++------------------------ ansible_testing/utils.py | 26 ++++++++++++++ 2 files changed, 53 insertions(+), 46 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 01fddbc9569..8eeb710d2be 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -10,9 +10,6 @@ import re import sys import traceback -# We only use StringIO, since we cannot setattr on cStringIO -from StringIO import StringIO - from distutils.version import StrictVersion from fnmatch import fnmatch @@ -22,7 +19,7 @@ from ansible.module_utils import basic as module_utils_basic from ansible.plugins import module_loader from ansible.utils.module_docs import BLACKLIST_MODULES, get_docstring -from utils import find_globals +from utils import CaptureStd, find_globals import yaml @@ -359,24 +356,17 @@ class ModuleValidator(Validator): except AttributeError: self.errors.append('No DOCUMENTATION provided') else: - sys_stdout = sys.stdout - sys_stderr = sys.stderr - sys.stdout = sys.stderr = buf = StringIO() - # instead of adding noqa to the above, do something with buf - assert buf - setattr(sys.stdout, 'encoding', sys_stdout.encoding) - setattr(sys.stderr, 'encoding', sys_stderr.encoding) - try: - get_docstring(self.path, verbose=True) - except AssertionError: - fragment = doc['extends_documentation_fragment'] - self.errors.append('DOCUMENTATION fragment missing: %s' % fragment) - except Exception as e: - self.traces.append(e) - self.errors.append('Unknown DOCUMENTATION error, see TRACE') - finally: - sys.stdout = sys_stdout - sys.stderr = sys_stderr + with CaptureStd(): + try: + get_docstring(self.path, verbose=True) + except AssertionError: + fragment = doc['extends_documentation_fragment'] + self.errors.append('DOCUMENTATION fragment missing: %s' % + fragment) + except Exception as e: + self.traces.append(e) + self.errors.append('Unknown DOCUMENTATION error, see ' + 'TRACE') self._check_version_added(doc) self._check_for_new_args(doc) @@ -435,30 +425,21 @@ class ModuleValidator(Validator): if self._is_new_module(): return - sys_stdout = sys.stdout - sys_stderr = sys.stderr - sys.stdout = sys.stderr = buf = StringIO() - # instead of adding noqa to the above, do something with buf - assert buf - setattr(sys.stdout, 'encoding', sys_stdout.encoding) - setattr(sys.stderr, 'encoding', sys_stderr.encoding) - try: - existing = module_loader.find_plugin(self.name, mod_type='.py') - existing_doc, _, _ = get_docstring(existing, verbose=True) - existing_options = existing_doc.get('options', {}) - except AssertionError: - fragment = doc['extends_documentation_fragment'] - self.errors.append('Existing DOCUMENTATION fragment missing: %s' % - fragment) - return - except Exception as e: - self.traces.append(e) - self.errors.append('Unknown existing DOCUMENTATION error, see ' - 'TRACE') - return - finally: - sys.stdout = sys_stdout - sys.stderr = sys_stderr + with CaptureStd(): + try: + existing = module_loader.find_plugin(self.name, mod_type='.py') + existing_doc, _, _ = get_docstring(existing, verbose=True) + existing_options = existing_doc.get('options', {}) + except AssertionError: + fragment = doc['extends_documentation_fragment'] + self.errors.append('Existing DOCUMENTATION fragment missing: ' + '%s' % fragment) + return + except Exception as e: + self.traces.append(e) + self.errors.append('Unknown existing DOCUMENTATION error, see ' + 'TRACE') + return options = doc.get('options', {}) diff --git a/ansible_testing/utils.py b/ansible_testing/utils.py index dc5a84e95af..1962040a977 100644 --- a/ansible_testing/utils.py +++ b/ansible_testing/utils.py @@ -1,4 +1,8 @@ import ast +import sys + +# We only use StringIO, since we cannot setattr on cStringIO +from StringIO import StringIO def find_globals(g, tree): @@ -22,3 +26,25 @@ def find_globals(g, tree): if g_name == '*': continue g.add(g_name) + + +class CaptureStd(): + """Context manager to handle capturing stderr and stdout""" + + def __enter__(self): + self.sys_stdout = sys.stdout + self.sys_stderr = sys.stderr + sys.stdout = self.stdout = StringIO() + sys.stderr = self.stderr = StringIO() + setattr(sys.stdout, 'encoding', self.sys_stdout.encoding) + setattr(sys.stderr, 'encoding', self.sys_stderr.encoding) + return self + + def __exit__(self, exc_type, exc_value, traceback): + sys.stdout = self.sys_stdout + sys.stderr = self.sys_stderr + + def get(self): + """Return ``(stdout, stderr)``""" + + return self.stdout.getvalue(), self.stderr.getvalue() From 113c74faac3e083b0410e2b2a4891e6e3753f9fa Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Sat, 20 Feb 2016 11:54:13 -0600 Subject: [PATCH 58/89] flake8 clean ups --- ansible_testing/modules.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 8eeb710d2be..4ac54375a7b 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -8,7 +8,6 @@ import ast import os import re import sys -import traceback from distutils.version import StrictVersion from fnmatch import fnmatch @@ -328,7 +327,6 @@ class ModuleValidator(Validator): elif grandchild.id == 'EXAMPLES': docs['EXAMPLES']['value'] = child.value.s[1:] docs['EXAMPLES']['lineno'] = child.lineno - examples = child.value.s[1:] elif grandchild.id == 'RETURN': docs['RETURN']['value'] = child.value.s docs['RETURN']['lineno'] = child.lineno From 3842ae9dedb598d27a5d70439acfa4a9d1e66406 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 22 Feb 2016 10:01:41 -0600 Subject: [PATCH 59/89] Don't error on version_added for arg, if version_added should be the same as when the module was added. Fixes #18 --- ansible_testing/modules.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 4ac54375a7b..086c5f5bd06 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -411,8 +411,8 @@ class ModuleValidator(Validator): 'number: %r' % version_added) return - strict_ansible_version = StrictVersion(ansible_version) should_be = '.'.join(ansible_version.split('.')[:2]) + strict_ansible_version = StrictVersion(should_be) if (version_added < strict_ansible_version or strict_ansible_version < version_added): @@ -423,6 +423,8 @@ class ModuleValidator(Validator): if self._is_new_module(): return + mod_version_added = StrictVersion(str(doc.get('version_added', '0.0'))) + with CaptureStd(): try: existing = module_loader.find_plugin(self.name, mod_type='.py') @@ -441,8 +443,8 @@ class ModuleValidator(Validator): options = doc.get('options', {}) - strict_ansible_version = StrictVersion(ansible_version) should_be = '.'.join(ansible_version.split('.')[:2]) + strict_ansible_version = StrictVersion(should_be) for option, details in options.iteritems(): new = not bool(existing_options.get(option)) @@ -460,8 +462,9 @@ class ModuleValidator(Validator): (option, version_added)) continue - if (version_added < strict_ansible_version or - strict_ansible_version < version_added): + if (strict_ansible_version != mod_version_added and + (version_added < strict_ansible_version or + strict_ansible_version < version_added)): self.errors.append('version_added for new option (%s) should ' 'be %s. Currently %s' % (option, should_be, version_added)) From 9b31175cf8601f469339f5b1760e358b236c40ff Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 22 Feb 2016 11:11:06 -0600 Subject: [PATCH 60/89] Get module version_added from existing, and catch invalid versions --- ansible_testing/modules.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 086c5f5bd06..721d4e13646 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -423,8 +423,6 @@ class ModuleValidator(Validator): if self._is_new_module(): return - mod_version_added = StrictVersion(str(doc.get('version_added', '0.0'))) - with CaptureStd(): try: existing = module_loader.find_plugin(self.name, mod_type='.py') @@ -441,6 +439,14 @@ class ModuleValidator(Validator): 'TRACE') return + + try: + mod_version_added = StrictVersion( + str(existing_doc.get('version_added', '0.0')) + ) + except ValueError: + mod_version_added = StrictVersion('0.0') + options = doc.get('options', {}) should_be = '.'.join(ansible_version.split('.')[:2]) From 28774875b4888fb605649915f26b55c6afbe3d82 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 29 Feb 2016 09:19:33 -0600 Subject: [PATCH 61/89] On ast parser failure, try compiliation to get error --- ansible_testing/modules.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 721d4e13646..0572576714a 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -498,6 +498,10 @@ class ModuleValidator(Validator): if self._python_module() and self.ast is None: self.errors.append('Python SyntaxError while parsing module') + try: + compile(self.text, self.path, 'exec') + except Exception as e: + self.traces.append(e) return if self._python_module(): From 10d683a962d4e904352beae09adba30f120a86a6 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 29 Feb 2016 09:24:09 -0600 Subject: [PATCH 62/89] Look inside try/except for requests imports --- ansible_testing/modules.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 0572576714a..ee9a83f1aca 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -204,6 +204,18 @@ class ModuleValidator(Validator): 'should use ' 'ansible.module_utils.urls ' 'instead') + elif isinstance(child, ast.TryExcept): + bodies = child.body + for handler in child.handlers: + bodies.extend(handler.body) + for grandchild in bodies: + if isinstance(grandchild, ast.Import): + for name in grandchild.names: + if name.name == 'requests': + self.errors.append('requests import found, ' + 'should use ' + 'ansible.module_utils.urls ' + 'instead') def _find_module_utils(self, main): linenos = [] From a103f81513c2fa631c41e7a8d095cd9cc0873d3d Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 1 Mar 2016 14:20:57 -0600 Subject: [PATCH 63/89] Validate DOCUMENTATION schema --- ansible_testing/modules.py | 24 +++++++++++++++++++++++- ansible_testing/schema.py | 27 +++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 ansible_testing/schema.py diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index ee9a83f1aca..ecece1ef131 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -18,6 +18,8 @@ from ansible.module_utils import basic as module_utils_basic from ansible.plugins import module_loader from ansible.utils.module_docs import BLACKLIST_MODULES, get_docstring +from schema import doc_schema, option_schema + from utils import CaptureStd, find_globals import yaml @@ -345,6 +347,26 @@ class ModuleValidator(Validator): return docs + def _validate_docs_schema(self, doc): + errors = [] + try: + doc_schema(doc) + except Exception as e: + errors.extend(e.errors) + + for key, option in doc.get('options', {}).iteritems(): + try: + option_schema(option) + except Exception as e: + for error in e.errors: + error.path[:0] = ['options', key] + errors.extend(e.errors) + + for error in errors: + path = [str(p) for p in error.path] + self.errors.append('DOCUMENTATION.%s: %s' % + ('.'.join(path), error.error_message)) + def _validate_docs(self): doc_info = self._get_docs() try: @@ -378,6 +400,7 @@ class ModuleValidator(Validator): self.errors.append('Unknown DOCUMENTATION error, see ' 'TRACE') + self._validate_docs_schema(doc) self._check_version_added(doc) self._check_for_new_args(doc) @@ -451,7 +474,6 @@ class ModuleValidator(Validator): 'TRACE') return - try: mod_version_added = StrictVersion( str(existing_doc.get('version_added', '0.0')) diff --git a/ansible_testing/schema.py b/ansible_testing/schema.py new file mode 100644 index 00000000000..76d0df7fdfa --- /dev/null +++ b/ansible_testing/schema.py @@ -0,0 +1,27 @@ +from voluptuous import ALLOW_EXTRA, Any, Required, Schema + +option_schema = Schema( + { + Required('description'): Any(basestring, [basestring]), + 'required': bool, + 'choices': Any(list, basestring), + 'aliases': list, + 'version_added': Any(basestring, float) + }, + extra=ALLOW_EXTRA +) + +doc_schema = Schema( + { + Required('module'): basestring, + 'short_description': Any(basestring, [basestring]), + 'description': Any(basestring, [basestring]), + 'version_added': Any(basestring, float), + 'author': Any(None, basestring, [basestring]), + 'notes': Any(None, [basestring]), + 'requirements': [basestring], + 'options': Any(None, dict), + 'extends_documentation_fragment': Any(basestring, [basestring]) + }, + extra=ALLOW_EXTRA +) From 3d2c5f30162b3e562422f90f48f2a865b34ef9a4 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 1 Mar 2016 16:18:56 -0600 Subject: [PATCH 64/89] Choices should be a list, not a string --- ansible_testing/schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/schema.py b/ansible_testing/schema.py index 76d0df7fdfa..e53831045b7 100644 --- a/ansible_testing/schema.py +++ b/ansible_testing/schema.py @@ -4,7 +4,7 @@ option_schema = Schema( { Required('description'): Any(basestring, [basestring]), 'required': bool, - 'choices': Any(list, basestring), + 'choices': list, 'aliases': list, 'version_added': Any(basestring, float) }, From eb352e2876264385c824909b900076a6b11075e7 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 23 Mar 2016 11:42:00 -0500 Subject: [PATCH 65/89] Ignore the .github directory --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index ecece1ef131..b86aaf67536 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -25,7 +25,7 @@ from utils import CaptureStd, find_globals import yaml -BLACKLIST_DIRS = frozenset(('.git', 'test')) +BLACKLIST_DIRS = frozenset(('.git', 'test', '.github')) INDENT_REGEX = re.compile(r'([\t]*)') BASIC_RESERVED = frozenset((r for r in dir(module_utils_basic) if r[0] != '_')) From d945198faf191437f7e322265161b4eda649d9ab Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 6 Apr 2016 13:58:35 -0500 Subject: [PATCH 66/89] Fix license headers and copyright across all files --- ansible_testing/__init__.py | 18 ++++++++++++++++++ ansible_testing/schema.py | 18 ++++++++++++++++++ ansible_testing/utils.py | 18 ++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/ansible_testing/__init__.py b/ansible_testing/__init__.py index 06cf07d6899..365ff9e927c 100644 --- a/ansible_testing/__init__.py +++ b/ansible_testing/__init__.py @@ -1 +1,19 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2015 Matt Martz +# Copyright (C) 2015 Rackspace US, Inc. +# +# This program 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. +# +# This program 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 this program. If not, see . + __version__ = '0.0.1b' diff --git a/ansible_testing/schema.py b/ansible_testing/schema.py index e53831045b7..6a078306ab4 100644 --- a/ansible_testing/schema.py +++ b/ansible_testing/schema.py @@ -1,3 +1,21 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2015 Matt Martz +# Copyright (C) 2015 Rackspace US, Inc. +# +# This program 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. +# +# This program 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 this program. If not, see . + from voluptuous import ALLOW_EXTRA, Any, Required, Schema option_schema = Schema( diff --git a/ansible_testing/utils.py b/ansible_testing/utils.py index 1962040a977..46ba81d9262 100644 --- a/ansible_testing/utils.py +++ b/ansible_testing/utils.py @@ -1,3 +1,21 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2015 Matt Martz +# Copyright (C) 2015 Rackspace US, Inc. +# +# This program 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. +# +# This program 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 this program. If not, see . + import ast import sys From 9c029eca9e55f8fd41db5c5926cbdf32450c8126 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 6 Apr 2016 14:02:31 -0500 Subject: [PATCH 67/89] Fix license headers and copyright across all files --- ansible_testing/modules.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index b86aaf67536..f7be1135116 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -1,4 +1,21 @@ #!/usr/bin/env python +# -*- coding: utf-8 -*- +# +# Copyright (C) 2015 Matt Martz +# Copyright (C) 2015 Rackspace US, Inc. +# +# This program 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. +# +# This program 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 this program. If not, see . from __future__ import print_function From 71562d83e1f5a83a7e8701b358788c259bba081e Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 29 Apr 2016 09:31:40 -0500 Subject: [PATCH 68/89] Consolidate blackist imports, remove JSON import check. See #23 --- ansible_testing/modules.py | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index f7be1135116..3e3d25fa86b 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -45,6 +45,10 @@ import yaml BLACKLIST_DIRS = frozenset(('.git', 'test', '.github')) INDENT_REGEX = re.compile(r'([\t]*)') BASIC_RESERVED = frozenset((r for r in dir(module_utils_basic) if r[0] != '_')) +BLACKLIST_IMPORTS = { + 'requests': ('requests import found, should use ' + 'ansible.module_utils.urls instead'), +} class Validator(object): @@ -205,24 +209,12 @@ class ModuleValidator(Validator): self.errors.append('indentation contains tabs. line %d ' 'column %d' % (line_no + 1, index)) - def _find_json_import(self): + def _find_blacklist_imports(self): for child in self.ast.body: if isinstance(child, ast.Import): for name in child.names: - if name.name == 'json': - self.warnings.append('JSON import found, ' - 'already provided by ' - 'ansible.module_utils.basic') - - def _find_requests_import(self): - for child in self.ast.body: - if isinstance(child, ast.Import): - for name in child.names: - if name.name == 'requests': - self.errors.append('requests import found, ' - 'should use ' - 'ansible.module_utils.urls ' - 'instead') + if name.name in BLACKLIST_IMPORTS: + self.errors.append(BLACKLIST_IMPORTS[name.name]) elif isinstance(child, ast.TryExcept): bodies = child.body for handler in child.handlers: @@ -230,11 +222,10 @@ class ModuleValidator(Validator): for grandchild in bodies: if isinstance(grandchild, ast.Import): for name in grandchild.names: - if name.name == 'requests': - self.errors.append('requests import found, ' - 'should use ' - 'ansible.module_utils.urls ' - 'instead') + if name.name in BLACKLIST_IMPORTS: + self.errors.append( + BLACKLIST_IMPORTS[name.name] + ) def _find_module_utils(self, main): linenos = [] @@ -560,8 +551,7 @@ class ModuleValidator(Validator): if self._python_module() and not self._just_docs(): self._check_for_sys_exit() - self._find_json_import() - self._find_requests_import() + self._find_blacklist_imports() main = self._find_main_call() self._find_module_utils(main) self._find_has_import() From a90e1c353e31dece07abb34d01143471c27265a9 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 29 Apr 2016 09:33:40 -0500 Subject: [PATCH 69/89] Drop bottom import checking. Fixes #22 --- ansible_testing/modules.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 3e3d25fa86b..1a0e5eae74d 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -115,18 +115,6 @@ class ModuleValidator(Validator): '__init__.py', 'VERSION', 'test-docs.sh')) BLACKLIST = BLACKLIST_FILES.union(BLACKLIST_MODULES) - BOTTOM_IMPORTS = frozenset(( - 'ansible.module_utils.basic', - 'ansible.module_utils.urls', - 'ansible.module_utils.facts', - 'ansible.module_utils.splitter', - 'ansible.module_utils.known_hosts', - 'ansible.module_utils.rax', - )) - BOTTOM_IMPORTS_BLACKLIST = frozenset(( - 'command.py', - )) - PS_DOC_BLACKLIST = frozenset(( 'slurp.ps1', 'setup.ps1' @@ -176,9 +164,6 @@ class ModuleValidator(Validator): except AttributeError: return False - def _is_bottom_import_blacklisted(self): - return self.object_name in self.BOTTOM_IMPORTS_BLACKLIST - def _is_new_module(self): return not module_loader.has_plugin(self.name) @@ -235,12 +220,6 @@ class ModuleValidator(Validator): if child.module.startswith('ansible.module_utils.'): found_module_utils_import = True - if child.module in self.BOTTOM_IMPORTS: - if (child.lineno < main - 10 and - not self._is_bottom_import_blacklisted()): - self.errors.append('%s import not near call to ' - 'main()' % child.module) - linenos.append(child.lineno) if not child.names: From 074661ef0e41a64ab9566fc6aa8c60e0bc17e5bc Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 5 May 2016 10:11:52 -0500 Subject: [PATCH 70/89] Make modules accept multiple paths --- ansible_testing/modules.py | 58 ++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 1a0e5eae74d..bedaafb9a64 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -587,48 +587,50 @@ def re_compile(value): def main(): parser = argparse.ArgumentParser() - parser.add_argument('modules', help='Path to module or module directory') + parser.add_argument('modules', nargs='+', + help='Path to module or module directory') parser.add_argument('-w', '--warnings', help='Show warnings', action='store_true') parser.add_argument('--exclude', help='RegEx exclusion pattern', type=re_compile) args = parser.parse_args() - args.modules = args.modules.rstrip('/') + args.modules[:] = [m.rstrip('/') for m in args.modules] exit = [] # Allow testing against a single file - if os.path.isfile(args.modules): - path = args.modules - if args.exclude and args.exclude.search(path): - sys.exit(0) - mv = ModuleValidator(path) - mv.validate() - exit.append(mv.report(args.warnings)) - sys.exit(sum(exit)) - - for root, dirs, files in os.walk(args.modules): - basedir = root[len(args.modules)+1:].split('/', 1)[0] - if basedir in BLACKLIST_DIRS: - continue - for dirname in dirs: - if root == args.modules and dirname in BLACKLIST_DIRS: - continue - path = os.path.join(root, dirname) - if args.exclude and args.exclude.search(path): - continue - pv = PythonPackageValidator(path) - pv.validate() - exit.append(pv.report(args.warnings)) - - for filename in files: - path = os.path.join(root, filename) + for module in args.modules: + if os.path.isfile(module): + path = module if args.exclude and args.exclude.search(path): - continue + sys.exit(0) mv = ModuleValidator(path) mv.validate() exit.append(mv.report(args.warnings)) + sys.exit(sum(exit)) + + for root, dirs, files in os.walk(module): + basedir = root[len(module)+1:].split('/', 1)[0] + if basedir in BLACKLIST_DIRS: + continue + for dirname in dirs: + if root == module and dirname in BLACKLIST_DIRS: + continue + path = os.path.join(root, dirname) + if args.exclude and args.exclude.search(path): + continue + pv = PythonPackageValidator(path) + pv.validate() + exit.append(pv.report(args.warnings)) + + for filename in files: + path = os.path.join(root, filename) + if args.exclude and args.exclude.search(path): + continue + mv = ModuleValidator(path) + mv.validate() + exit.append(mv.report(args.warnings)) sys.exit(sum(exit)) From 38464b1fdc3b13b1ccb49f3d1e50d1ff8a71aebf Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 5 May 2016 11:01:04 -0500 Subject: [PATCH 71/89] Don't exit after individual file --- ansible_testing/modules.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index bedaafb9a64..cc3d762b5c3 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -599,7 +599,6 @@ def main(): exit = [] - # Allow testing against a single file for module in args.modules: if os.path.isfile(module): path = module @@ -608,7 +607,6 @@ def main(): mv = ModuleValidator(path) mv.validate() exit.append(mv.report(args.warnings)) - sys.exit(sum(exit)) for root, dirs, files in os.walk(module): basedir = root[len(module)+1:].split('/', 1)[0] From b8b3003b29f99d9e9038405463f32678ca9669ea Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 5 May 2016 11:33:19 -0500 Subject: [PATCH 72/89] Rework module_utils detection for zipmodule --- ansible_testing/modules.py | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index cc3d762b5c3..26b12d5f5fa 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -214,31 +214,32 @@ class ModuleValidator(Validator): def _find_module_utils(self, main): linenos = [] + found_basic = False for child in self.ast.body: - found_module_utils_import = False - if isinstance(child, ast.ImportFrom): - if child.module.startswith('ansible.module_utils.'): - found_module_utils_import = True + if isinstance(child, (ast.Import, ast.ImportFrom)): + names = [] + try: + names.append(child.module) + if child.module.endswith('.basic'): + found_basic = True + except AttributeError: + pass + names.extend([n.name for n in child.names]) + if [n for n in names if n.startswith('ansible.module_utils')]: linenos.append(child.lineno) - if not child.names: - self.errors.append('%s: not a "from" import"' % - child.module) - - found_alias = False for name in child.names: - if isinstance(name, ast.alias): - found_alias = True - if name.asname or name.name != '*': - self.errors.append('%s: did not import "*"' % - child.module) - - if found_module_utils_import and not found_alias: - self.errors.append('%s: did not import "*"' % child.module) + print(name.name) + if (isinstance(name, ast.alias) and + name.name == 'basic'): + found_basic = True if not linenos: self.errors.append('Did not find a module_utils import') + elif not found_basic: + self.errors.append('Did not find "ansible.module_utils.basic" ' + 'import') return linenos From 9ce546d03e062574b2d83863a2bec937d71371b0 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 5 May 2016 11:45:26 -0500 Subject: [PATCH 73/89] Don't check for basic.py redeclarations. Fixes #10 --- ansible_testing/modules.py | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 26b12d5f5fa..d2a1f1f57dd 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -31,20 +31,18 @@ from fnmatch import fnmatch from ansible import __version__ as ansible_version from ansible.executor.module_common import REPLACER_WINDOWS -from ansible.module_utils import basic as module_utils_basic from ansible.plugins import module_loader from ansible.utils.module_docs import BLACKLIST_MODULES, get_docstring from schema import doc_schema, option_schema -from utils import CaptureStd, find_globals +from utils import CaptureStd import yaml BLACKLIST_DIRS = frozenset(('.git', 'test', '.github')) INDENT_REGEX = re.compile(r'([\t]*)') -BASIC_RESERVED = frozenset((r for r in dir(module_utils_basic) if r[0] != '_')) BLACKLIST_IMPORTS = { 'requests': ('requests import found, should use ' 'ansible.module_utils.urls instead'), @@ -414,14 +412,6 @@ class ModuleValidator(Validator): e.problem_mark.column + 1)) self.traces.append(e) - def _find_redeclarations(self): - g = set() - find_globals(g, self.ast.body) - redeclared = BASIC_RESERVED.intersection(g) - if redeclared: - self.warnings.append('Redeclared basic.py variable or ' - 'function: %s' % ', '.join(redeclared)) - def _check_version_added(self, doc): if not self._is_new_module(): return @@ -536,7 +526,6 @@ class ModuleValidator(Validator): self._find_module_utils(main) self._find_has_import() self._check_for_tabs() - self._find_redeclarations() if self._powershell_module(): self._find_ps_replacers() From 7cc11e4ad516a5aba0ede50a0ed3c16338e2b017 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 5 May 2016 12:17:18 -0500 Subject: [PATCH 74/89] mark requests and boto as blacklisted imports for new modules. Fixes #21 --- ansible_testing/modules.py | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index d2a1f1f57dd..bc2ef60d575 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -44,8 +44,15 @@ import yaml BLACKLIST_DIRS = frozenset(('.git', 'test', '.github')) INDENT_REGEX = re.compile(r'([\t]*)') BLACKLIST_IMPORTS = { - 'requests': ('requests import found, should use ' - 'ansible.module_utils.urls instead'), + 'requests': { + 'new_only': True, + 'msg': ('requests import found, should use ' + 'ansible.module_utils.urls instead') + }, + 'boto': { + 'new_only': True, + 'msg': ('boto import found, new modules should use boto3') + }, } @@ -194,21 +201,25 @@ class ModuleValidator(Validator): def _find_blacklist_imports(self): for child in self.ast.body: + names = [] if isinstance(child, ast.Import): - for name in child.names: - if name.name in BLACKLIST_IMPORTS: - self.errors.append(BLACKLIST_IMPORTS[name.name]) + names.extend(child.names) elif isinstance(child, ast.TryExcept): bodies = child.body for handler in child.handlers: bodies.extend(handler.body) for grandchild in bodies: if isinstance(grandchild, ast.Import): - for name in grandchild.names: - if name.name in BLACKLIST_IMPORTS: - self.errors.append( - BLACKLIST_IMPORTS[name.name] - ) + names.extend(grandchild.names) + for name in names: + if name.name in BLACKLIST_IMPORTS: + msg = BLACKLIST_IMPORTS[name.name]['msg'] + new_only = BLACKLIST_IMPORTS[name.name]['new_only'] + if self._is_new_module() and new_only: + self.errors.append(msg) + elif not new_only: + self.errors.append(msg) + def _find_module_utils(self, main): linenos = [] @@ -228,7 +239,6 @@ class ModuleValidator(Validator): linenos.append(child.lineno) for name in child.names: - print(name.name) if (isinstance(name, ast.alias) and name.name == 'basic'): found_basic = True From 44fa8c1fb285811e502603c1b22ed6770797be31 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Fri, 6 May 2016 10:37:41 -0500 Subject: [PATCH 75/89] Add ability to analyze the argument_spec for a module --- ansible_testing/module_args.py | 113 +++++++++++++++++++++++++++++++++ ansible_testing/modules.py | 28 ++++++-- 2 files changed, 137 insertions(+), 4 deletions(-) create mode 100644 ansible_testing/module_args.py diff --git a/ansible_testing/module_args.py b/ansible_testing/module_args.py new file mode 100644 index 00000000000..07f3f82a484 --- /dev/null +++ b/ansible_testing/module_args.py @@ -0,0 +1,113 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2016 Matt Martz +# Copyright (C) 2016 Rackspace US, Inc. +# +# This program 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. +# +# This program 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 this program. If not, see . + +import imp +import sys + +from modulefinder import ModuleFinder + +import mock + + +MODULE_CLASSES = [ + 'ansible.module_utils.basic.AnsibleModule', + 'ansible.module_utils.vca.VcaAnsibleModule', + 'ansible.module_utils.nxos.NetworkModule', + 'ansible.module_utils.eos.NetworkModule', + 'ansible.module_utils.ios.NetworkModule', + 'ansible.module_utils.iosxr.NetworkModule', + 'ansible.module_utils.junos.NetworkModule', + 'ansible.module_utils.openswitch.NetworkModule', +] + + +class AnsibleModuleCallError(RuntimeError): + pass + + +def add_mocks(filename): + gp = mock.patch('ansible.module_utils.basic.get_platform').start() + gp.return_value = 'linux' + + module_mock = mock.MagicMock() + mocks = [] + for module_class in MODULE_CLASSES: + mocks.append( + mock.patch('ansible.module_utils.basic.AnsibleModule', + new=module_mock) + ) + for m in mocks: + p = m.start() + p.side_effect = AnsibleModuleCallError() + + finder = ModuleFinder() + try: + finder.run_script(filename) + except: + pass + + sys_mock = mock.MagicMock() + sys_mock.__version__ = '0.0.0' + sys_mocks = [] + for module, sources in finder.badmodules.items(): + if module in sys.modules: + continue + if [s for s in sources if s[:7] in ['ansible', '__main_']]: + parts = module.split('.') + for i in range(len(parts)): + dotted = '.'.join(parts[:i+1]) + sys.modules[dotted] = sys_mock + sys_mocks.append(dotted) + + return module_mock, mocks, sys_mocks + + +def remove_mocks(mocks, sys_mocks): + for m in mocks: + m.stop() + + for m in sys_mocks: + try: + del sys.modules[m] + except KeyError: + pass + + +def get_argument_spec(filename): + module_mock, mocks, sys_mocks = add_mocks(filename) + + try: + mod = imp.load_source('module', filename) + if not module_mock.call_args: + mod.main() + except AnsibleModuleCallError: + pass + except Exception: + # We can probably remove this branch, it is here for use while testing + pass + + remove_mocks(mocks, sys_mocks) + + try: + args, kwargs = module_mock.call_args + try: + return kwargs['argument_spec'] + except KeyError: + return args[0] + except TypeError: + return {} diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index bc2ef60d575..f56e83c2dc4 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -34,6 +34,8 @@ from ansible.executor.module_common import REPLACER_WINDOWS from ansible.plugins import module_loader from ansible.utils.module_docs import BLACKLIST_MODULES, get_docstring +from module_args import get_argument_spec + from schema import doc_schema, option_schema from utils import CaptureStd @@ -125,13 +127,15 @@ class ModuleValidator(Validator): 'setup.ps1' )) - def __init__(self, path): + def __init__(self, path, analyze_arg_spec=False): super(ModuleValidator, self).__init__() self.path = path self.basename = os.path.basename(self.path) self.name, _ = os.path.splitext(self.basename) + self.analyze_arg_spec = analyze_arg_spec + self._python_module_override = False with open(path) as f: @@ -442,6 +446,16 @@ class ModuleValidator(Validator): self.errors.append('version_added should be %s. Currently %s' % (should_be, version_added)) + def _validate_argument_spec(self): + if not self.analyze_arg_spec: + return + spec = get_argument_spec(self.path) + for arg, data in spec.items(): + if data.get('required') and data.get('default', object) != object: + self.errors.append('"%s" is marked as required but specifies ' + 'a default. Arguments with a default ' + 'should not be marked as required' % arg) + def _check_for_new_args(self, doc): if self._is_new_module(): return @@ -530,6 +544,7 @@ class ModuleValidator(Validator): self._validate_docs() if self._python_module() and not self._just_docs(): + self._validate_argument_spec() self._check_for_sys_exit() self._find_blacklist_imports() main = self._find_main_call() @@ -593,6 +608,8 @@ def main(): action='store_true') parser.add_argument('--exclude', help='RegEx exclusion pattern', type=re_compile) + parser.add_argument('--arg-spec', help='Analyze module argument spec', + action='store_true', default=False) args = parser.parse_args() args.modules[:] = [m.rstrip('/') for m in args.modules] @@ -604,7 +621,7 @@ def main(): path = module if args.exclude and args.exclude.search(path): sys.exit(0) - mv = ModuleValidator(path) + mv = ModuleValidator(path, analyze_arg_spec=args.arg_spec) mv.validate() exit.append(mv.report(args.warnings)) @@ -626,7 +643,7 @@ def main(): path = os.path.join(root, filename) if args.exclude and args.exclude.search(path): continue - mv = ModuleValidator(path) + mv = ModuleValidator(path, analyze_arg_spec=args.arg_spec) mv.validate() exit.append(mv.report(args.warnings)) @@ -634,4 +651,7 @@ def main(): if __name__ == '__main__': - main() + try: + main() + except KeyboardInterrupt: + pass From 3c02af64942d96241ec3effc08fd6b27a300bdc7 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 12 May 2016 11:24:28 -0500 Subject: [PATCH 76/89] BLACKLIST_IMPORTS can be regex to making matching easier --- ansible_testing/modules.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index f56e83c2dc4..4833569e323 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -51,7 +51,7 @@ BLACKLIST_IMPORTS = { 'msg': ('requests import found, should use ' 'ansible.module_utils.urls instead') }, - 'boto': { + 'boto(?:\.|$)': { 'new_only': True, 'msg': ('boto import found, new modules should use boto3') }, @@ -216,13 +216,14 @@ class ModuleValidator(Validator): if isinstance(grandchild, ast.Import): names.extend(grandchild.names) for name in names: - if name.name in BLACKLIST_IMPORTS: - msg = BLACKLIST_IMPORTS[name.name]['msg'] - new_only = BLACKLIST_IMPORTS[name.name]['new_only'] - if self._is_new_module() and new_only: - self.errors.append(msg) - elif not new_only: - self.errors.append(msg) + for blacklist_import, options in BLACKLIST_IMPORTS.items(): + if re.search(blacklist_import, name.name): + msg = options['msg'] + new_only = options['new_only'] + if self._is_new_module() and new_only: + self.errors.append(msg) + elif not new_only: + self.errors.append(msg) def _find_module_utils(self, main): From d6ecdfd00ad23f9d8cab340d64a1e53c22e7e4c4 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 12 May 2016 14:10:23 -0700 Subject: [PATCH 77/89] Blacklist __pycache__ directories --- ansible_testing/modules.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 4833569e323..7f222e20ec3 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -563,6 +563,8 @@ class ModuleValidator(Validator): class PythonPackageValidator(Validator): + BLACKLIST_FILES = frozenset(('__pycache__',)) + def __init__(self, path): super(PythonPackageValidator, self).__init__() @@ -580,6 +582,9 @@ class PythonPackageValidator(Validator): def validate(self): super(PythonPackageValidator, self).validate() + if self.basename in self.BLACKLIST_FILES: + return + init_file = os.path.join(self.path, '__init__.py') if not os.path.exists(init_file): self.errors.append('Ansible module subdirectories must contain an ' From aeb064520eddd0b663d38368d19f626f6ee3dd14 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Sun, 15 May 2016 18:03:06 -0500 Subject: [PATCH 78/89] When checking option version_added, if unexpected exception, continue --- ansible_testing/modules.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 7f222e20ec3..38b4167bff6 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -504,6 +504,11 @@ class ModuleValidator(Validator): 'is not a valid version number: %r' % (option, version_added)) continue + except: + # If there is any other exception it should have been caught + # in schema validation, so we won't duplicate errors by + # listing it again + continue if (strict_ansible_version != mod_version_added and (version_added < strict_ansible_version or From d777e217e7ec6fcec2e2e108a858dd1260f2c1e0 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Sat, 21 May 2016 10:51:29 -0500 Subject: [PATCH 79/89] Issues with pre-existing docs should not cause errors, only warnings --- ansible_testing/modules.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 38b4167bff6..707387f0231 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -72,6 +72,7 @@ class Validator(object): self.errors = [] self.warnings = [] self.traces = [] + self.warning_traces = [] @abc.abstractproperty def object_name(self): @@ -98,7 +99,11 @@ class Validator(object): ret = [] - for trace in self.traces: + traces = self.traces[:] + if warnings and self.warnings: + traces.extend(self.warning_traces) + + for trace in traces: print('TRACE:') print('\n '.join((' %s' % trace).splitlines())) for error in self.errors: @@ -225,7 +230,6 @@ class ModuleValidator(Validator): elif not new_only: self.errors.append(msg) - def _find_module_utils(self, main): linenos = [] found_basic = False @@ -468,13 +472,14 @@ class ModuleValidator(Validator): existing_options = existing_doc.get('options', {}) except AssertionError: fragment = doc['extends_documentation_fragment'] - self.errors.append('Existing DOCUMENTATION fragment missing: ' - '%s' % fragment) + self.warnings.append('Pre-existing DOCUMENTATION fragment ' + 'missing: %s' % fragment) return except Exception as e: - self.traces.append(e) - self.errors.append('Unknown existing DOCUMENTATION error, see ' - 'TRACE') + self.warning_traces.append(e) + self.warnings.append('Unknown pre-existing DOCUMENTATION ' + 'error, see TRACE. Submodule refs may ' + 'need updated') return try: From 43c028d52d2938f1732bd82afab345edc6937830 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 31 May 2016 10:40:11 -0500 Subject: [PATCH 80/89] options can be None, don't iterate None --- ansible_testing/modules.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 707387f0231..9d786441443 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -359,7 +359,8 @@ class ModuleValidator(Validator): except Exception as e: errors.extend(e.errors) - for key, option in doc.get('options', {}).iteritems(): + options = doc.get('options', {}) + for key, option in (options or {}).iteritems(): try: option_schema(option) except Exception as e: From 247066e6a899529328ae2d0aae4584c26186ae34 Mon Sep 17 00:00:00 2001 From: John R Barker Date: Wed, 6 Jul 2016 18:52:51 +0100 Subject: [PATCH 81/89] Report missing ansible.module_utils.basic import as a warning (#31) For new Networking modules we import via another library See sivel/ansible-testing/issue/30 --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 9d786441443..3eb711b228b 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -255,7 +255,7 @@ class ModuleValidator(Validator): if not linenos: self.errors.append('Did not find a module_utils import') elif not found_basic: - self.errors.append('Did not find "ansible.module_utils.basic" ' + self.warnings.append('Did not find "ansible.module_utils.basic" ' 'import') return linenos From 5d342050a6f7fcac79f46cafc79a6ca2b528e1fd Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Wed, 6 Jul 2016 21:50:56 -0700 Subject: [PATCH 82/89] Add shippable.yml to blacklist. (#32) --- ansible_testing/modules.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 3eb711b228b..4cbe8353967 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -123,6 +123,7 @@ class Validator(object): class ModuleValidator(Validator): BLACKLIST_PATTERNS = ('.git*', '*.pyc', '*.pyo', '.*', '*.md', '*.txt') BLACKLIST_FILES = frozenset(('.git', '.gitignore', '.travis.yml', + 'shippable.yml', '.gitattributes', '.gitmodules', 'COPYING', '__init__.py', 'VERSION', 'test-docs.sh')) BLACKLIST = BLACKLIST_FILES.union(BLACKLIST_MODULES) From 54118d45daf0aa9d62cd5ee7790335e9529df38f Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Fri, 15 Jul 2016 13:03:45 -0700 Subject: [PATCH 83/89] Add blacklist/ignore for .idea dir. --- ansible_testing/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index 4cbe8353967..fb7ad0aaca1 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -43,7 +43,7 @@ from utils import CaptureStd import yaml -BLACKLIST_DIRS = frozenset(('.git', 'test', '.github')) +BLACKLIST_DIRS = frozenset(('.git', 'test', '.github', '.idea')) INDENT_REGEX = re.compile(r'([\t]*)') BLACKLIST_IMPORTS = { 'requests': { From 3eec84b69a0b4030160af0f5314bed350e60ae38 Mon Sep 17 00:00:00 2001 From: nitzmahone Date: Tue, 6 Sep 2016 09:59:20 -0700 Subject: [PATCH 84/89] add async_status to PS module doc blacklist --- ansible_testing/modules.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index fb7ad0aaca1..adef93965f9 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -129,6 +129,7 @@ class ModuleValidator(Validator): BLACKLIST = BLACKLIST_FILES.union(BLACKLIST_MODULES) PS_DOC_BLACKLIST = frozenset(( + 'async_status.ps1', 'slurp.ps1', 'setup.ps1' )) From f883b33441c7ef50c2c3c12619cd3f86914c3fcc Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Mon, 19 Sep 2016 14:54:07 -0400 Subject: [PATCH 85/89] short_description is just basestring (#35) As of ansible/ansible 883f4511580761c01d3b39b20b6fb4739c94a7fa 'short_description' value is expected to only be a string. This should catch issues like https://github.com/ansible/ansible/issues/17634 --- ansible_testing/schema.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible_testing/schema.py b/ansible_testing/schema.py index 6a078306ab4..dbe606c6485 100644 --- a/ansible_testing/schema.py +++ b/ansible_testing/schema.py @@ -32,7 +32,7 @@ option_schema = Schema( doc_schema = Schema( { Required('module'): basestring, - 'short_description': Any(basestring, [basestring]), + 'short_description': basestring, 'description': Any(basestring, [basestring]), 'version_added': Any(basestring, float), 'author': Any(None, basestring, [basestring]), From 60e8cf9aa79e1f0508d27e9e9ce90571216b84b3 Mon Sep 17 00:00:00 2001 From: John R Barker Date: Thu, 22 Sep 2016 20:49:57 +0100 Subject: [PATCH 86/89] version_added for deprecated modules (#36) * version_added for deprecated modules Modules are deprecated by renaming so they start with "_". This means we will not find an existing module with that name, so look up the original name, i.e. without the leading '_'. * Deal with aliased/symlinked modules --- ansible_testing/modules.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ansible_testing/modules.py b/ansible_testing/modules.py index adef93965f9..82889e020a2 100644 --- a/ansible_testing/modules.py +++ b/ansible_testing/modules.py @@ -181,7 +181,11 @@ class ModuleValidator(Validator): return False def _is_new_module(self): - return not module_loader.has_plugin(self.name) + if self.name.startswith("_") and not os.path.islink(self.name): + # This is a deprecated module, so look up the *old* name + return not module_loader.has_plugin(self.name[1:]) + else: + return not module_loader.has_plugin(self.name) def _check_interpreter(self, powershell=False): if powershell: From ef06b5501cce5d84a577109158fca88be04afc30 Mon Sep 17 00:00:00 2001 From: John Barker Date: Thu, 13 Oct 2016 14:36:22 +0100 Subject: [PATCH 87/89] Port sivel/ansible-validate-modules into Ansible --- lib/ansible/modules/core | 2 +- lib/ansible/modules/extras | 2 +- test/sanity/validate-modules/README.rst | 77 +++++++++++++++++++ .../sanity/validate-modules}/__init__.py | 0 .../sanity/validate-modules}/module_args.py | 0 .../sanity/validate-modules}/schema.py | 0 .../sanity/validate-modules}/utils.py | 0 .../sanity/validate-modules/validate-modules | 0 8 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 test/sanity/validate-modules/README.rst rename {ansible_testing => test/sanity/validate-modules}/__init__.py (100%) rename {ansible_testing => test/sanity/validate-modules}/module_args.py (100%) rename {ansible_testing => test/sanity/validate-modules}/schema.py (100%) rename {ansible_testing => test/sanity/validate-modules}/utils.py (100%) rename ansible_testing/modules.py => test/sanity/validate-modules/validate-modules (100%) mode change 100644 => 100755 diff --git a/lib/ansible/modules/core b/lib/ansible/modules/core index 275fa3f055a..149f10f8b7f 160000 --- a/lib/ansible/modules/core +++ b/lib/ansible/modules/core @@ -1 +1 @@ -Subproject commit 275fa3f055aacf2286eed54c13bde17db5082035 +Subproject commit 149f10f8b7f0dfe669b3c5ef26c2f4107f589e61 diff --git a/lib/ansible/modules/extras b/lib/ansible/modules/extras index 3e1ea76a75c..cc2651422ad 160000 --- a/lib/ansible/modules/extras +++ b/lib/ansible/modules/extras @@ -1 +1 @@ -Subproject commit 3e1ea76a75c297e2c17a4cd0e7856ca82139c761 +Subproject commit cc2651422ad6c7971fa48a45d2e16ac297ededd6 diff --git a/test/sanity/validate-modules/README.rst b/test/sanity/validate-modules/README.rst new file mode 100644 index 00000000000..026aab85f9f --- /dev/null +++ b/test/sanity/validate-modules/README.rst @@ -0,0 +1,77 @@ +ansible-testing +=============== + +Python module to help test or validate Ansible, specifically ansible +modules + +Installation +------------ + +This module must be installed alongside the current development +release of Ansible to appropriately test the current development +state of modules. + +Usage +~~~~~ + +:: + + pip install git+https://github.com/ansible/ansible.git@devel#egg=ansible + pip install git+https://github.com/sivel/ansible-testing.git#egg=ansible_testing + ansible-validate-modules /path/to/ansible-modules-extras + +Help +~~~~ + +:: + + usage: ansible-validate-modules [-h] [-w] [--exclude EXCLUDE] modules + + positional arguments: + modules Path to module or module directory + + optional arguments: + -h, --help show this help message and exit + -w, --warnings Show warnings + --exclude EXCLUDE RegEx exclusion pattern + +Current Validations +------------------- + +Modules +~~~~~~~ + +Errors +^^^^^^ + +#. Interpreter line is not ``#!/usr/bin/python`` +#. ``main()`` not at the bottom of the file +#. Module does not import ``ansible.module_utils.basic`` +#. Missing ``DOCUMENTATION`` +#. Documentation is invalid YAML +#. Invalid schema for ``DOCUMENTATION`` +#. Missing ``EXAMPLES`` +#. Invalid Python Syntax +#. Tabbed indentation +#. Use of ``sys.exit()`` instead of ``exit_json`` or ``fail_json`` +#. Missing GPLv3 license header in module +#. Powershell module missing ``WANT_JSON`` +#. Powershell module missing ``POWERSHELL_COMMON`` +#. New modules have the correct ``version_added`` +#. New arguments have the correct ``version_added`` +#. Modules should not import requests, instead use ``ansible.module_utils.urls`` +#. Missing ``RETURN`` for new modules + +Warnings +^^^^^^^^ + +#. Try/Except ``HAS_`` expression missing +#. Missing ``RETURN`` for existing modules +#. ``import json`` found +#. Module contains duplicate globals from basic.py + +Module Directories (Python Packages) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +#. Missing ``__init__.py`` + diff --git a/ansible_testing/__init__.py b/test/sanity/validate-modules/__init__.py similarity index 100% rename from ansible_testing/__init__.py rename to test/sanity/validate-modules/__init__.py diff --git a/ansible_testing/module_args.py b/test/sanity/validate-modules/module_args.py similarity index 100% rename from ansible_testing/module_args.py rename to test/sanity/validate-modules/module_args.py diff --git a/ansible_testing/schema.py b/test/sanity/validate-modules/schema.py similarity index 100% rename from ansible_testing/schema.py rename to test/sanity/validate-modules/schema.py diff --git a/ansible_testing/utils.py b/test/sanity/validate-modules/utils.py similarity index 100% rename from ansible_testing/utils.py rename to test/sanity/validate-modules/utils.py diff --git a/ansible_testing/modules.py b/test/sanity/validate-modules/validate-modules old mode 100644 new mode 100755 similarity index 100% rename from ansible_testing/modules.py rename to test/sanity/validate-modules/validate-modules From 25286c3c7d1b9818d450ec13fa411da8681ba042 Mon Sep 17 00:00:00 2001 From: John Barker Date: Thu, 13 Oct 2016 15:52:03 +0100 Subject: [PATCH 88/89] README.rst to reference new name --- test/sanity/validate-modules/README.rst | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/test/sanity/validate-modules/README.rst b/test/sanity/validate-modules/README.rst index 026aab85f9f..aab8ee5eb1e 100644 --- a/test/sanity/validate-modules/README.rst +++ b/test/sanity/validate-modules/README.rst @@ -1,31 +1,26 @@ -ansible-testing +validate-modules =============== -Python module to help test or validate Ansible, specifically ansible -modules +Python program to help test or validate Ansible modules. -Installation ------------- -This module must be installed alongside the current development -release of Ansible to appropriately test the current development -state of modules. +Originally developed by Matt Martz (@sivel) Usage ~~~~~ :: + # If you are running Ansible from source + source /path/to/ansible/hacking/env-setup - pip install git+https://github.com/ansible/ansible.git@devel#egg=ansible - pip install git+https://github.com/sivel/ansible-testing.git#egg=ansible_testing - ansible-validate-modules /path/to/ansible-modules-extras + validate-modules /path/to/ansible-modules-extras Help ~~~~ :: - usage: ansible-validate-modules [-h] [-w] [--exclude EXCLUDE] modules + usage: validate-modules [-h] [-w] [--exclude EXCLUDE] modules positional arguments: modules Path to module or module directory From 2445ad72dedd7045d5b020549b37b186419c8c82 Mon Sep 17 00:00:00 2001 From: John Barker Date: Thu, 13 Oct 2016 15:53:40 +0100 Subject: [PATCH 89/89] Don't update submodules --- lib/ansible/modules/core | 2 +- lib/ansible/modules/extras | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/core b/lib/ansible/modules/core index 149f10f8b7f..275fa3f055a 160000 --- a/lib/ansible/modules/core +++ b/lib/ansible/modules/core @@ -1 +1 @@ -Subproject commit 149f10f8b7f0dfe669b3c5ef26c2f4107f589e61 +Subproject commit 275fa3f055aacf2286eed54c13bde17db5082035 diff --git a/lib/ansible/modules/extras b/lib/ansible/modules/extras index cc2651422ad..3e1ea76a75c 160000 --- a/lib/ansible/modules/extras +++ b/lib/ansible/modules/extras @@ -1 +1 @@ -Subproject commit cc2651422ad6c7971fa48a45d2e16ac297ededd6 +Subproject commit 3e1ea76a75c297e2c17a4cd0e7856ca82139c761