Skip to content

Comments

Add tests for PR #38#40

Closed
urucoder wants to merge 11 commits intotcalmant:masterfrom
urucoder:master
Closed

Add tests for PR #38#40
urucoder wants to merge 11 commits intotcalmant:masterfrom
urucoder:master

Conversation

@urucoder
Copy link

In addition, fixes some minor stuff on core and removes very old unused code at the tests

@coveralls
Copy link

coveralls commented Apr 15, 2020

Coverage Status

Coverage increased (+0.8%) to 79.99% when pulling 80acea1 on UruDev:master into 776e894 on tcalmant:master.

@urucoder
Copy link
Author

Seems like the test is failing in pyhton versions previous to 3.6, I'll check it.
Let me know if you have some idea on why this is happening

@tcalmant
Copy link
Owner

Seems like it is a float precision error in the test:

FAIL: Tests support for custom writeObject (PR #38)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/tcalmant/python-javaobj/tests/tests_v2.py", line 565, in test_writeObject
    self.assertEqual(expected['custom_obj']['annotations'], super_data)
AssertionError: {'seed': 25214903879, 'haveNextNextGaussian': False, 'nextNextGaussian': 0.0} != {'nextNextGaussian': 1.245781777e-313, 'haveNextNextGaussian': False, 'seed': 0}
- {'haveNextNextGaussian': False, 'nextNextGaussian': 0.0, 'seed': 25214903879}
?                                                     ^ ^          ------ ----
+ {'haveNextNextGaussian': False, 'nextNextGaussian': 1.245781777e-313, 'seed': 0}
?                                                     ^ ^^^^^^^^^^^^^^

You could try to check the expected dictionary key by key and use self.assertAlmostEqual

@tcalmant
Copy link
Owner

Mmh I didn't see: there is also an error with the seed value

@tcalmant
Copy link
Owner

OK so the issue is in the test:
You use a dictionary, which were not ordered in Python < 3.6, so you don't read the fields in the expected order.
You should use 2 lists or a collections.OrderedDict object

@urucoder
Copy link
Author

Ok, after some fails with dict ordering all tests are working and backward compatible

@tcalmant
Copy link
Owner

Great, I'll check that ASAP

@tcalmant
Copy link
Owner

Squashed, rebased and merged as commit b711479

@tcalmant tcalmant closed this Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants