classification
Title: test_precision in test_format.py is not executed and has unused variable
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: eric.smith, mark.dickinson, python-dev, serhiy.storchaka, vajrasky, vstinner
Priority: normal Keywords: patch

Created on 2013-08-05 08:03 by vajrasky, last changed 2013-10-13 12:49 by mark.dickinson. This issue is now closed.

Files
File name Uploaded Description Edit
test_precision.patch vajrasky, 2013-08-05 08:03 review
test_precision_v2.patch vajrasky, 2013-08-05 15:14 review
test_precision_v3.patch vajrasky, 2013-08-06 03:06 review
Messages (20)
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) * (Python committer) Date: 2013-08-05 09:24
Patch looks good to me.
msg194463 - (view) Author: Roundup Robot (python-dev) (Python triager) 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) * (Python committer) Date: 2013-08-05 09:31
Fixed.  Thanks!
msg194467 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) 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) (Python triager) 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) * (Python committer) 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 <module>
    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 <module>
    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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) (Python triager) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2013-10-13 12:49
Buildbots are happy now.  Closing.  Thank you!
History
Date User Action Args
2013-10-13 12:49:13mark.dickinsonsetstatus: open -> closed
resolution: fixed
messages: + msg199703
2013-10-13 10:54:20mark.dickinsonsetmessages: + msg199688
2013-10-13 10:05:52mark.dickinsonsetmessages: + msg199684
2013-10-13 10:05:17python-devsetmessages: + msg199683
2013-08-06 21:28:00pitrousetnosy: + eric.smith
2013-08-06 03:06:43vajraskysetfiles: + test_precision_v3.patch

messages: + msg194522
2013-08-06 00:32:11vstinnersetmessages: + msg194519
2013-08-05 15:14:25vajraskysetfiles: + test_precision_v2.patch

messages: + msg194485
2013-08-05 13:39:29vajraskysetmessages: + msg194479
2013-08-05 13:26:11serhiy.storchakasetmessages: + msg194478
2013-08-05 13:16:03vstinnersetmessages: + msg194477
2013-08-05 13:12:18vstinnersetmessages: + msg194476
2013-08-05 12:54:43serhiy.storchakasetnosy: + vstinner, serhiy.storchaka
2013-08-05 10:48:30vajraskysetmessages: + msg194471
2013-08-05 10:35:00vajraskysetmessages: + msg194470
2013-08-05 10:01:42mark.dickinsonsetmessages: + msg194469
2013-08-05 10:00:08python-devsetmessages: + msg194468
2013-08-05 09:57:37mark.dickinsonsetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg194467
2013-08-05 09:31:45mark.dickinsonsetstatus: open -> closed
type: behavior
messages: + msg194464

assignee: mark.dickinson
resolution: fixed
stage: resolved
2013-08-05 09:30:41python-devsetnosy: + python-dev
messages: + msg194463
2013-08-05 09:24:53mark.dickinsonsetnosy: + mark.dickinson
messages: + msg194462
2013-08-05 08:03:10vajraskycreate