classification
Title: Test enum in test_json is ignorant of infinity value
Type: Stage: resolved
Components: Tests Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ethan.furman Nosy List: barry, eli.bendersky, ethan.furman, python-dev, vajrasky
Priority: normal Keywords: patch

Created on 2013-08-15 07:59 by vajrasky, last changed 2013-09-02 08:15 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
add_infinity_to_test_enum_in_json.patch vajrasky, 2013-08-15 07:58 review
issue18745.stoneleaf.01.patch ethan.furman, 2013-08-28 07:15 review
issue18745.stoneleaf.02.patch ethan.furman, 2013-09-01 02:27 review
Messages (8)
msg195238 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-08-15 07:58
Test enum json in Lib/test/test_json/test_enum.py is ignorant of infinity values. Also, NaN, but since NaN is a weirdo, let's not take that into account.

The unit test should represent of what will work in every case. For example:

    def test_floats(self):
         for enum in FloatNum:
            self.assertEqual(self.dumps(enum), repr(enum.value))

This will fail if enum is infinity.

This wisdom about infinity was bestowed upon me when I was reading Lib/test/test_json/test_float.py.

    def test_floats(self):
        for num in [1617161771.7650001, math.pi, math.pi**100, math.pi**-100, 3.1]:
            self.assertEqual(float(self.dumps(num)), num)
            self.assertEqual(self.loads(self.dumps(num)), num)

    def test_ints(self):
        for num in [1, 1<<32, 1<<64]:
            self.assertEqual(self.dumps(num), str(num))
            self.assertEqual(int(self.dumps(num)), num)

As you can see, in float case, we don't use str(num) because it does not work with infinity.

Attached the patch to refactor the test to handle infinity value. For the completeness sake, I added the case of negative infinity and NaN as well.
msg195447 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-08-17 02:06
Tests look good to me.
msg196353 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-08-28 07:15
The added tests for inf, -inf, and nan are good.

The refactoring of the dictionary tests are not good.  The reason is that before _json.c was fixed (issue18264) it would return the string value of an IntEnum instead of the string value of the IntEnum member's value attribute.

--> class Number(IntEnum):
...    one = 1
...    two = 2

--> json.dumps(Number.one)
'Number.one'  # should be '1'

As a constant we get a failure when trying to round-trip:

--> json.loads(json.dumps(Number.one))
Traceback (most recent call last):
  ...
ValueError: Expecting value: line 1 column 1 (char 0)

But as a dictionary we do not (because keys get morphed into strings):

--> json.dumps({Number.one: 'one'})
'{"Number.one": "one"}'  # should be '{"1": "one"}'

Which round-trips fine, but yields the wrong result:

>>> loads(dumps({Number.one: 'one'}))
{'Number.one': 'one'}  # should be {'1': 'one'}

So the dictionary tests (and the list tests) are there to make sure that json.dumps is converting them properly, not to make sure that they round-trip, and by changing from repr to loads/dumps the point of the test is lost.

Updated tests attached.
msg196698 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-01 02:27
Fixed formatting in patch.  ;)
msg196755 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-09-01 23:48
lgtm
msg196769 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-09-02 03:51
To test the infinity, negative infinity, and NaN (Not a Number), you named the test as test_weird_floats. So implicitely, you admitted that they are floats but just weird. Yet, you named the sample data test class as NotNum. Implicitely, you were saying they are not numbers. This is contradictory.

In my personal opinion, I disagree slightly with the naming because infinity and negative infinity are numbers. Why not name it WeirdNum (or SpecialNum) to make it more mathematically correct and consistent with the test naming?
msg196770 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2013-09-02 03:57
From my layman's perspective they are all not numbers -- you can't add 5 to any of them and get a number back that is 5 
more than you started with.

By I have no problem with your proposed name -- I'll make the change.
msg196773 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-09-02 08:15
New changeset 50e583f20d78 by Ethan Furman in branch 'default':
Close #18745: Improve enum tests in test_json for infinities and NaN.
http://hg.python.org/cpython/rev/50e583f20d78
History
Date User Action Args
2013-09-02 08:15:23python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg196773

resolution: fixed
stage: patch review -> resolved
2013-09-02 03:57:52ethan.furmansetmessages: + msg196770
2013-09-02 03:51:59vajraskysetmessages: + msg196769
2013-09-01 23:48:52eli.benderskysetmessages: + msg196755
2013-09-01 02:27:12ethan.furmansetfiles: + issue18745.stoneleaf.02.patch

messages: + msg196698
2013-08-28 07:15:10ethan.furmansetfiles: + issue18745.stoneleaf.01.patch

messages: + msg196353
2013-08-17 02:06:37ethan.furmansetnosy: + barry, eli.bendersky

messages: + msg195447
stage: patch review
2013-08-16 14:52:16ethan.furmansetassignee: ethan.furman
2013-08-15 07:59:00vajraskycreate