Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Flaky Tests Caused by JSON permutations #734

Merged
merged 1 commit into from Mar 27, 2023

Conversation

superMaaax
Copy link
Contributor

@superMaaax superMaaax commented Mar 22, 2023

Description

Flaky Tests found using NonDex by running the commands -

mvn -pl . edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.json.junit.XMLTest#testIndentComplicatedJsonObject
mvn -pl . edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.json.junit.XMLTest#testIndentSimpleJsonArray
mvn -pl . edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.json.junit.XMLTest#testIndentSimpleJsonObject

The logged failure was-

[[ERROR] Failures:
[ERROR] XMLTest.testIndentSimpleJsonObject:1193 expected:<...>

<[married>true
sonoo
56000</salary]>

</Te...> but was:<...>

<[name>sonoo
56000
true</married]>

</Te...>

The issue is the same for all three tests, so here I only show the failure message for the third test (to reduce the length of the error message).

Investigation

The tests fail with a comparison error while comparing an expected JSON String and the result from the value returned from XML.toString(). The toString function of XML makes no guarantees as to the iteration order of the attributes in the object. This makes the test outcome non-deterministic, and the test fails whenever the function returns a mismatch in order of the elements in the JSON String. To fix this, the expected and actual keys should be checked in a more deterministic way so that the assertions do not fail.

Fix

Expected and Actual values can be converted into JSONObject and the similar function can be used to compare these objects. As this function compares the values inside the JSONObjects without needing order, the test becomes deterministic and ensures that the flakiness from the test is removed.

The PR does not introduce a breaking change.

###Description
Flaky Tests found using NonDex by running the commands -
mvn -pl . edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.json.junit.XMLTest#testIndentComplicatedJsonObject

mvn -pl . edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.json.junit.XMLTest#testIndentSimpleJsonArray

mvn -pl . edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.json.junit.XMLTest#testIndentSimpleJsonObject

The logged failure was-

[ERROR] Failures:
[ERROR] XMLTest.testIndentSimpleJsonObject:1193 expected:<...>
<employee>
<[married>true</married>
<name>sonoo</name>
<salary>56000</salary]>
</employee>
</Te...> but was:<...>
<employee>
<[name>sonoo</name>
<salary>56000</salary>
<married>true</married]>
</employee>
</Te...>

The issue is the same for all three tests, so here I only show the failure message for the third test (to reduce the length of the error message).

### Investigation

The tests fail with a comparison error while comparing an expected JSON String and the result from the value returned from XML.toString(). The toString function of XML makes no guarantees as to the iteration order of the attributes in the object. This makes the test outcome non-deterministic, and the test fails whenever the function returns a mismatch in order of the elements in the JSON String. To fix this, the expected and actual keys should be checked in a more deterministic way so that the assertions do not fail.

### Fix

Expected and Actual values can be converted into JSONObject and the similar function can be used to compare these objects. As this function compares the values inside the JSONObjects without needing order, the test becomes deterministic and ensures that the flakiness from the test is removed.

The PR does not introduce a breaking change.
@stleary
Copy link
Owner

stleary commented Mar 25, 2023

What problem does this code solve?
Address flaky test failures in XMLTest

Risks
Low

Changes to the API?
N/A

Will this require a new release?
No

Should the documentation be updated?
No

Does it break the unit tests?
No

Was any code refactored in this commit?
No

Review status
APPROVED

Starting 3-day comment window

@stleary stleary merged commit 0578285 into stleary:master Mar 27, 2023
5 checks passed
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.

None yet

2 participants