Skip to content

Conversation

@Jake-Zhi0Wang
Copy link
Contributor

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.

@Jake-Zhi0Wang Jake-Zhi0Wang changed the title convert JSON string to JSON element before comparing Test Fix: Fix a test that can unintentionally fail in testAssignAtomicInteger() Oct 4, 2024
@jdereg
Copy link
Owner

jdereg commented Oct 5, 2024

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.

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.

2 participants