Minor cleanup of code-smell tests. (#45658)

* Minor cleanup of code-smell tests.
* Add exception handling for YAML load.

test/sanity/code-smell/botmeta.json

@@ -1,4 +1,4 @@
 {
     "always": true,
-    "output": "path-message"
+    "output": "path-line-column-message"
 }

test/sanity/code-smell/botmeta.py

@@ -1,8 +1,9 @@
 #!/usr/bin/env python
 """Make sure the data in BOTMETA.yml is valid"""
 
-import os.path
 import glob
+import os
+import re
 import sys
 import yaml
 
@@ -10,15 +11,23 @@ from voluptuous import Any, MultipleInvalid, Required, Schema
 from voluptuous.humanize import humanize_error
 
 from ansible.module_utils.six import string_types
+
 list_string_types = list(string_types)
 
 
 def main():
     """Validate BOTMETA"""
     path = '.github/BOTMETA.yml'
-    with open(path, 'r') as f_path:
+
-        botmeta = yaml.safe_load(f_path)
+    try:
-    f_path.close()
+        with open(path, 'r') as f_path:
+            botmeta = yaml.safe_load(f_path)
+    except yaml.error.MarkedYAMLError as ex:
+        print('%s:%d:%d: YAML load failed: %s' % (path, ex.context_mark.line + 1, ex.context_mark.column + 1, re.sub(r'\s+', ' ', str(ex))))
+        sys.exit()
+    except Exception as ex:
+        print('%s:%d:%d: YAML load failed: %s' % (path, 0, 0, re.sub(r'\s+', ' ', str(ex))))
+        sys.exit()
 
     files_schema = Any(
         Schema(*string_types),
@@ -48,24 +57,27 @@ def main():
     except MultipleInvalid as ex:
         for error in ex.errors:
             # No way to get line numbers
-            print('%s: %s' % (path, humanize_error(botmeta, error)))
+            print('%s:%d:%d: %s' % (path, 0, 0, humanize_error(botmeta, error)))
 
     # We have two macros to define locations, ensure they haven't been removed
-    if botmeta['macros']['module_utils'] != 'lib/ansible/module_utils':
+    module_utils_path = botmeta.get('macros', {}).get('module_utils', '')
-        print('%s: [macros][module_utils] has been removed' % path)
+    modules_path = botmeta.get('macros', {}).get('modules', '')
 
-    if botmeta['macros']['modules'] != 'lib/ansible/modules':
+    if module_utils_path != 'lib/ansible/module_utils':
-        print('%s: [macros][modules] has been removed' % path)
+        print('%s:%d:%d: [macros][module_utils] has been changed or removed' % (path, 0, 0))
+
+    if modules_path != 'lib/ansible/modules':
+        print('%s:%d:%d: [macros][modules] has been changed or removed' % (path, 0, 0))
 
     # See if all `files:` are valid
     for file in botmeta['files']:
-        file = file.replace('$module_utils', botmeta['macros']['module_utils'])
+        file = file.replace('$module_utils', module_utils_path)
-        file = file.replace('$modules', botmeta['macros']['modules'])
+        file = file.replace('$modules', modules_path)
         if not os.path.exists(file):
             # Not a file or directory, though maybe the prefix to one?
             # https://github.com/ansible/ansibullbot/pull/1023
             if not glob.glob('%s*' % file):
-                print("%s: Can't find '%s.*' in this branch" % (path, file))
+                print("%s:%d:%d: Can't find '%s.*' in this branch" % (path, 0, 0, file))
 
 
 if __name__ == '__main__':

test/sanity/code-smell/no-illegal-filenames.py

@@ -53,8 +53,8 @@ ILLEGAL_END_CHARS = [
 ]
 
 
-def check_path(path, dir=False):
+def check_path(path, is_dir=False):
-    type_name = 'directory' if dir else 'file'
+    type_name = 'directory' if is_dir else 'file'
     parent, file_name = os.path.split(path)
     name, ext = os.path.splitext(file_name)
 
@@ -85,10 +85,10 @@ def main():
             continue
 
         for dir_name in dirs:
-            check_path(os.path.join(root, dir_name), dir=True)
+            check_path(os.path.join(root, dir_name), is_dir=True)
 
         for file_name in files:
-            check_path(os.path.join(root, file_name), dir=False)
+            check_path(os.path.join(root, file_name), is_dir=False)
 
 
 if __name__ == '__main__':

test/sanity/code-smell/no-tests-as-filters.py

@@ -45,7 +45,7 @@ TEST_MAP = {
 }
 
 
-FILTER_RE = re.compile(r'((.+?)\s*(?P<left>[\w \.\'"]+)(\s*)\|(\s*)(?P<filter>\w+))')
+FILTER_RE = re.compile(r'((.+?)\s*(?P<left>[\w .\'"]+)(\s*)\|(\s*)(?P<filter>\w+))')
 
 
 def main():

test/sanity/code-smell/no-underscore-variable.py

@@ -129,7 +129,7 @@ def main():
 
         with open(path, 'r') as path_fd:
             for line, text in enumerate(path_fd.readlines()):
-                match = re.search(r'(?: |[^C]\()(_)(?: |,|\))', text)
+                match = re.search(r'(?: |[^C]\()(_)(?:[ ,)])', text)
 
                 if match:
                     print('%s:%d:%d: use `dummy` instead of `_` for a variable name' % (