fix JSON flaky tests

[hofi1]

Fixes apache#15198, apache#15199, apache#15200, apache#15201, apache#15202, apache#15204

These flaky tests have been found using the NonDex Plugin.

Description

Fixes flaky test org.apache.druid.java.util.emitter.core.ParametrizedUriEmitterTest#testEmitterWithMultipleFeeds

ID: apache#15198

Problem:

java.lang.AssertionError: 
expected:<{http://example.com/test1=[{"metrics":{"value":1},"feed":"test1"}]
, http://example.com/test2=[{"metrics":{"value":2},"feed":"test2"}]
}> but was:<{http://example.com/test1=[{"feed":"test1","metrics":{"value":1}}]
, http://example.com/test2=[{"feed":"test2","metrics":{"value":2}}]
}>

This test checks the Equality of two maps, containing the same keys (strings) and values (strings). These values are JSON-String. JSON strings are not unique – they are considered equal if they contain the same elements without taking care of the order of the elements on the same level. The current assertion leads to a flaky test.

Solution

To fix this, the strings are getting parsed using the Jackson library and compared to remove the flakiness.

To reproduce run

mvn -pl processing edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.apache.druid.java.util.emitter.core.ParametrizedUriEmitterTest#testEmitterWithMultipleFeeds

---

Fixes flaky tests org.apache.druid.java.util.emitter.core.EmitterTest#testSanityWithGeneralizedCreation , test org.apache.druid.java.util.emitter.core.EmitterTest#testSanity , org.apache.druid.java.util.emitter.core.EmitterTest#testGzipContentEncoding

ID: apache#15199, apache#15200, apache#15201

Problem:

java.lang.RuntimeException: org.junit.ComparisonFailure: expected:<[{"[metrics":{"value":1},"feed":"test"]},{"feed":"test","me...> but was:<[{"[feed":"test","metrics":{"value":1}]},{"feed":"test","me...>

These tests check for a specific ordering of elements in a JSON string. JSON strings are equal even if the ordering of the elements in the JSON strings are not equal.

Solution

To fix this, the strings are getting parsed using the Jackson library and compared to remove the flakiness.

To reproduce run

mvn -pl processing edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=testname

---

---

Fixes flaky test org.apache.druid.java.util.emitter.core.EmitterTest#testBatchSplitting

ID: apache#15202

Problem:

[ERROR] org.apache.druid.java.util.emitter.core.EmitterTest.testBatchSplitting -- Time elapsed: 6.516 s <<< FAILURE!
java.lang.AssertionError: expected:<2> but was:<0>
	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
	at org.apache.druid.java.util.emitter.core.EmitterTest.testBatchSplitting(EmitterTest.java:541)

This test fails on EmitterTest.java:541, but the actual error is the JSON assertion in the GoHandler again. It checks for a specific ordering of elements in a JSON string. JSON strings are equal even if the ordering of the elements in the JSON strings are not equal.

Solution

To fix this, the strings are getting parsed using the Jackson library and compared to remove the flakiness.

To reproduce run

mvn -pl processing edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.apache.druid.java.util.emitter.core.EmitterTest#testBatchSplitting

---

Fixes flaky test org.apache.druid.java.util.emitter.core.EmitterTest#testBasicAuthenticationAndNewlineSeparating

ID: apache#15204

Problem:

[INFO] Running org.apache.druid.java.util.emitter.core.EmitterTest
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 3.200 s <<< FAILURE! -- in org.apache.druid.java.util.emitter.core.EmitterTest
[ERROR] org.apache.druid.java.util.emitter.core.EmitterTest.testBasicAuthenticationAndNewlineSeparating -- Time elapsed: 3.192 s <<< FAILURE!
java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:87)
	at org.junit.Assert.assertTrue(Assert.java:42)
	at org.junit.Assert.assertTrue(Assert.java:53)
	at org.apache.druid.java.util.emitter.core.EmitterTest.testBasicAuthenticationAndNewlineSeparating(EmitterTest.java:489)

This test fails in a later line, but the actual error is the JSON assertion in the GoHandler again. It checks for a specific ordering of elements in a JSON string. JSON strings are equal even if the ordering of the elements in the JSON strings are not equal.

Solution

To fix this, the strings are getting parsed using the Jackson library and compared to remove the flakiness.

To reproduce run

mvn -pl processing edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.apache.druid.java.util.emitter.core.EmitterTest#testBasicAuthenticationAndNewlineSeparating

---

Key changed/added classes in this PR

• org.apache.druid.java.util.emitter.core.ParametrizedUriEmitterTest
• org.apache.druid.java.util.emitter.core.EmitterTest

---

This PR has:

• been self-reviewed.
• added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.

[kavvya97]

Just a few comments on the PR.

1. I feel that the description is too long, developers might not look into everything. If all the tests have the same error log, try to mention the error log only once or group the tests with the same error log. If the solution/fix is to use the JSONassert library to fix all of them, again mentioning them once will reduce the PR description.
2. For the code base, I see that you are adding dependencies. Developers, sometimes, might not be comfortable with adding new libraries due to security issues etc. so please provide valid reasoning for why this library is required or check for alternate libraries with which the same issue can be fixed.
3. provided some inline comments in code

[zzjas]

I remember you had some PRs to add this dependency. If you opened a PR already, you can either push this to the existing PR or mention the existing one. Or if you haven't opened a PR with this dependency, ignore me. You can proceed to open a real PR. Once you open a real PR, please mark this tentative PR as Opened in your tentative_pr.csv file and also raise a PR to IDoFT marking this as Opened. Thanks!

[zzjas]

Actually, I noticed someone else is using fasterxml.jackson to do JSON comparison. It should be an existing dependency. Did you check this one before?

[hofi1]

@zzjas thanks for pointing that out, I was not aware of this!

[Doom-Prophet]

Overall a good fix with very detailed descriptions in the PR, but I agree with kavvya97's opinion that maybe containing the long log messages in this description will annoy the developers, thus try to replace it with brief summary and let them run the commands to get the logs if they want.