homepage

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

Locations:
Objects/longobject.c
1994: "int() arg 2 must be >= 2 and <= 36");
4273: "int() arg 2 must be >= 2 and <= 36");

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

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).

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?

> 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.

Attaching failing test for pure Python portion.

> + 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)

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.

Can't you test the PyLong_FromString code path by passing bytes to int()? int(b"42")

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).

Ah, sorry, I misread what you wanted to be testing.

Ok, I've now attached a patch with tests.

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.

Since the first parameter of int() is now positional-only, this issue looks outdated.

> 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")

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: "...").

Isn't the information provided in your previous message enough?

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

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

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?

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

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!