Add Python 3 support for validate-modules test. (#21195)

* Improve PEP8 compatibility

* Fix Python 3 incompatibility

Is prohibited to mutate OrderedDict during iteration through it so
is better to add records with error or warning to empty dictionary
instead of delete records from copy of dictionary during iterating.

* Decode output of subprocess from bytes to unicode.

* Add Python 3 support for validate-modules test.

Fix #18367

test/sanity/code-smell/shebang.sh

@@ -7,7 +7,6 @@ grep '^#!' -rIn . \
     -e '^\./lib/ansible/modules/' \
     -e '^\./test/integration/targets/[^/]*/library/[^/]*:#!powershell$' \
     -e '^\./test/integration/targets/[^/]*/library/[^/]*:#!/usr/bin/python$' \
-    -e '^\./test/sanity/validate-modules/validate-modules:#!/usr/bin/env python2$' \
     -e '^\./hacking/cherrypick.py:#!/usr/bin/env python3$' \
     -e ':#!/bin/sh$' \
     -e ':#!/bin/bash( -[eux]|$)' \

test/sanity/validate-modules/schema.py

@@ -17,52 +17,62 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from voluptuous import PREVENT_EXTRA, Any, Required, Schema
+from ansible.module_utils.six import string_types
+list_string_types = list(string_types)
 
 suboption_schema = Schema(
     {
-        Required('description'): Any(basestring, [basestring]),
+        Required('description'): Any(list_string_types, *string_types),
         'required': bool,
         'choices': list,
-        'aliases': Any(basestring, list),
-        'version_added': Any(basestring, float),
-        'default': Any(None, basestring, float, int, bool, list, dict),
+        'aliases': Any(list, *string_types),
+        'version_added': Any(float, *string_types),
+        'default': Any(None, float, int, bool, list, dict, *string_types),
         # Note: Types are strings, not literal bools, such as True or False
         'type': Any(None, "bool")
     },
     extra=PREVENT_EXTRA
 )
 
+# This generates list of dicts with keys from string_types and suboption_schema value
+# for example in Python 3: {str: suboption_schema}
+list_dict_suboption_schema = [{str_type: suboption_schema} for str_type in string_types]
+
 option_schema = Schema(
     {
-        Required('description'): Any(basestring, [basestring]),
+        Required('description'): Any(list_string_types, *string_types),
         'required': bool,
         'choices': list,
-        'aliases': Any(basestring, list),
-        'version_added': Any(basestring, float),
-        'default': Any(None, basestring, float, int, bool, list, dict),
-        'suboptions': Any(None, {basestring: suboption_schema,}),
+        'aliases': Any(list, *string_types),
+        'version_added': Any(float, *string_types),
+        'default': Any(None, float, int, bool, list, dict, *string_types),
+        'suboptions': Any(None, *list_dict_suboption_schema),
         # Note: Types are strings, not literal bools, such as True or False
         'type': Any(None, "bool")
     },
     extra=PREVENT_EXTRA
 )
 
+# This generates list of dicts with keys from string_types and option_schema value
+# for example in Python 3: {str: option_schema}
+list_dict_option_schema = [{str_type: option_schema} for str_type in string_types]
+
 def doc_schema(module_name):
     if module_name.startswith('_'):
         module_name = module_name[1:]
     return Schema(
         {
             Required('module'): module_name,
-            'deprecated': basestring,
-            Required('short_description'): basestring,
-            Required('description'): Any(basestring, [basestring]),
-            Required('version_added'): Any(basestring, float),
-            Required('author'): Any(None, basestring, [basestring]),
-            'notes': Any(None, [basestring]),
-            'requirements': [basestring],
-            'todo': Any(None, basestring, [basestring]),
-            'options': Any(None, {basestring: option_schema}),
-            'extends_documentation_fragment': Any(basestring, [basestring])
+            'deprecated': Any(*string_types),
+            Required('short_description'): Any(*string_types),
+            Required('description'): Any(list_string_types, *string_types),
+            Required('version_added'): Any(float, *string_types),
+            Required('author'): Any(None, list_string_types, *string_types),
+            'notes': Any(None, list_string_types),
+            'requirements': list_string_types,
+            'todo': Any(None, list_string_types, *string_types),
+            'options': Any(None, *list_dict_option_schema),
+            'extends_documentation_fragment': Any(list_string_types, *string_types)
         },
         extra=PREVENT_EXTRA
     )

test/sanity/validate-modules/utils.py

@@ -19,8 +19,7 @@
 import ast
 import sys
 
-# We only use StringIO, since we cannot setattr on cStringIO
-from StringIO import StringIO
+from io import BytesIO, TextIOWrapper
 
 import yaml
 import yaml.reader
@@ -55,10 +54,8 @@ class CaptureStd():
     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)
