fix: flakiness in org.json.junit.JSONObjectTest#valueToString#2
fix: flakiness in org.json.junit.JSONObjectTest#valueToString#2
Conversation
|
If you can expand the PR description to include reasons for importing Apart from that, good to go! |
|
I have some comments on the PR.Adding a dependency in pom.xml might not be recommended since Developers might run some security checks on the code. It would be great to remove the dependency or look for alternate dependency that already exists in the package |
|
Just be prepared that some developers might not like to add extra dependencies. You can proceed to open a real PR. Once you open a real PR, please mark this tentative PR as |
Problem:
The string in the assertion can change, because the Map which is used to store the data in the JSONObject returns the data in non-deterministic order (the order of child elements in JSON strings do not matter, and JSON strings are equal regardless of the ordering of the elements on the same hierarchy level)
The flaky test was found by using the NonDex tool.
The flakiness was discovered on the following lines
JSON-java/src/test/java/org/json/junit/JSONObjectTest.java
Lines 2028 to 2029 in 01727fd
as well as
JSON-java/src/test/java/org/json/junit/JSONObjectTest.java
Lines 2039 to 2040 in 01727fd
Solution:
Changed the assert statement from a JUnit Assertion to a JSON Assertion, so the strings are not compared charwise but are compared the way as specified for JSON strings (not taking care of the order of the elements on the same level).
The JSONAssertion library was chosen because it provides general assertions to compare JSON strings without the need of writing complex JUnit Assertions, what would need a lot of work to convert the strings in a different data structure, a lot of boilerplate code as well as the chance of adding more errors than fixing.
Result:
The test is deterministic and not flaky. This improves the quality of the test and reduces the time to search for the bug during future development.
Reproduce: