msg171357 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-09-26 19:41 |
The following error text is incorrect in at least one way:
>>> int(base=1000, x='1')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: int() arg 2 must be >= 2 and <= 36
The *base* argument can also be 0. Secondly, the text should probably say "base arg" instead of "arg 2" since 1000 is not in position 2.
The 2.7 code does not have the second issue:
>>> int(base=1000, x='1')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: int() base must be >= 2 and <= 36
|
msg171359 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-09-26 19:44 |
Locations:
Objects/longobject.c
1994: "int() arg 2 must be >= 2 and <= 36");
4273: "int() arg 2 must be >= 2 and <= 36");
|
msg171437 - (view) |
Author: Peter Inglesby (inglesp) * |
Date: 2012-09-28 11:17 |
The attached patch updates the error message to:
>>> int(base=100, x='123')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: int() base must be >= 2 and <= 36, or 0
|
msg171444 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-09-28 11:59 |
I should have said that I had started working on this issue. I think failing tests for both messages should accompany the patch (otherwise I would have already submitted a patch). The tricky one is the error message for PyLong_FromString(), which I believe can only be tested from C (e.g. by adding to Modules/_testcapimodule.c).
|
msg171453 - (view) |
Author: Peter Inglesby (inglesp) * |
Date: 2012-09-28 13:03 |
Ah, sorry about that. Are you happy for me to write the test?
Poking around the C API docs suggests that I should call PyErr_Fetch() to get the value of the a raised exception, but I can't see any precedent for this in existing test code. Can you point me to something I could use for inspiration?
|
msg171460 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-09-28 13:31 |
> Are you happy for me to write the test?
I had started working on that, but sure, be my guest. :)
You seem to be on the right track. I didn't find precedent nearby either. We basically want a C version of unittest's assertRaisesRegex() (but it can be a straight string match). CHECK_INVALID comes close to that.
I would suggest defining a helper function that accepts an exception type and message text, and that clears the current error if it matches. If the error doesn't match, you can restore the existing one with PyErr_Restore(), or else call raiseTestError() if there isn't one (similar to CHECK_INVALID).
I was thinking of putting the test right after the call to TESTNAME() in test_long_api(), but maybe you know a better location:
http://hg.python.org/cpython/file/default/Modules/_testcapimodule.c#l313
I'll upload what I had for the pure Python failing test since that portion was finished.
|
msg171463 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-09-28 13:37 |
Attaching failing test for pure Python portion.
|
msg171473 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2012-09-28 14:26 |
> + with self.assertRaises(ValueError) as e:
> + int('100', 1)
> + self.assertEquals(str(e.exception),
> + "int() arg base must be 0 or >= 2 and <= 36")
Why not use assertRaisesRegex()?
self.assertRaisesRegex(ValueError,
r'^int\() arg base must be 0 or >= 2 and <= 36$',
int, '100', 1)
|
msg171481 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-09-28 14:47 |
It could be done that way. It just seems simpler to me to do a simple string check when regex's aren't necessary. Then you don't have to worry about escaping characters, etc.
|
msg171504 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2012-09-28 16:50 |
Can't you test the PyLong_FromString code path by passing bytes to int()? int(b"42")
|
msg171506 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-09-28 17:04 |
Benjamin, unless I'm overlooking something, long_new() does its own check of the base argument before calling PyLong_FromString():
http://hg.python.org/cpython/file/default/Objects/longobject.c#l4251
So long_new() won't let you pass a bad base to PyLong_FromString() (which is one of the code paths to be checked).
|
msg171508 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2012-09-28 17:08 |
Ah, sorry, I misread what you wanted to be testing.
|
msg171624 - (view) |
Author: Peter Inglesby (inglesp) * |
Date: 2012-09-30 10:57 |
Ok, I've now attached a patch with tests.
|
msg171645 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
Date: 2012-09-30 15:25 |
A few comments on the patch:
1) You should also check the exception type (e.g. by using PyErr_ExceptionMatches()).
2) If the exception doesn't match, you should restore the original exception so that the pure Python test framework will in turn raise it and the caller can inspect it. Currently, the caller will find out only that it doesn't match but not why. Take a look at CHECK_INVALID() to see an example of this pattern being used.
3) I would expose the functionality that checks an exception's type and text as a helper function so that it can be used throughout _testcapimodule.c. The function is nontrivial enough that we wouldn't want to be copying and pasting it throughout if we want to check exception texts for other parts of the C API.
|
msg289166 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-03-07 15:44 |
Since the first parameter of int() is now positional-only, this issue looks outdated.
|
msg289167 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-03-07 15:46 |
> Since the first parameter of int() is now positional-only, this issue looks outdated.
Right, but Python 3.7 still has this issue: "The *base* argument can also be 0." The error message should be:
"ValueError: int() base must be >= 2 and <= 36 or 0" (add "or 0")
|
msg296327 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-06-19 13:02 |
Serhiy Storchaka: "set keywords: + easy (C)"
Thanks for making easy issues as easy and no fixing them :-) But in my experience, even if the issue seems easy to *you*, you should explain which file has to be patched, and what steps should be done to write the proper change (ex: change the error message raising by this statement: "...").
|
msg296403 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-06-20 06:29 |
Isn't the information provided in your previous message enough?
|
msg306170 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-11-13 21:49 |
New changeset 28b624825eb92cb8c96fbf8da267d8d14a61a841 by Victor Stinner (Sanyam Khurana) in branch 'master':
bpo-16055: Fixes incorrect error text for int('1', base=1000) (#4376)
https://github.com/python/cpython/commit/28b624825eb92cb8c96fbf8da267d8d14a61a841
|
msg306193 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-11-14 09:35 |
New changeset 58cbae22930486814cc01cf9f981d9fe5e0c68f9 by Victor Stinner (Miss Islington (bot)) in branch '3.6':
bpo-16055: Fixes incorrect error text for int('1', base=1000) (GH-4376) (#4389)
https://github.com/python/cpython/commit/58cbae22930486814cc01cf9f981d9fe5e0c68f9
|
msg306194 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2017-11-14 09:36 |
Thank you Sanyam Khurana, I merged your PR and backported the fix automaticalled to Python 3.6. Can you please try to backport the fix to Python 2.7 and create a new PR?
|
msg317072 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-05-18 23:53 |
New changeset d13169fc5ac7572a272cbcff830c3d96ba27cc7c by Victor Stinner in branch '2.7':
bpo-16055: Fixes incorrect error text for int('1', base=1000) (#6980)
https://github.com/python/cpython/commit/d13169fc5ac7572a272cbcff830c3d96ba27cc7c
|
msg317073 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-05-18 23:54 |
I backported manually the fix to Python 2.7 for int and long types.
Thank you Chris Jerdonek for the bug report, and Sanyam Khurana for the bugfix!
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:36 | admin | set | github: 60259 |
2018-05-18 23:54:23 | vstinner | set | status: open -> closed versions:
+ Python 2.7, Python 3.6 messages:
+ msg317073
resolution: fixed stage: patch review -> resolved |
2018-05-18 23:53:17 | vstinner | set | messages:
+ msg317072 |
2018-05-18 23:28:58 | vstinner | set | pull_requests:
+ pull_request6635 |
2017-11-14 09:36:41 | vstinner | set | messages:
+ msg306194 |
2017-11-14 09:35:19 | vstinner | set | messages:
+ msg306193 |
2017-11-13 21:51:01 | python-dev | set | pull_requests:
+ pull_request4338 |
2017-11-13 21:49:33 | vstinner | set | messages:
+ msg306170 |
2017-11-12 09:55:10 | CuriousLearner | set | stage: needs patch -> patch review pull_requests:
+ pull_request4325 |
2017-06-20 06:29:36 | serhiy.storchaka | set | messages:
+ msg296403 |
2017-06-19 13:02:43 | vstinner | set | messages:
+ msg296327 |
2017-06-18 12:15:04 | serhiy.storchaka | set | keywords:
+ easy (C) stage: test needed -> needs patch type: behavior -> enhancement versions:
+ Python 3.7, - Python 2.7, Python 3.3, Python 3.4 |
2017-03-07 15:46:46 | vstinner | set | status: pending -> open nosy:
+ vstinner messages:
+ msg289167
|
2017-03-07 15:44:02 | serhiy.storchaka | set | status: open -> pending
messages:
+ msg289166 |
2013-12-09 16:00:23 | serhiy.storchaka | set | priority: normal -> low versions:
+ Python 3.4, - Python 3.2 |
2012-09-30 15:25:25 | chris.jerdonek | set | messages:
+ msg171645 |
2012-09-30 10:57:46 | inglesp | set | files:
+ issue16055-fix-with-tests.patch
messages:
+ msg171624 |
2012-09-28 17:08:47 | benjamin.peterson | set | messages:
+ msg171508 |
2012-09-28 17:04:47 | chris.jerdonek | set | messages:
+ msg171506 |
2012-09-28 16:50:58 | benjamin.peterson | set | nosy:
+ benjamin.peterson messages:
+ msg171504
|
2012-09-28 14:47:32 | chris.jerdonek | set | messages:
+ msg171481 |
2012-09-28 14:26:09 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg171473
|
2012-09-28 13:37:33 | chris.jerdonek | set | files:
+ issue-16055-1-failing-test.patch
messages:
+ msg171463 |
2012-09-28 13:31:03 | chris.jerdonek | set | messages:
+ msg171460 |
2012-09-28 13:03:16 | inglesp | set | messages:
+ msg171453 |
2012-09-28 11:59:00 | chris.jerdonek | set | keywords:
- easy
messages:
+ msg171444 |
2012-09-28 11:17:54 | inglesp | set | files:
+ issue16055.patch
nosy:
+ inglesp messages:
+ msg171437
keywords:
+ patch |
2012-09-26 19:44:44 | chris.jerdonek | set | messages:
+ msg171359 |
2012-09-26 19:41:00 | chris.jerdonek | create | |