From d533445b2673d9c98578b63f73e740667721718b Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 28 Aug 2015 20:06:42 -0400 Subject: [PATCH] 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 | 17 +++--- os_testr/tests/test_os_testr.py | 100 ++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 7 deletions(-) diff --git a/os_testr/os_testr.py b/os_testr/os_testr.py index 83d4140..9751c02 100755 --- a/os_testr/os_testr.py +++ b/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() - # After the # is a comment - comment = split_line[1].strip() - if regex: + line_regex = split_line[0].strip() + if len(split_line) > 1: + # After the # is a comment + comment = split_line[1].strip() + 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 diff --git a/os_testr/tests/test_os_testr.py b/os_testr/tests/test_os_testr.py index 9ae6dc5..f820ed1 100644 --- a/os_testr/tests/test_os_testr.py +++ b/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)