search

jdereg / json-io

Convert Java to JSON. Convert JSON to Java. Pretty print JSON. Java JSON serializer. Deep copy Java object graphs.
Apache License 2.0
331 stars 116 forks source link

Test Fix: Fix a test that can unintentionally fail in testAssignAtomicInteger() #295

Closed Jake-Zhi0Wang closed 1 month ago

Jake-Zhi0Wang commented 1 month ago

Due to TestUtil.toJson(), the json string may not be in a consistent order when converting an object into a JSON string. This could result in an AssertionError when comparing the converted JSON with the expected JSON. Below is an example where it fails under NonDex on line 60

Click on to see more details on the error message when running this flaky test ``` [INFO] Using auto detected provider org.apache.maven.surefire.junitplatform.JUnitPlatformProvider [INFO] [INFO] ------------------------------------------------------- [INFO] T E S T S [INFO] ------------------------------------------------------- [INFO] Running com.cedarsoftware.io.AtomicIntegerTest [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.698 s <<< FAILURE! -- in com.cedarsoftware.io.AtomicIntegerTest [ERROR] com.cedarsoftware.io.AtomicIntegerTest.testAssignAtomicInteger -- Time elapsed: 0.667 s <<< FAILURE! java.lang.AssertionError at com.cedarsoftware.io.AtomicIntegerTest.testAssignAtomicInteger(AtomicIntegerTest.java:60) at java.base/java.lang.reflect.Method.invoke(Method.java:569) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) [INFO] [INFO] Results: [INFO] [ERROR] Failures: [ERROR] AtomicIntegerTest.testAssignAtomicInteger:60 [INFO] [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 [INFO] ```

To reproduce, run this at the root directory:

mvn -pl . \
    edu.illinois:nondex-maven-plugin:2.1.7:nondex \
    -Dtest=com.cedarsoftware.io.AtomicIntegerTest#testAssignAtomicInteger -DnondexRuns=10

(Note: The failing test might not be seen if every test happens to have the JSON string in the correct order. Try running it several times or increase the the number of runs with -DnondexRuns= to reproduce the issue.)

To fix this, we could convert the JSON string to a JSON element using JsonParser, so that the order doesn't matter and we are only testing the content of the JSON to see if it matches the expected output.

jdereg commented 1 month ago

Thank you for making this change to improve reliablilty of the tests. I will incorporate it into the next release, like within a week or two.