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

sample tests for XML.toJSONObject(Reader) #22

Closed stleary closed 9 years ago

stleary commented 9 years ago

This commit just has a single sample test where the JSONObject is built and tested with StringReader and FileReader instead of just String. Ideally, all tests in this module that compare an expected string to the result of XML.toJSONObject(String) should be modified so that all 3 tests are performed.

Unit tests verify new behavior, but mostly they are used to protect against regressions due to unintended consequences of other code changes. In this case, a third reason is emphasized. We think that toJSONObject(String) and toJSONObject(Reader) produce exactly the same result. But we don't really know this for a fact. So the unit tests will confirm whether this is the case.

This branch should not be merged until the corresponding code changes proposed in https://github.com/douglascrockford/JSON-java/pull/146 have been pushed. The modified test will not even compile until then.

dtaillard commented 9 years ago

I thought this was just an example that should not be merged….

stleary commented 9 years ago

This branch will contain all of the new tests for toJSONObject(Reader). Once the tests are done, the JSON-java code will be merged, and then then this branch will be merged.

dtaillard commented 9 years ago

So where do I push to?

stleary commented 9 years ago

[Just tried this, it seems to work] Fork this branch into your own repository: https://github.com/stleary/JSON-Java-unit-test/tree/xml-reader-remote. Make your changes and push to your forked branch. When completed, browse to your branch on github.com and click the green icon to initiate a pull request. You can try this with any trivial local change to work through the process. If we can't get it to work, I will just merge the code manually.

dtaillard commented 9 years ago

Oh, so I create a pull request for a pull request. I got a little confused there, thanks!