Fix regression in error message introduced in bpo-29951. by serhiy-storchaka · Pull Request #2028 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation3 Commits3 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
If you can, it would be nice to have one unit test for the error message. It seems like this case is not currently tested by test_call.
| @@ -131,6 +131,10 @@ def test_varargs2(self): |
|---|
| msg = r"__contains__\(\) takes exactly one argument \(2 given\)" |
| self.assertRaisesRegex(TypeError, msg, {}.__contains__, 0, 1) |
| def test_varargs3(self): |
| msg = r"from_bytes\(\) takes at most 2 positional arguments \(3 given\)" |
| self.assertRaisesRegex(TypeError, msg, int.from_bytes, b'a', 'little', False) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This unit test is wrong: it doesn't catch the fixed bug. You need to add "^" at the start of the regex.
You may add "^" and "$" to all regex of this file ;-) (Maybe even write an helper function to create the regex?)
Thanks for the fix, but also for your overall work on error messages!
Labels
An unexpected behavior, bug, or error