openstack/os-testr
Code Issues Proposed changes

Fix regex builder

Browse Source 
Currently when the blacklist_file and regex string is provided,
the constructed regex looks like '^((?!black1|black2|...).)*$regex'.
This is incorrect, as it will match nothing - some string
is expected after end of the line [denoted as $ in the regex].

The proper construction is like ^(?!black1|black2|...).*(regex).*$
This solves the issue with Tempest, where using blacklist for smoke
tests is not working now, as it leads into the issue described above.

Change-Id: Icdeb3c311f7eb414158aedb4c030494b419211c0
Closes-Bug: #1506215
Closes-Bug: #1595119
Closes-Bug: #1622722
Closes-Bug: #1669455
This commit is contained in:
Szymon Datko 2017-11-23 15:54:29 +01:00
parent 3fd82f674c
commit 96db91056a
2 changed files with 113 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
os_testr/regex_builder.py
View File

@ -71,12 +71,9 @@ def get_regex_from_whitelist_file(file_path):
return '|'.join(lines) return '|'.join(lines)
def construct_regex(blacklist_file, whitelist_file, regex, print_exclude): def get_regex_from_blacklist_file(file_path, print_exclude=False):
"""Deprecated, please use testlist_builder.construct_list instead."""
if not blacklist_file:
exclude_regex = '' exclude_regex = ''
else: with open(file_path, 'r') as black_file:
with open(blacklist_file, 'r') as black_file:
exclude_regex = '' exclude_regex = ''
for line in black_file: for line in black_file:
raw_line = line.strip() raw_line = line.strip()
@ -96,9 +93,24 @@ def construct_regex(blacklist_file, whitelist_file, regex, print_exclude):
else: else:
exclude_regex = line_regex exclude_regex = line_regex
if exclude_regex: if exclude_regex:
exclude_regex = "^((?!" + exclude_regex + ").)*$" exclude_regex = "(?!" + exclude_regex + ")"
if regex:
exclude_regex += regex
if whitelist_file:
exclude_regex += '%s' % get_regex_from_whitelist_file(whitelist_file)
return exclude_regex return exclude_regex
def construct_regex(blacklist_file, whitelist_file, regex, print_exclude):
"""Deprecated, please use testlist_builder.construct_list instead."""
bregex = ''
wregex = ''
pregex = ''
if blacklist_file:
bregex = get_regex_from_blacklist_file(blacklist_file, print_exclude)
if whitelist_file:
wregex = get_regex_from_whitelist_file(whitelist_file)
if regex:
pregex = regex
combined_regex = '^%s.*(%s).*$' % (bregex, '|'.join(
filter(None, [pregex, wregex])
))
return combined_regex

101
os_testr/tests/test_regex_builder.py
View File

