You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/12/12 02:12:54 UTC
[GitHub] [pinot] aditya-kumbhar opened a new pull request, #9966: Fix flaky tests caused by json ordering
aditya-kumbhar opened a new pull request, #9966:
URL: https://github.com/apache/pinot/pull/9966
The test `org.apache.pinot.common.utils.PinotDataTypeTest#testObject` was detected to be flaky, using the [NonDex](https://github.com/TestingResearchIllinois/NonDex) tool:
Command to reproduce using Nondex:
`edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.apache.pinot.common.utils.PinotDataTypeTest#testObject -Dlicense.skip=true -Drat.skip=true`
The following assertion failed:
```
assertEquals(OBJECT.toJson(getGenericTestObject()),
"{\"bytes\":\"AAE=\",\"map\":{\"key1\":\"value\",\"key2\":null,\"array\":[-5.4,4,\"2\"]},"
+ "\"timestamp\":1620324238610}");
```
The test fails due to non-deterministic ordering of the Json objects that are being compared for equality.
Error log:
```
[ERROR] java.lang.AssertionError: expected [{"bytes":"AAE=","map":{"key1":"value","key2":null,"array":[-5.4,4,"2"]},"timestamp":1620324238610}] but found [{"map":{"key1":"value","key2":null,"array":[-5.4,4,"2"]},"bytes":"AAE=","timestamp":1620324238610}]
at org.testng.Assert.fail(Assert.java:93)
at org.testng.Assert.failNotEquals(Assert.java:512)
at org.testng.Assert.assertEqualsImpl(Assert.java:134)
at org.testng.Assert.assertEquals(Assert.java:115)
at org.testng.Assert.assertEquals(Assert.java:189)
at org.testng.Assert.assertEquals(Assert.java:199)
at org.apache.pinot.common.utils.PinotDataTypeTest.testObject(PinotDataTypeTest.java:234)
```
Changes proposed in this pull request:
-
The flakiness of the test is because the json string created by `OBJECT.toJson(getGenericTestObject())` does not have deterministic ordering of its elements.
Using the gson [JsonParser](https://stackoverflow.com/questions/2253750/testing-two-json-objects-for-equality-ignoring-child-order-in-java/9065247#9065247).parseString(), the json strings are converted to json parse trees.
The generated parse trees are then compared for equality in the assertions, ignoring the order of the child elements.
The tests pass using [NonDex](https://github.com/TestingResearchIllinois/NonDex) after this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] aditya-kumbhar commented on a diff in pull request #9966: Use gson JSONParser for asserting JSON equality deterministically
Posted by GitBox <gi...@apache.org>.
aditya-kumbhar commented on code in PR #9966:
URL: https://github.com/apache/pinot/pull/9966#discussion_r1046481023
##########
pinot-common/src/test/java/org/apache/pinot/common/utils/PinotDataTypeTest.java:
##########
@@ -18,6 +18,7 @@
*/
package org.apache.pinot.common.utils;
+import com.google.gson.JsonParser;
Review Comment:
Okay, I will update the PR with JsonUtils.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9966: Use gson JSONParser for asserting JSON equality deterministically
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9966:
URL: https://github.com/apache/pinot/pull/9966#discussion_r1046450757
##########
pinot-common/src/test/java/org/apache/pinot/common/utils/PinotDataTypeTest.java:
##########
@@ -18,6 +18,7 @@
*/
package org.apache.pinot.common.utils;
+import com.google.gson.JsonParser;
Review Comment:
Within Pinot, we try to unify the usage of json library to `fasterxml.jackson`. You may use `JsonUtils.stringToJsonNode()` to convert the string to JsonNode and then compare.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Use gson JSONParser for asserting JSON equality deterministically [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang closed pull request #9966: Use gson JSONParser for asserting JSON equality deterministically
URL: https://github.com/apache/pinot/pull/9966
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org