+        sys.stdout = self.stdout = TextIOWrapper(BytesIO(), encoding=self.sys_stdout.encoding)
+        sys.stderr = self.stderr = TextIOWrapper(BytesIO(), encoding=self.sys_stderr.encoding)
         return self
 
     def __exit__(self, exc_type, exc_value, traceback):

test/sanity/validate-modules/validate-modules

@@ -1,4 +1,4 @@
-#!/usr/bin/env python2
+#!/usr/bin/env python
 # -*- coding: utf-8 -*-
 #
 # Copyright (C) 2015 Matt Martz <matt@sivel.net>
@@ -46,6 +46,16 @@ from schema import doc_schema, option_schema, metadata_schema
 from utils import CaptureStd, parse_yaml
 from voluptuous.humanize import humanize_error
 
+from ansible.module_utils.six import PY3, with_metaclass
+
+if PY3:
+    # Because there is no ast.TryExcept in Python 3 ast module
+    TRY_EXCEPT = ast.Try
+    # REPLACER_WINDOWS from ansible.executor.module_common is byte
+    # string but we need unicode for Python 3
+    REPLACER_WINDOWS = REPLACER_WINDOWS.decode('utf-8')
+else:
+    TRY_EXCEPT = ast.TryExcept
 
 BLACKLIST_DIRS = frozenset(('.git', 'test', '.github', '.idea'))
 INDENT_REGEX = re.compile(r'([\t]*)')
@@ -78,6 +88,7 @@ class ReporterEncoder(json.JSONEncoder):
 
 
 class Reporter(object):
+
     @staticmethod
     @contextmanager
     def _output_handle(output):
@@ -93,10 +104,10 @@ class Reporter(object):
 
     @staticmethod
     def _filter_out_ok(reports):
-        temp_reports = reports.copy()
-        for path, report in temp_reports.items():
-            if not (report['errors'] or report['warnings']):
-                del temp_reports[path]
+        temp_reports = OrderedDict()
+        for path, report in reports.items():
+            if report['errors'] or report['warnings']:
+                temp_reports[path] = report
 
         return temp_reports
 
@@ -148,11 +159,10 @@ class Reporter(object):
         return 3 if sum(ret) else 0
 
 
-class Validator(object):
+class Validator(with_metaclass(abc.ABCMeta, 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()
@@ -352,7 +362,7 @@ class ModuleValidator(Validator):
             names = []
             if isinstance(child, ast.Import):
                 names.extend(child.names)
-            elif isinstance(child, ast.TryExcept):
+            elif isinstance(child, TRY_EXCEPT):
                 bodies = child.body
                 for handler in child.handlers:
                     bodies.extend(handler.body)
@@ -459,7 +469,7 @@ class ModuleValidator(Validator):
         for child in self.ast.body:
             found_try_except_import = False
             found_has = False
-            if isinstance(child, ast.TryExcept):
+            if isinstance(child, TRY_EXCEPT):
                 bodies = child.body
                 for handler in child.handlers:
                     bodies.extend(handler.body)
@@ -498,7 +508,7 @@ class ModuleValidator(Validator):
                          'line %d' % (child.lineno,))
                     ))
                     break
-            elif isinstance(child, ast.TryExcept):
+            elif isinstance(child, TRY_EXCEPT):
                 bodies = child.body
                 for handler in child.handlers:
                     bodies.extend(handler.body)
@@ -970,7 +980,7 @@ def main():
                 reports.update(mv.report())
 
         for root, dirs, files in os.walk(module):
-            basedir = root[len(module)+1:].split('/', 1)[0]
+            basedir = root[len(module) + 1:].split('/', 1)[0]
             if basedir in BLACKLIST_DIRS:
                 continue
             for dirname in dirs:
@@ -1027,7 +1037,7 @@ class GitCache(object):
         cmd = ['git'] + args
         p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         stdout, stderr = p.communicate()
-        return stdout.splitlines()
+        return stdout.decode('utf-8').splitlines()
 
 
 if __name__ == '__main__':