-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds tests for numbers #58
Conversation
fixes test to be applicable
Thanks, this is a good addition for Number tests, though it can't be accepted while the tests fail. |
All these tests should pass. The bug is validated as a positive test. |
I am using JSON-Java latest master branch and JSON-Java-unit-test SimplifyNumberWrap branch. Got an error in JSONObjectTest: |
ah, I'll retest then. when I tried they all passed. |
fixed |
Fails now in line 207 - I think the JSONObject string is ordered differently on my machine than on yours:
|
ah that makes sense, and I knew that when I wrote it, but forgot to fix it On Wed, Aug 17, 2016 at 12:06 PM, Sean Leary notifications@github.com
John. |
This adds positive tests for how numbers are handled and output. It adds in 3 new classes:
These tests show a bug in the way numbers are currently output by the library as no validation is done to ensure that they are the proper format.
See PR stleary/JSON-java#272 on the main library for conversations on fixes. that PR now offers a fix which solve the bug.