Skip to content

Conversation

@stleary
Copy link
Owner

@stleary stleary commented Aug 20, 2015

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
stleary/JSON-java#146 have been pushed. The modified test
will not even compile until then.

@dtaillard
Copy link

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

@stleary
Copy link
Owner Author

stleary commented Aug 23, 2015

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
Copy link

So where do I push to?

@stleary
Copy link
Owner Author

stleary commented Aug 23, 2015

[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
Copy link

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

stleary added a commit that referenced this pull request Aug 24, 2015
sample tests for XML.toJSONObject(Reader)
@stleary stleary merged commit 3f78a85 into master Aug 24, 2015
@stleary stleary deleted the xml-reader-remote branch January 2, 2016 21:00
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