-
Notifications
You must be signed in to change notification settings - Fork 749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix cross-platfrom issues with test suite #529
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,26 @@ | |
from testsuite.support import ROOT_DIR, PseudoFile | ||
|
||
|
||
def safe_line_split(line): | ||
""" parses the path, from message details """ | ||
|
||
# On certain platforms, the drive notation conflicts | ||
# with the message seperator, inducing problems during tuple unpacking. | ||
|
||
split = line.split(':') | ||
if 'win' in sys.platform: | ||
if len(split) == 5: | ||
drive, path, x, y, msg = split | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you leave comments explaining what kind of situation would result in having 5 items? (Likewise for the elif checking for a length of 4) Bonus points for an example in the comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, that will be helpful as we continue to build up the windows support and verify that use case. |
||
path = drive + ':' + path | ||
elif len(split) == 4: | ||
path, x, y, msg = split | ||
else: | ||
raise Exception("Unhandled edge case parsing message: " + line) | ||
else: | ||
path, x, y, msg = split | ||
return path, x, y, msg | ||
|
||
|
||
class ShellTestCase(unittest.TestCase): | ||
"""Test the usual CLI options and output.""" | ||
|
||
|
@@ -77,7 +97,7 @@ def test_check_simple(self): | |
self.assertFalse(stderr) | ||
self.assertEqual(len(stdout), 17) | ||
for line, num, col in zip(stdout, (3, 6, 9, 12), (3, 6, 1, 5)): | ||
path, x, y, msg = line.split(':') | ||
path, x, y, msg = safe_line_split(line) | ||
self.assertTrue(path.endswith(E11)) | ||
self.assertEqual(x, str(num)) | ||
self.assertEqual(y, str(col)) | ||
|
@@ -141,7 +161,7 @@ def test_check_diff(self): | |
self.assertEqual(errcode, 1) | ||
self.assertFalse(stderr) | ||
for line, num, col in zip(stdout, (3, 6), (3, 6)): | ||
path, x, y, msg = line.split(':') | ||
path, x, y, msg = safe_line_split(line) | ||
self.assertEqual(x, str(num)) | ||
self.assertEqual(y, str(col)) | ||
self.assertTrue(msg.startswith(' E11')) | ||
|
@@ -154,7 +174,7 @@ def test_check_diff(self): | |
self.assertEqual(errcode, 1) | ||
self.assertFalse(stderr) | ||
for line, num, col in zip(stdout, (3, 6), (3, 6)): | ||
path, x, y, msg = line.split(':') | ||
path, x, y, msg = safe_line_split(line) | ||
self.assertEqual(x, str(num)) | ||
self.assertEqual(y, str(col)) | ||
self.assertTrue(msg.startswith(' E11')) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, these comments can / should probably just move into the function docstring.