-
-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
bpo-24780: unittest assertEqual difference output foiled by newlines #11548
Conversation
@@ -1238,6 +1238,11 @@ def assertCountEqual(self, first, second, msg=None): | |||
msg = self._formatMessage(msg, standardMsg) | |||
self.fail(msg) | |||
|
|||
def addTrailingNewLine(self, line): | |||
if line != '' and (line[-1] != '\n' or line[-1] != ' '): | |||
line = line + '\n' |
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.
Thanks for the PR. line
is a list here and this causes test failures due to TypeError
. lines
seems to be a better name since it makes me think it's a single line but it's actually a list of lines.
class AssertEqualTest(unittest.TestCase): | ||
|
||
def test_trailing_new_line_at_end(self): | ||
self.assertEqual("abc\n", "abc\n") |
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.
I think a better test would be to test the output diff like https://github.com/python/cpython/blob/e709c1be97a04156c628f534116a236b9581aa2f/Lib/unittest/test/test_case.py#L1122 and it should be testing the failure cases and asserting the Exception output string.
222a928
to
df4bdc9
Compare
6467b25
to
819b341
Compare
@tirkarthi I made the necessary updates to this PR. |
Thanks @nanjekyejoannah, tests should be fixed as noted in https://github.com/python/cpython/pull/11548/files#r247392463 . The tests are about the strings being equal and are not related to the issue. The actual tests should be about the diff output for the tests. |
closed this. Someone can open another PR. |
@nanjekyejoannah Why did you close the PR? |
I have created a fix for this issue and relevant tests.
https://bugs.python.org/issue24780