Fix issues with the blacklist file regex generation

This commit fixes a couple of bugs in the blacklist file regex
generator. The first where a comment line was accidently made
mandatory. If a comment wasn't specified an IndexError would be
raised. The second was related to the variable naming in the
function's blacklist file if branch. Variable reuse there was causing
the last blacklist regex to take precendence over a regex passed
in and would be used instead.

Change-Id: Ib80a0c1781db7c8c9e4449b4773258fe3348411a
Closes-Bug: #1488700

os_testr/os_testr.py

@@ -134,15 +134,18 @@ def construct_regex(blacklist_file, regex, print_exclude):
             raw_line = line.strip()
             split_line = raw_line.split('#')
             # Before the # is the regex
-            regex = split_line[0].strip()
+            line_regex = split_line[0].strip()
+            if len(split_line) > 1:
                 # After the # is a comment
                 comment = split_line[1].strip()
-            if regex:
+            else:
+                comment = ''
+            if line_regex:
                 if print_exclude:
-                    print_skips(regex, comment)
-                exclude_regex = '|'.join([regex, exclude_regex])
+                    print_skips(line_regex, comment)
+                exclude_regex = '|'.join([line_regex, exclude_regex])
         if exclude_regex:
-            exclude_regex = "'(?!.*" + exclude_regex + ")"
+            exclude_regex = "(?!.*" + exclude_regex + ")"
     if regex:
         exclude_regex += regex
     return exclude_regex

os_testr/tests/test_os_testr.py

@@ -18,7 +18,9 @@ test_os_testr
 
 Tests for `os_testr` module.
 """
+
 import mock
+import six
 
 from os_testr import os_testr
 from os_testr.tests import base
@@ -123,3 +125,101 @@ class TestCallers(base.TestCase):
                                   'call_subunit_run',
                                   side_effect=_fake_run):
             os_testr.main()
+
+
+class TestConstructRegex(base.TestCase):
+    def test_regex_passthrough(self):
+        result = os_testr.construct_regex(None, 'fake_regex', False)
+        self.assertEqual(result, 'fake_regex')
+
+    def test_blacklist_regex_with_comments(self):
+        blacklist_file = six.StringIO()
+        for i in range(4):
+            blacklist_file.write('fake_regex_%s # A Comment\n' % i)
+        blacklist_file.seek(0)
+        with mock.patch('six.moves.builtins.open',
+                        return_value=blacklist_file):
+            result = os_testr.construct_regex('fake_path', None, False)
+        self.assertEqual(
+            result,
+            "(?!.*fake_regex_3|fake_regex_2|fake_regex_1|fake_regex_0|)")
+
+    def test_blacklist_regex_without_comments(self):
+        blacklist_file = six.StringIO()
+        for i in range(4):
+            blacklist_file.write('fake_regex_%s\n' % i)
+        blacklist_file.seek(0)
+        with mock.patch('six.moves.builtins.open',
+                        return_value=blacklist_file):
+            result = os_testr.construct_regex('fake_path', None, False)
+        self.assertEqual(
+            result,
+            "(?!.*fake_regex_3|fake_regex_2|fake_regex_1|fake_regex_0|)")
+
+    def test_blacklist_regex_with_comments_and_regex(self):
+        blacklist_file = six.StringIO()
+        for i in range(4):
+            blacklist_file.write('fake_regex_%s # Comments\n' % i)
+        blacklist_file.seek(0)
+        with mock.patch('six.moves.builtins.open',
+                        return_value=blacklist_file):
+            result = os_testr.construct_regex('fake_path', 'fake_regex', False)
+
+            expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|"
+                              "fake_regex_0|)fake_regex")
+            self.assertEqual(result, expected_regex)
+
+    def test_blacklist_regex_without_comments_and_regex(self):
+        blacklist_file = six.StringIO()
+        for i in range(4):
+            blacklist_file.write('fake_regex_%s\n' % i)
+        blacklist_file.seek(0)
+        with mock.patch('six.moves.builtins.open',
+                        return_value=blacklist_file):
+            result = os_testr.construct_regex('fake_path', 'fake_regex', False)
+
+            expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|"
+                              "fake_regex_0|)fake_regex")
+            self.assertEqual(result, expected_regex)
+
+    @mock.patch.object(os_testr, 'print_skips')
+    def test_blacklist_regex_with_comment_print_skips(self, print_mock):
+        blacklist_file = six.StringIO()
+        for i in range(4):
+            blacklist_file.write('fake_regex_%s # Comment\n' % i)
+        blacklist_file.seek(0)
+        with mock.patch('six.moves.builtins.open',
+                        return_value=blacklist_file):
+            result = os_testr.construct_regex('fake_path', None, True)
+
+        expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|"
+                          "fake_regex_0|)")
+        self.assertEqual(result, expected_regex)
+        calls = print_mock.mock_calls
+        self.assertEqual(len(calls), 4)
+        args = list(map(lambda x: x[1], calls))
+        self.assertIn(('fake_regex_0', 'Comment'), args)
+        self.assertIn(('fake_regex_1', 'Comment'), args)
+        self.assertIn(('fake_regex_2', 'Comment'), args)
+        self.assertIn(('fake_regex_3', 'Comment'), args)
+
+    @mock.patch.object(os_testr, 'print_skips')
+    def test_blacklist_regex_without_comment_print_skips(self, print_mock):
+        blacklist_file = six.StringIO()
+        for i in range(4):
+            blacklist_file.write('fake_regex_%s\n' % i)
+        blacklist_file.seek(0)
+        with mock.patch('six.moves.builtins.open',
+                        return_value=blacklist_file):
+            result = os_testr.construct_regex('fake_path', None, True)
+
+        expected_regex = ("(?!.*fake_regex_3|fake_regex_2|fake_regex_1|"
+                          "fake_regex_0|)")
+        self.assertEqual(result, expected_regex)
+        calls = print_mock.mock_calls
+        self.assertEqual(len(calls), 4)
+        args = list(map(lambda x: x[1], calls))
+        self.assertIn(('fake_regex_0', ''), args)
+        self.assertIn(('fake_regex_1', ''), args)
+        self.assertIn(('fake_regex_2', ''), args)
+        self.assertIn(('fake_regex_3', ''), args)