fix: flakiness in org.json.junit.XMLTest#testIndentComplicatedJsonObjectWithArrayAndWithConfig #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
One test method was missing `@Test` so it was never run. One test method used another test class as the base for finding a test resource. While this works in practice with Maven, it is not strictly right.
Small test fixes.
hofi1
force-pushed
the
bugfix/fix-flakyness-org.json.junit.XMLTest#testIndentComplicatedJsonObjectWithArrayAndWithConfig
branch
2 times, most recently
from
157baee to
1397794
Compare
For object keys, we can just skip the part of `nextValue()` that parses values
that are objects or arrays. Then the existing logic for unquoted values will
already stop at `{` or `[`, and that will produce a `Missing value` exception.
|
The fix makes sense, but can you try to make a fix without adding a dependency to pom as adding a new dependency may cause security implications which might lead to rejection of the PR. |
Add optJSONArray method to JSONObject with a default value
Disallow nested objects and arrays as keys in objects.
JUnit 4.13.2
Removing excessive synchronization
Owner
Author
|
@KiruthikaJanakiraman thank you for your great feedback. |
XMLTest.testIndentComplicatedJsonObjectWithArrayAndWithConfig fails
when run on Windows due to mismatching linebreaks (that aren't
important for the test's functionality) between the actual and
expected strings.
For the actual strings, linebreaks are canonized to the platform's
native linebreak using `replaceAll("\\n|\\r\\n",
System.getProperty("line.separator")`. However, the expected result is
read from a file, and is left with the linebreaks that were originally
used to create it.
The solution is to perform the same canonization on both strings.
Closes stleary#781
Use the built-in System.lineSeparator() instead of implementing it
ourselves with System.getProperty("line.separator") in order to clean
up the code and make it easier to maintain.
For exponential decimal conversion, number is not touched. Leading zeros removed from numeric number strings before converting to number.
* fixes stleary#713 * document JSONException in JavaDoc * remove unused Comparable<T> boundary to reuse GenericBean in test
Fix XMLTest.testIndentComplicatedJsonObjectWithArrayAndWithConfig() for Windows - in the test
Fix XMLTest.testIndentComplicatedJsonObjectWithArrayAndWithConfig() for Windows - in the test
add validity check for JSONObject constructors
optLong vs getLong inconsistencies
update the docs for release 20231013
hofi1
force-pushed
the
bugfix/fix-flakyness-org.json.junit.XMLTest#testIndentComplicatedJsonObjectWithArrayAndWithConfig
branch
4 times, most recently
from
f4f7fc4 to
f346203
Compare
|
Awesome work Simon! 🎉 congratulation for getting merged |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
In this test, the contents of an XML file and a JSON file is getting parsed and compared. But XML does not specify the order of the elements in the String, what means that the actual and the expected string cannot be directly compared for equality.
The flaky test was found by using the NonDex tool.
JSON-java/src/test/java/org/json/junit/XMLTest.java
Line 1242 in 01727fd
Solution:
To fix this assertion without the import of a new XMLAssertion library, just like in other PRs discussed, the XML String gets converted to a JSON Object and afterward compared for similarity. This is needed due to the fact that XML Objects do not offer a
similarity()method.JSON-java/src/test/java/org/json/junit/XMLTest.java
Line 1242 in f4f7fc4
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, as well as the need for reruns of the pipeline.
Reproduce