| msg194459 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-05 08:03 |
| There is test_precision in Lib/test_format.py which is not being unit tested. Also, there is a unused variable inside test_precision. Attached the patch to fix these problems. |
|
|
| msg194462 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2013-08-05 09:24 |
| Patch looks good to me. |
|
|
| msg194463 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-08-05 09:30 |
| New changeset cfd875bcbe41 by Mark Dickinson in branch 'default': Issue #18659: fix test_format test that wasn't being executed. Thanks Vajrasky Kok for the patch. http://hg.python.org/cpython/rev/cfd875bcbe41 |
|
|
| msg194464 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-08-05 09:31 |
| Fixed. Thanks! |
|
|
| msg194467 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-08-05 09:57 |
| Okay, that caused some buildbots to fail. I'm going to back out the change until I have time to figure out what's going on. |
|
|
| msg194468 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-08-05 10:00 |
| New changeset 9bee1fd64ee6 by Mark Dickinson in branch 'default': Issue #18659: Backed out changeset cfd875bcbe41 after buildbot failures. http://hg.python.org/cpython/rev/9bee1fd64ee6 |
|
|
| msg194469 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-08-05 10:01 |
| Sample buildbot output here: http://buildbot.python.org/all/builders/x86%20RHEL%206%203.x/builds/2485/steps/test/logs/stdio Relevant snippet: test_precision (test.test_format.FormatTest) ... FAIL ====================================================================== FAIL: test_precision (test.test_format.FormatTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/buildbot/buildarea/3.x.coghlan-redhat/build/Lib/test/test_format.py", line 338, in test_precision self.assertEqual(str(cm.exception), "precision too big") AssertionError: 'Too many decimal digits in format string' != 'precision too big' - Too many decimal digits in format string + precision too big ---------------------------------------------------------------------- Ran 5 tests in 0.011s |
|
|
| msg194470 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-05 10:35 |
| Let me help you to debug this issue. ethan@amiau:~/Documents/code/python/cpython$ cat /tmp/a.py import sys INT_MAX = sys.maxsize f = 1.2 format(f, ".%sf" % (INT_MAX + 1)) ethan@amiau:~/Documents/code/python/cpython$ ./python /tmp/a.py Traceback (most recent call last): File "/tmp/a.py", line 5, in format(f, ".%sf" % (INT_MAX + 1)) ValueError: Too many decimal digits in format string ethan@amiau:~/Documents/code/python/cpython$ cat /tmp/b.py import sys INT_MAX = 2147483647 f = 1.2 format(f, ".%sf" % (INT_MAX + 1)) ethan@amiau:~/Documents/code/python/cpython$ ./python /tmp/b.py Traceback (most recent call last): File "/tmp/b.py", line 5, in format(f, ".%sf" % (INT_MAX + 1)) ValueError: precision too big My question is whether we should have different exception message for these two cases? |
|
|
| msg194471 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-05 10:48 |
| For now, instead of hardcoding INT_MAX to 2147483647 in test, maybe we can use module: >>> import IN >>> IN.INT_MAX 2147483647 |
|
|
| msg194476 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-08-05 13:12 |
| The IN module must not be used, it is hardcoded and never regenerated. |
|
|
| msg194477 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-08-05 13:16 |
| I added the test in the following commit: changeset: 84266:ef5175d08e7e branch: 3.3 parent: 84263:7ecca1a98220 user: Victor Stinner <victor.stinner@gmail.com> date: Sun Jun 23 14:54:30 2013 +0200 files: Lib/test/test_format.py Misc/NEWS Python/formatter_unicode.c description: Issue #18137: Detect integer overflow on precision in float.__format__() and complex.__format__(). |
|
|
| msg194478 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-08-05 13:26 |
| We have _testcapi.INT_MAX. I guess different exceptions raised on 64-bit platform. First parser checks that a number can be represented as Py_ssize_t (i.e. <= PY_SSIZE_T_MAX). Here "Too many decimal digits in format string" can be raised. Then precision passed to internal function which accepts int and checked to be <= INT_MAX before cast to int. Here "precision too big" can be raised. |
|
|
| msg194479 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-05 13:39 |
| For the passers-by who want to help: The "precision too big" exception is raised in Python/formatter_unicode.c line 1168 and 1002. The "Too many decimal digits..." exception is raised in Python/formatter_unicode.c line 71. So the question is whether it is beneficial to differentiate the exception message. If not, we can change the exception message or test whether the digit passes INT_MAX first before checking with sys.max_size. If yes, we need to add test case for sys.max_size and use _testcapi.INT_MAX for current unit test case. |
|
|
| msg194485 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-05 15:14 |
| Okay, I guess the fix for this ticket should be kept simple. If we want to merge the exception message or touch the code base or do something smarter than fixing the test, maybe we should create a separate ticket. So I used Serhiy Storchaka's suggestion to use _testcapi.INT_MAX for the second patch. |
|
|
| msg194519 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2013-08-06 00:32 |
| I realized that the new tests checking that precision larger with INT_MAX fail should be marked as specific to CPython. The import of _testcapi should be moved to a CPython specific test. |
|
|
| msg194522 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-06 03:06 |
| Attached the third patch. The importing _testcapi part was moved inside the test. Added cpython support only decorator for this test. |
|
|
| msg199683 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-10-13 10:05 |
| New changeset e7eed20f2da7 by Mark Dickinson in branch 'default': Issue #18659: fix test_format test that wasn't being executed. Thanks Vajrasky Kok for the patch. http://hg.python.org/cpython/rev/e7eed20f2da7 |
|
|
| msg199684 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-10-13 10:05 |
| Let's try again. I'll close once the buildbots have run. |
|
|
| msg199688 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-10-13 10:54 |
| ... and the buildbots with sizeof(int) == sizeof(size_t) == 4 still fail, of course. http://hg.python.org/cpython/rev/d115dc671f52 fixes that by removing the check for the exact error message. It should be enough to check for ValueError anyway. |
|
|
| msg199703 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2013-10-13 12:49 |
| Buildbots are happy now. Closing. Thank you! |
|
|