Adds tests for numbers - Githubissues

stleary / JSON-Java-unit-test

Junit test harness to validate the JSON-Java GitHub project code.

Apache License 2.0

28 stars 45 forks source link

Adds tests for numbers #58

Closed johnjaylward closed 8 years ago

johnjaylward commented 8 years ago

This adds positive tests for how numbers are handled and output. It adds in 3 new classes:

Fraction: simple fraction class that has a toString() which outputs numbers in a format not compatible with JSON
MyNumber: simple number override that expresses the constant 42. This has a valid toString method that outputs numbers in a JSON compatible format
MyNumberContainer: a simple bean that holds a MyNumber instance. No toString() method is defined.

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 https://github.com/stleary/JSON-java/pull/272 on the main library for conversations on fixes. that PR now offers a fix which solve the bug.

stleary commented 8 years ago

Thanks, this is a good addition for Number tests, though it can't be accepted while the tests fail.

johnjaylward commented 8 years ago

All these tests should pass. The bug is validated as a positive test.

stleary commented 8 years ago

I am using JSON-Java latest master branch and JSON-Java-unit-test SimplifyNumberWrap branch. Got an error in JSONObjectTest: org.json.junit.JSONObjectTest > verifyNumberOutput FAILED org.junit.ComparisonFailure at JSONObjectTest.java:188

johnjaylward commented 8 years ago

ah, I'll retest then. when I tried they all passed.

johnjaylward commented 8 years ago

fixed

stleary commented 8 years ago

Fails now in line 207 - I think the JSONObject string is ordered differently on my machine than on yours: actual "{"myNumber":{"numerator":4,"denominator":2}}" (id=45) expected "{"myNumber":{"denominator":2,"numerator":4}}" (id=50)
Try fixing with JSONPointer :) e.g.

JSONPointer jsonPointer = new JSONPointer("/myNumber/denominator");
assertEquals(BigInteger.valueOf(2), jsonPointer.queryFrom(jsonObject));

johnjaylward commented 8 years ago

ah that makes sense, and I knew that when I wrote it, but forgot to fix it before finalizing it.

On Wed, Aug 17, 2016 at 12:06 PM, Sean Leary notifications@github.com wrote:

Fails now in line 207 - I think the JSONObject string is ordered differently on my machine than on yours: actual "{"myNumber":{"numerator":4,"denominator":2}}" (id=45)

expected "{"myNumber":{"denominator":2,"numerator":4}}" (id=50)

Try fixing with JSONPointer :) e.g. JSONPointer jsonPointer = new JSONPointer("/myNumber/denominator"); assertEquals(BigInteger.valueOf(2), jsonPointer.queryFrom(jsonObject));

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/stleary/JSON-Java-unit-test/pull/58#issuecomment-240461307, or mute the thread https://github.com/notifications/unsubscribe-auth/AAXa12-MWgEKyfM1HDVVJp7ycNG75Faoks5qgzGHgaJpZM4Jl9p5 .

John.