@ -31,7 +31,7 @@ class TestPathToRegex(base.TestCase):
class TestConstructRegex(base.TestCase): class TestConstructRegex(base.TestCase):
def test_regex_passthrough(self): def test_regex_passthrough(self):
result = os_testr.construct_regex(None, None, 'fake_regex', False) result = os_testr.construct_regex(None, None, 'fake_regex', False)
self.assertEqual(result, 'fake_regex') self.assertEqual(result, '^.*(fake_regex).*$')
def test_blacklist_regex_with_comments(self): def test_blacklist_regex_with_comments(self):
with io.StringIO() as blacklist_file: with io.StringIO() as blacklist_file:
@ -42,8 +42,8 @@ class TestConstructRegex(base.TestCase):
return_value=blacklist_file): return_value=blacklist_file):
result = os_testr.construct_regex( result = os_testr.construct_regex(
'fake_path', None, None, False) 'fake_path', None, None, False)
self.assertEqual(result, "^((?!fake_regex_3|fake_regex_2|" self.assertEqual(result, "^(?!fake_regex_3|fake_regex_2|"
"fake_regex_1|fake_regex_0).)*$") "fake_regex_1|fake_regex_0).*().*$")
def test_whitelist_regex_with_comments(self): def test_whitelist_regex_with_comments(self):
with io.StringIO() as whitelist_file: with io.StringIO() as whitelist_file:
@ -56,7 +56,7 @@ class TestConstructRegex(base.TestCase):
None, 'fake_path', None, False) None, 'fake_path', None, False)
self.assertEqual( self.assertEqual(
result, result,
"fake_regex_0|fake_regex_1|fake_regex_2|fake_regex_3") "^.*(fake_regex_0|fake_regex_1|fake_regex_2|fake_regex_3).*$")
def test_blacklist_regex_without_comments(self): def test_blacklist_regex_without_comments(self):
with io.StringIO() as blacklist_file: with io.StringIO() as blacklist_file:
@ -67,8 +67,8 @@ class TestConstructRegex(base.TestCase):
return_value=blacklist_file): return_value=blacklist_file):
result = os_testr.construct_regex( result = os_testr.construct_regex(
'fake_path', None, None, False) 'fake_path', None, None, False)
self.assertEqual(result, "^((?!fake_regex_3|fake_regex_2|" self.assertEqual(result, "^(?!fake_regex_3|fake_regex_2|"
"fake_regex_1|fake_regex_0).)*$") "fake_regex_1|fake_regex_0).*().*$")
def test_blacklist_regex_with_comments_and_regex(self): def test_blacklist_regex_with_comments_and_regex(self):
with io.StringIO() as blacklist_file: with io.StringIO() as blacklist_file:
@ -81,8 +81,8 @@ class TestConstructRegex(base.TestCase):
'fake_regex', False) 'fake_regex', False)
expected_regex = ( expected_regex = (
"^((?!fake_regex_3|fake_regex_2|fake_regex_1|" "^(?!fake_regex_3|fake_regex_2|fake_regex_1|"
"fake_regex_0).)*$fake_regex") "fake_regex_0).*(fake_regex).*$")
self.assertEqual(result, expected_regex) self.assertEqual(result, expected_regex)
def test_blacklist_regex_without_comments_and_regex(self): def test_blacklist_regex_without_comments_and_regex(self):
@ -96,8 +96,8 @@ class TestConstructRegex(base.TestCase):
'fake_regex', False) 'fake_regex', False)
expected_regex = ( expected_regex = (
"^((?!fake_regex_3|fake_regex_2|fake_regex_1|" "^(?!fake_regex_3|fake_regex_2|fake_regex_1|"
"fake_regex_0).)*$fake_regex") "fake_regex_0).*(fake_regex).*$")
self.assertEqual(result, expected_regex) self.assertEqual(result, expected_regex)
@mock.patch.object(os_testr, 'print_skips') @mock.patch.object(os_testr, 'print_skips')
@ -111,8 +111,8 @@ class TestConstructRegex(base.TestCase):
result = os_testr.construct_regex('fake_path', None, result = os_testr.construct_regex('fake_path', None,
None, True) None, True)
expected_regex = ("^((?!fake_regex_3|fake_regex_2|fake_regex_1|" expected_regex = ("^(?!fake_regex_3|fake_regex_2|fake_regex_1|"
"fake_regex_0).)*$") "fake_regex_0).*().*$")
self.assertEqual(result, expected_regex) self.assertEqual(result, expected_regex)
calls = print_mock.mock_calls calls = print_mock.mock_calls
self.assertEqual(len(calls), 4) self.assertEqual(len(calls), 4)
@ -133,8 +133,8 @@ class TestConstructRegex(base.TestCase):
result = os_testr.construct_regex('fake_path', None, result = os_testr.construct_regex('fake_path', None,
None, True) None, True)
expected_regex = ("^((?!fake_regex_3|fake_regex_2|" expected_regex = ("^(?!fake_regex_3|fake_regex_2|"
"fake_regex_1|fake_regex_0).)*$") "fake_regex_1|fake_regex_0).*().*$")
self.assertEqual(result, expected_regex) self.assertEqual(result, expected_regex)
calls = print_mock.mock_calls calls = print_mock.mock_calls
self.assertEqual(len(calls), 4) self.assertEqual(len(calls), 4)
@ -144,9 +144,56 @@ class TestConstructRegex(base.TestCase):
self.assertIn(('fake_regex_2', ''), args) self.assertIn(('fake_regex_2', ''), args)
self.assertIn(('fake_regex_3', ''), args) self.assertIn(('fake_regex_3', ''), args)
def test_whitelist_regex_without_comments_and_regex_passthrough(self):
file_contents = u"""regex_a
regex_b"""
with io.StringIO() as whitelist_file:
whitelist_file.write(file_contents)
whitelist_file.seek(0)
with mock.patch('six.moves.builtins.open',
return_value=whitelist_file):
result = os_testr.construct_regex(None, 'fake_path',
None, False)
class TestWhitelistFile(base.TestCase): expected_regex = '^.*(regex_a|regex_b).*$'
def test_read_whitelist_file(self): self.assertEqual(result, expected_regex)
def test_whitelist_regex_without_comments_with_regex_passthrough(self):
file_contents = u"""regex_a
regex_b"""
with io.StringIO() as whitelist_file:
whitelist_file.write(file_contents)
whitelist_file.seek(0)
with mock.patch('six.moves.builtins.open',
return_value=whitelist_file):
result = os_testr.construct_regex(None, 'fake_path',
'fake_regex', False)
expected_regex = '^.*(fake_regex|regex_a|regex_b).*$'
self.assertEqual(result, expected_regex)
def test_blacklist_whitelist_and_regex_passthrough_at_once(self):
with io.StringIO() as blacklist_file, io.StringIO() as whitelist_file:
for i in range(4):
blacklist_file.write(u'fake_regex_%s\n' % i)
blacklist_file.seek(0)
whitelist_file.write(u'regex_a\n')
whitelist_file.write(u'regex_b\n')
whitelist_file.seek(0)
with mock.patch('six.moves.builtins.open',
side_effect=[blacklist_file, whitelist_file]):
result = os_testr.construct_regex('fake_path_1', 'fake_path_2',
'fake_regex', False)
expected_regex = (
"^(?!fake_regex_3|fake_regex_2|fake_regex_1|"
"fake_regex_0).*(fake_regex|regex_a|regex_b).*$")
self.assertEqual(result, expected_regex)
class TestGetRegexFromListFile(base.TestCase):
def test_get_regex_from_whitelist_file(self):
file_contents = u"""regex_a file_contents = u"""regex_a
regex_b""" regex_b"""
with io.StringIO() as whitelist_file: with io.StringIO() as whitelist_file:
@ -158,19 +205,17 @@ regex_b"""
'/path/to/not_used') '/path/to/not_used')
self.assertEqual('regex_a|regex_b', regex) self.assertEqual('regex_a|regex_b', regex)
def test_whitelist_regex_without_comments_and_regex(self): def test_get_regex_from_blacklist_file(self):
file_contents = u"""regex_a with io.StringIO() as blacklist_file:
regex_b""" for i in range(4):
with io.StringIO() as whitelist_file: blacklist_file.write(u'fake_regex_%s\n' % i)
whitelist_file.write(file_contents) blacklist_file.seek(0)
whitelist_file.seek(0)
with mock.patch('six.moves.builtins.open', with mock.patch('six.moves.builtins.open',
return_value=whitelist_file): return_value=blacklist_file):
result = os_testr.construct_regex(None, 'fake_path', regex = os_testr.get_regex_from_blacklist_file(
None, False) '/path/to/not_used')
self.assertEqual('(?!fake_regex_3|fake_regex_2'
expected_regex = 'regex_a|regex_b' '|fake_regex_1|fake_regex_0)', regex)
self.assertEqual(result, expected_regex)
class TestGetTestList(base.TestCase): class TestGetTestList(base.TestCase):