From 5a3000af19b81c1baf592e970683c7fd0ac0d43f Mon Sep 17 00:00:00 2001
From: Matt Clay <matt@mystile.com>
Date: Wed, 19 Sep 2018 23:20:27 -0700
Subject: [PATCH] Support comments in ansible-test flat files.

---
 test/runner/lib/cli.py                     | 16 +++----
 test/runner/lib/sanity/__init__.py         |  5 ++-
 test/runner/lib/sanity/ansible_doc.py      |  5 ++-
 test/runner/lib/sanity/compile.py          |  8 ++--
 test/runner/lib/sanity/import.py           |  8 ++--
 test/runner/lib/sanity/pep8.py             | 22 ++++++----
 test/runner/lib/sanity/pslint.py           | 37 +++++++++-------
 test/runner/lib/sanity/pylint.py           | 51 ++++++++++++----------
 test/runner/lib/sanity/rstcheck.py         |  5 ++-
 test/runner/lib/sanity/shellcheck.py       |  9 ++--
 test/runner/lib/sanity/validate_modules.py | 34 +++++++++------
 test/runner/lib/target.py                  |  5 ++-
 test/runner/lib/util.py                    | 20 ++++++++-
 13 files changed, 132 insertions(+), 93 deletions(-)

diff --git a/test/runner/lib/cli.py b/test/runner/lib/cli.py
index 4f404fe91f4..b32887807dc 100644
--- a/test/runner/lib/cli.py
+++ b/test/runner/lib/cli.py
@@ -12,6 +12,7 @@ from lib.util import (
     raw_command,
     get_docker_completion,
     generate_pip_command,
+    read_lines_without_comments,
 )
 
 from lib.delegation import (
@@ -687,8 +688,7 @@ def complete_remote(prefix, parsed_args, **_):
     """
     del parsed_args
 
-    with open('test/runner/completion/remote.txt', 'r') as completion_fd:
-        images = completion_fd.read().splitlines()
+    images = read_lines_without_comments('test/runner/completion/remote.txt', remove_blank_lines=True)
 
     return [i for i in images if i.startswith(prefix)]
 
@@ -701,12 +701,10 @@ def complete_remote_shell(prefix, parsed_args, **_):
     """
     del parsed_args
 
-    with open('test/runner/completion/remote.txt', 'r') as completion_fd:
-        images = completion_fd.read().splitlines()
+    images = read_lines_without_comments('test/runner/completion/remote.txt', remove_blank_lines=True)
 
     # 2008 doesn't support SSH so we do not add to the list of valid images
-    with open('test/runner/completion/windows.txt', 'r') as completion_fd:
-        images.extend(["windows/%s" % i for i in completion_fd.read().splitlines() if i != '2008'])
+    images.extend(["windows/%s" % i for i in read_lines_without_comments('test/runner/completion/windows.txt', remove_blank_lines=True) if i != '2008'])
 
     return [i for i in images if i.startswith(prefix)]
 
@@ -730,8 +728,7 @@ def complete_windows(prefix, parsed_args, **_):
     :type parsed_args: any
     :rtype: list[str]
     """
-    with open('test/runner/completion/windows.txt', 'r') as completion_fd:
-        images = completion_fd.read().splitlines()
+    images = read_lines_without_comments('test/runner/completion/windows.txt', remove_blank_lines=True)
 
     return [i for i in images if i.startswith(prefix) and (not parsed_args.windows or i not in parsed_args.windows)]
 
@@ -742,8 +739,7 @@ def complete_network_platform(prefix, parsed_args, **_):
     :type parsed_args: any
     :rtype: list[str]
     """
-    with open('test/runner/completion/network.txt', 'r') as completion_fd:
-        images = completion_fd.read().splitlines()
+    images = read_lines_without_comments('test/runner/completion/network.txt', remove_blank_lines=True)
 
     return [i for i in images if i.startswith(prefix) and (not parsed_args.platform or i not in parsed_args.platform)]
 
diff --git a/test/runner/lib/sanity/__init__.py b/test/runner/lib/sanity/__init__.py
index 57d237cf4ad..9f8c9b8f113 100644
--- a/test/runner/lib/sanity/__init__.py
+++ b/test/runner/lib/sanity/__init__.py
@@ -18,6 +18,7 @@ from lib.util import (
     parse_to_dict,
     ABC,
     is_binary_file,
+    read_lines_without_comments,
 )
 
 from lib.ansible_util import (
@@ -134,8 +135,8 @@ def collect_code_smell_tests():
     """
     :rtype: tuple[SanityCodeSmellTest]
     """
-    with open('test/sanity/code-smell/skip.txt', 'r') as skip_fd:
-        skip_tests = skip_fd.read().splitlines()
+    skip_file = 'test/sanity/code-smell/skip.txt'
+    skip_tests = read_lines_without_comments(skip_file, remove_blank_lines=True)
 
     paths = glob.glob('test/sanity/code-smell/*')
     paths = sorted(p for p in paths if os.access(p, os.X_OK) and os.path.isfile(p) and os.path.basename(p) not in skip_tests)
diff --git a/test/runner/lib/sanity/ansible_doc.py b/test/runner/lib/sanity/ansible_doc.py
index 2e5c73e8092..a3b35d93e69 100644
--- a/test/runner/lib/sanity/ansible_doc.py
+++ b/test/runner/lib/sanity/ansible_doc.py
@@ -17,6 +17,7 @@ from lib.util import (
     SubprocessError,
     display,
     intercept_command,
+    read_lines_without_comments,
 )
 
 from lib.ansible_util import (
@@ -37,8 +38,8 @@ class AnsibleDocTest(SanityMultipleVersion):
         :type python_version: str
         :rtype: TestResult
         """
-        with open('test/sanity/ansible-doc/skip.txt', 'r') as skip_fd:
-            skip_modules = set(skip_fd.read().splitlines())
+        skip_file = 'test/sanity/ansible-doc/skip.txt'
+        skip_modules = set(read_lines_without_comments(skip_file, remove_blank_lines=True))
 
         plugin_type_blacklist = set([
             # not supported by ansible-doc
diff --git a/test/runner/lib/sanity/compile.py b/test/runner/lib/sanity/compile.py
index 9427fe73a07..b2ecb8f86ba 100644
--- a/test/runner/lib/sanity/compile.py
+++ b/test/runner/lib/sanity/compile.py
@@ -17,6 +17,7 @@ from lib.util import (
     run_command,
     display,
     find_python,
+    read_lines_without_comments,
 )
 
 from lib.config import (
@@ -37,12 +38,10 @@ class CompileTest(SanityMultipleVersion):
         :type python_version: str
         :rtype: TestResult
         """
-        # optional list of regex patterns to exclude from tests
         skip_file = 'test/sanity/compile/python%s-skip.txt' % python_version
 
         if os.path.exists(skip_file):
-            with open(skip_file, 'r') as skip_fd:
-                skip_paths = skip_fd.read().splitlines()
+            skip_paths = read_lines_without_comments(skip_file)
         else:
             skip_paths = []
 
@@ -87,6 +86,9 @@ class CompileTest(SanityMultipleVersion):
         for path in skip_paths:
             line += 1
 
+            if not path:
+                continue
+
             if not os.path.exists(path):
                 # Keep files out of the list which no longer exist in the repo.
                 results.append(SanityMessage(
diff --git a/test/runner/lib/sanity/import.py b/test/runner/lib/sanity/import.py
index 60daeff79ec..ebe3893af55 100644
--- a/test/runner/lib/sanity/import.py
+++ b/test/runner/lib/sanity/import.py
@@ -19,6 +19,7 @@ from lib.util import (
     remove_tree,
     display,
     find_python,
+    read_lines_without_comments,
 )
 
 from lib.ansible_util import (
@@ -43,9 +44,8 @@ class ImportTest(SanityMultipleVersion):
         :type python_version: str
         :rtype: TestResult
         """
-        with open('test/sanity/import/skip.txt', 'r') as skip_fd:
-            skip_paths = skip_fd.read().splitlines()
-
+        skip_file = 'test/sanity/import/skip.txt'
+        skip_paths = read_lines_without_comments(skip_file, remove_blank_lines=True)
         skip_paths_set = set(skip_paths)
 
         paths = sorted(
@@ -121,7 +121,7 @@ class ImportTest(SanityMultipleVersion):
                 column=int(r['column']),
             ) for r in results]
 
-            results = [result for result in results if result.path not in skip_paths]
+            results = [result for result in results if result.path not in skip_paths_set]
 
         if results:
             return SanityFailure(self.name, messages=results, python_version=python_version)
diff --git a/test/runner/lib/sanity/pep8.py b/test/runner/lib/sanity/pep8.py
index 92ddd083416..cf48749385e 100644
--- a/test/runner/lib/sanity/pep8.py
+++ b/test/runner/lib/sanity/pep8.py
@@ -15,6 +15,7 @@ from lib.util import (
     SubprocessError,
     display,
     run_command,
+    read_lines_without_comments,
 )
 
 from lib.config import (
@@ -37,17 +38,14 @@ class Pep8Test(SanitySingleVersion):
         :type targets: SanityTargets
         :rtype: TestResult
         """
-        with open(PEP8_SKIP_PATH, 'r') as skip_fd:
-            skip_paths = skip_fd.read().splitlines()
+        skip_paths = read_lines_without_comments(PEP8_SKIP_PATH)
+        legacy_paths = read_lines_without_comments(PEP8_LEGACY_PATH)
 
-        with open(PEP8_LEGACY_PATH, 'r') as legacy_fd:
-            legacy_paths = legacy_fd.read().splitlines()
+        legacy_ignore_file = 'test/sanity/pep8/legacy-ignore.txt'
+        legacy_ignore = set(read_lines_without_comments(legacy_ignore_file, remove_blank_lines=True))
 
-        with open('test/sanity/pep8/legacy-ignore.txt', 'r') as ignore_fd:
-            legacy_ignore = set(ignore_fd.read().splitlines())
-
-        with open('test/sanity/pep8/current-ignore.txt', 'r') as ignore_fd:
-            current_ignore = sorted(ignore_fd.read().splitlines())
+        current_ignore_file = 'test/sanity/pep8/current-ignore.txt'
+        current_ignore = sorted(read_lines_without_comments(current_ignore_file, remove_blank_lines=True))
 
         skip_paths_set = set(skip_paths)
         legacy_paths_set = set(legacy_paths)
@@ -106,6 +104,9 @@ class Pep8Test(SanitySingleVersion):
         for path in legacy_paths:
             line += 1
 
+            if not path:
+                continue
+
             if not os.path.exists(path):
                 # Keep files out of the list which no longer exist in the repo.
                 errors.append(SanityMessage(
@@ -133,6 +134,9 @@ class Pep8Test(SanitySingleVersion):
         for path in skip_paths:
             line += 1
 
+            if not path:
+                continue
+
             if not os.path.exists(path):
                 # Keep files out of the list which no longer exist in the repo.
                 errors.append(SanityMessage(
diff --git a/test/runner/lib/sanity/pslint.py b/test/runner/lib/sanity/pslint.py
index f2f39a90726..8e028e7cb50 100644
--- a/test/runner/lib/sanity/pslint.py
+++ b/test/runner/lib/sanity/pslint.py
@@ -18,6 +18,7 @@ from lib.util import (
     SubprocessError,
     run_command,
     find_executable,
+    read_lines_without_comments,
 )
 
 from lib.config import (
@@ -41,30 +42,31 @@ class PslintTest(SanitySingleVersion):
         :type targets: SanityTargets
         :rtype: TestResult
         """
-        with open(PSLINT_SKIP_PATH, 'r') as skip_fd:
-            skip_paths = skip_fd.read().splitlines()
+        skip_paths = read_lines_without_comments(PSLINT_SKIP_PATH)
 
         invalid_ignores = []
 
-        with open(PSLINT_IGNORE_PATH, 'r') as ignore_fd:
-            ignore_entries = ignore_fd.read().splitlines()
-            ignore = collections.defaultdict(dict)
-            line = 0
+        ignore_entries = read_lines_without_comments(PSLINT_IGNORE_PATH)
+        ignore = collections.defaultdict(dict)
+        line = 0
 
-            for ignore_entry in ignore_entries:
-                line += 1
+        for ignore_entry in ignore_entries:
+            line += 1
 
-                if ' ' not in ignore_entry:
-                    invalid_ignores.append((line, 'Invalid syntax'))
-                    continue
+            if not ignore_entry:
+                continue
 
-                path, code = ignore_entry.split(' ', 1)
+            if ' ' not in ignore_entry:
+                invalid_ignores.append((line, 'Invalid syntax'))
+                continue
 
-                if not os.path.exists(path):
-                    invalid_ignores.append((line, 'Remove "%s" since it does not exist' % path))
-                    continue
+            path, code = ignore_entry.split(' ', 1)
 
-                ignore[path][code] = line
+            if not os.path.exists(path):
+                invalid_ignores.append((line, 'Remove "%s" since it does not exist' % path))
+                continue
+
+            ignore[path][code] = line
 
         paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] in ('.ps1', '.psm1', '.psd1') and i.path not in skip_paths)
 
@@ -138,6 +140,9 @@ class PslintTest(SanitySingleVersion):
         for path in skip_paths:
             line += 1
 
+            if not path:
+                continue
+
             if not os.path.exists(path):
                 # Keep files out of the list which no longer exist in the repo.
                 errors.append(SanityMessage(
diff --git a/test/runner/lib/sanity/pylint.py b/test/runner/lib/sanity/pylint.py
index 86701b8d8a5..9736ac56dc0 100644
--- a/test/runner/lib/sanity/pylint.py
+++ b/test/runner/lib/sanity/pylint.py
@@ -24,6 +24,7 @@ from lib.util import (
     run_command,
     display,
     find_executable,
+    read_lines_without_comments,
 )
 
 from lib.executor import (
@@ -69,43 +70,44 @@ class PylintTest(SanitySingleVersion):
             display.warning('Skipping pylint on unsupported Python version %s.' % args.python_version)
             return SanitySkipped(self.name)
 
-        with open(PYLINT_SKIP_PATH, 'r') as skip_fd:
-            skip_paths = skip_fd.read().splitlines()
+        skip_paths = read_lines_without_comments(PYLINT_SKIP_PATH)
 
         invalid_ignores = []
 
         supported_versions = set(SUPPORTED_PYTHON_VERSIONS) - set(UNSUPPORTED_PYTHON_VERSIONS)
         supported_versions = set([v.split('.')[0] for v in supported_versions]) | supported_versions
 
-        with open(PYLINT_IGNORE_PATH, 'r') as ignore_fd:
-            ignore_entries = ignore_fd.read().splitlines()
-            ignore = collections.defaultdict(dict)
-            line = 0
+        ignore_entries = read_lines_without_comments(PYLINT_IGNORE_PATH)
+        ignore = collections.defaultdict(dict)
+        line = 0
 
-            for ignore_entry in ignore_entries:
-                line += 1
+        for ignore_entry in ignore_entries:
+            line += 1
 
-                if ' ' not in ignore_entry:
-                    invalid_ignores.append((line, 'Invalid syntax'))
-                    continue
+            if not ignore_entry:
+                continue
 
-                path, code = ignore_entry.split(' ', 1)
+            if ' ' not in ignore_entry:
+                invalid_ignores.append((line, 'Invalid syntax'))
+                continue
 
-                if not os.path.exists(path):
-                    invalid_ignores.append((line, 'Remove "%s" since it does not exist' % path))
-                    continue
+            path, code = ignore_entry.split(' ', 1)
 
-                if ' ' in code:
-                    code, version = code.split(' ', 1)
+            if not os.path.exists(path):
+                invalid_ignores.append((line, 'Remove "%s" since it does not exist' % path))
+                continue
+
+            if ' ' in code:
+                code, version = code.split(' ', 1)
 
-                    if version not in supported_versions:
-                        invalid_ignores.append((line, 'Invalid version: %s' % version))
-                        continue
+                if version not in supported_versions:
+                    invalid_ignores.append((line, 'Invalid version: %s' % version))
+                    continue
 
-                    if version != args.python_version and version != args.python_version.split('.')[0]:
-                        continue  # ignore version specific entries for other versions
+                if version != args.python_version and version != args.python_version.split('.')[0]:
+                    continue  # ignore version specific entries for other versions
 
-                ignore[path][code] = line
+            ignore[path][code] = line
 
         skip_paths_set = set(skip_paths)
 
@@ -193,6 +195,9 @@ class PylintTest(SanitySingleVersion):
         for path in skip_paths:
             line += 1
 
+            if not path:
+                continue
+
             if not os.path.exists(path):
                 # Keep files out of the list which no longer exist in the repo.
                 errors.append(SanityMessage(
diff --git a/test/runner/lib/sanity/rstcheck.py b/test/runner/lib/sanity/rstcheck.py
index 33bab62d37f..9cd9d8313df 100644
--- a/test/runner/lib/sanity/rstcheck.py
+++ b/test/runner/lib/sanity/rstcheck.py
@@ -17,6 +17,7 @@ from lib.util import (
     parse_to_dict,
     display,
     find_executable,
+    read_lines_without_comments,
 )
 
 from lib.config import (
@@ -40,8 +41,8 @@ class RstcheckTest(SanitySingleVersion):
             display.warning('Skipping rstcheck on unsupported Python version %s.' % args.python_version)
             return SanitySkipped(self.name)
 
-        with open('test/sanity/rstcheck/ignore-substitutions.txt', 'r') as ignore_fd:
-            ignore_substitutions = sorted(set(ignore_fd.read().splitlines()))
+        ignore_file = 'test/sanity/rstcheck/ignore-substitutions.txt'
+        ignore_substitutions = sorted(set(read_lines_without_comments(ignore_file, remove_blank_lines=True)))
 
         paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] in ('.rst',))
 
diff --git a/test/runner/lib/sanity/shellcheck.py b/test/runner/lib/sanity/shellcheck.py
index cee0c29f060..229391e304e 100644
--- a/test/runner/lib/sanity/shellcheck.py
+++ b/test/runner/lib/sanity/shellcheck.py
@@ -19,6 +19,7 @@ from lib.sanity import (
 from lib.util import (
     SubprocessError,
     run_command,
+    read_lines_without_comments,
 )
 
 from lib.config import (
@@ -34,11 +35,11 @@ class ShellcheckTest(SanitySingleVersion):
         :type targets: SanityTargets
         :rtype: TestResult
         """
-        with open('test/sanity/shellcheck/skip.txt', 'r') as skip_fd:
-            skip_paths = set(skip_fd.read().splitlines())
+        skip_file = 'test/sanity/shellcheck/skip.txt'
+        skip_paths = set(read_lines_without_comments(skip_file, remove_blank_lines=True))
 
-        with open('test/sanity/shellcheck/exclude.txt', 'r') as exclude_fd:
-            exclude = set(exclude_fd.read().splitlines())
+        exclude_file = 'test/sanity/shellcheck/exclude.txt'
+        exclude = set(read_lines_without_comments(exclude_file, remove_blank_lines=True))
 
         paths = sorted(i.path for i in targets.include if os.path.splitext(i.path)[1] == '.sh' and i.path not in skip_paths)
 
diff --git a/test/runner/lib/sanity/validate_modules.py b/test/runner/lib/sanity/validate_modules.py
index e570281322a..98f2c5f972e 100644
--- a/test/runner/lib/sanity/validate_modules.py
+++ b/test/runner/lib/sanity/validate_modules.py
@@ -17,6 +17,7 @@ from lib.util import (
     SubprocessError,
     display,
     run_command,
+    read_lines_without_comments,
 )
 
 from lib.ansible_util import (
@@ -44,9 +45,7 @@ class ValidateModulesTest(SanitySingleVersion):
         :type targets: SanityTargets
         :rtype: TestResult
         """
-        with open(VALIDATE_SKIP_PATH, 'r') as skip_fd:
-            skip_paths = skip_fd.read().splitlines()
-
+        skip_paths = read_lines_without_comments(VALIDATE_SKIP_PATH)
         skip_paths_set = set(skip_paths)
 
         env = ansible_environment(args, color=False)
@@ -65,21 +64,23 @@ class ValidateModulesTest(SanitySingleVersion):
 
         invalid_ignores = []
 
-        with open(VALIDATE_IGNORE_PATH, 'r') as ignore_fd:
-            ignore_entries = ignore_fd.read().splitlines()
-            ignore = collections.defaultdict(dict)
-            line = 0
+        ignore_entries = read_lines_without_comments(VALIDATE_IGNORE_PATH)
+        ignore = collections.defaultdict(dict)
+        line = 0
 
-            for ignore_entry in ignore_entries:
-                line += 1
+        for ignore_entry in ignore_entries:
+            line += 1
 
-                if ' ' not in ignore_entry:
-                    invalid_ignores.append((line, 'Invalid syntax'))
-                    continue
+            if not ignore_entry:
+                continue
 
-                path, code = ignore_entry.split(' ', 1)
+            if ' ' not in ignore_entry:
+                invalid_ignores.append((line, 'Invalid syntax'))
+                continue
 
-                ignore[path][code] = line
+            path, code = ignore_entry.split(' ', 1)
+
+            ignore[path][code] = line
 
         if args.base_branch:
             cmd.extend([
@@ -139,9 +140,14 @@ class ValidateModulesTest(SanitySingleVersion):
                 confidence=calculate_confidence(VALIDATE_IGNORE_PATH, line, args.metadata) if args.metadata.changes else None,
             ))
 
+        line = 0
+
         for path in skip_paths:
             line += 1
 
+            if not path:
+                continue
+
             if not os.path.exists(path):
                 # Keep files out of the list which no longer exist in the repo.
                 errors.append(SanityMessage(
diff --git a/test/runner/lib/target.py b/test/runner/lib/target.py
index 260189f2063..55c3c35d729 100644
--- a/test/runner/lib/target.py
+++ b/test/runner/lib/target.py
@@ -12,6 +12,7 @@ import sys
 
 from lib.util import (
     ApplicationError,
+    read_lines_without_comments,
 )
 
 MODULE_EXTENSIONS = '.py', '.ps1'
@@ -511,8 +512,8 @@ class IntegrationTarget(CompletionTarget):
         # static_aliases
 
         try:
-            with open(os.path.join(path, 'aliases'), 'r') as aliases_file:
-                static_aliases = tuple(aliases_file.read().splitlines())
+            aliases_path = os.path.join(path, 'aliases')
+            static_aliases = tuple(read_lines_without_comments(aliases_path, remove_blank_lines=True))
         except IOError as ex:
             if ex.errno != errno.ENOENT:
                 raise
diff --git a/test/runner/lib/util.py b/test/runner/lib/util.py
index 95ae9ae6677..373cdaf93d8 100644
--- a/test/runner/lib/util.py
+++ b/test/runner/lib/util.py
@@ -42,8 +42,7 @@ def get_docker_completion():
     :rtype: dict[str, str]
     """
     if not DOCKER_COMPLETION:
-        with open('test/runner/completion/docker.txt', 'r') as completion_fd:
-            images = completion_fd.read().splitlines()
+        images = read_lines_without_comments('test/runner/completion/docker.txt', remove_blank_lines=True)
 
         DOCKER_COMPLETION.update(dict(kvp for kvp in [parse_docker_completion(i) for i in images] if kvp))
 
@@ -81,6 +80,23 @@ def remove_file(path):
         os.remove(path)
 
 
+def read_lines_without_comments(path, remove_blank_lines=False):
+    """
+    :type path: str
+    :type remove_blank_lines: bool
+    :rtype: list[str]
+    """
+    with open(path, 'r') as path_fd:
+        lines = path_fd.read().splitlines()
+
+    lines = [re.sub(r' *#.*$', '', line) for line in lines]
+
+    if remove_blank_lines:
+        lines = [line for line in lines if line]
+
+    return lines
+
+
 def find_executable(executable, cwd=None, path=None, required=True):
     """
     :type executable: str