You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "yashdeep97 (via GitHub)" <gi...@apache.org> on 2023/11/23 04:25:44 UTC

[PR] Fix flaky tests in processing module (druid)

yashdeep97 opened a new pull request, #15418:
URL: https://github.com/apache/druid/pull/15418

   <!-- Thanks for trying to help us make Apache Druid be the best it can be! Please fill out as much of the following information as is possible (where relevant, and remove it when irrelevant) to help make the intention and scope of this PR clear in order to ease review. -->
   
   <!-- Please read the doc for contribution (https://github.com/apache/druid/blob/master/CONTRIBUTING.md) before making this PR. Also, once you open a PR, please _avoid using force pushes and rebasing_ since these make it difficult for reviewers to see what you've changed in response to their reviews. See [the 'If your pull request shows conflicts with master' section](https://github.com/apache/druid/blob/master/CONTRIBUTING.md#if-your-pull-request-shows-conflicts-with-master) for more details. -->
   
   I found some flaky tests using the [NonDex](https://github.com/TestingResearchIllinois/NonDex) Plugin for maven in the `processing` module.
   
   <!-- Replace XXXX with the id of the issue fixed in this PR. Remove this section if there is no corresponding issue. Don't reference the issue in the title of this pull-request. -->
   
   <!-- If you are a committer, follow the PR action item checklist for committers:
   https://github.com/apache/druid/blob/master/dev/committer-instructions.md#pr-and-issue-action-item-checklist-for-committers. -->
   
   ### Description
   
   <!-- Describe the goal of this PR, what problem are you fixing. If there is a corresponding issue (referenced above), it's not necessary to repeat the description here, however, you may choose to keep one summary sentence. -->
   
   <!-- Describe your patch: what did you change in code? How did you fix the problem? -->
   
   <!-- If there are several relatively logically separate changes in this PR, create a mini-section for each of them. For example: -->
   
   #### Fixed the following flaky tests:
   1. org.apache.druid.segment.nested.NestedDataColumnSupplierV4Test#testBasicFunctionality
   2. org.apache.druid.segment.nested.NestedDataColumnSupplierV4Test#testConcurrency
   3. org.apache.druid.segment.nested.NestedDataColumnSupplierTest#testBasicFunctionality
   4. org.apache.druid.segment.nested.NestedDataColumnSupplierTest#testArrayFunctionality
   5. org.apache.druid.segment.nested.NestedDataColumnSupplierTest#testConcurrency
   6. org.apache.druid.segment.virtual.ExpressionVirtualColumnTest#testRequiredColumns
   
   #### Problem:
   The above mentioned tests have been reported as flaky (tests assuming deterministic implementation of a non-deterministic specification ) when ran against the [NonDex](https://github.com/TestingResearchIllinois/NonDex) tool. 
   The tests contain assertions that compare strings created from JSON objects([Assertion 1](https://github.com/apache/druid/blob/fb260f3e412522a4993d43209b2bc20cb3dae80c/processing/src/test/java/org/apache/druid/segment/nested/NestedDataColumnSupplierTest.java#L408) and lists created from HashSets & [Assertion 2](https://github.com/apache/druid/blob/fb260f3e412522a4993d43209b2bc20cb3dae80c/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVirtualColumnTest.java#L737)). 
   
   However, HashSet does not guarantee the ordering of elements and thus resulting in these flaky tests that assume deterministic implementation of HashSet. Also, 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. This results in flakiness as the Jackson ObjectMapper does not guarantee consistent ordering of the JSON keys.
   
   Thus, when the NonDex tool shuffles the HashSet elements and the JSON keys, it results in test failures like:
   
   
   ```
   [ERROR] Failures: 
   [ERROR] org.apache.druid.segment.nested.NestedDataColumnSupplierTest.testArrayFunctionality
   [ERROR]   Run 1: NestedDataColumnSupplierTest.testArrayFunctionality:291->smokeTestArrays:644 expected:<{"[s":["b","c"],"l":[null,null],"d":[1.1,null,2.2]]}> but was:<{"[d":[1.1,null,2.2],"s":["b","c"],"l":[null,null]]}>
   [ERROR]   Run 2: NestedDataColumnSupplierTest.testArrayFunctionality:291->smokeTestArrays:644 expected:<{"s":["a","b","c"],"[l":[1,2,3],"d":[1.1,2.2]]}> but was:<{"s":["a","b","c"],"[d":[1.1,2.2],"l":[1,2,3]]}>
   [ERROR]   Run 3: NestedDataColumnSupplierTest.testArrayFunctionality:291->smokeTestArrays:644 expected:<{"[d":[1.1,2.2],"l":[1,2,3]],"s":["a","b","c"]}> but was:<{"[l":[1,2,3],"d":[1.1,2.2]],"s":["a","b","c"]}>
   [ERROR]   Run 4: NestedDataColumnSupplierTest.testArrayFunctionality:291->smokeTestArrays:644 expected:<{"[l":[null,null],"d":[1.1,null,2.2],"s":["b","c"]]}> but was:<{"[s":["b","c"],"d":[1.1,null,2.2],"l":[null,null]]}>
   
   -----------------
   
   [ERROR] org.apache.druid.segment.virtual.ExpressionVirtualColumnTest.testRequiredColumns
   [ERROR]   Run 1: ExpressionVirtualColumnTest.testRequiredColumns:737 expected:<[x, y]> but was:<[y, x]>
   
   ```
   
   To reproduce run:
   ```
   mvn -pl processing edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=<test_name>
   ```
   
   <!--
   In each section, please describe design decisions made, including:
    - Choice of algorithms
    - Behavioral aspects. What configuration values are acceptable? How are corner cases and error conditions handled, such as when there are insufficient resources?
    - Class organization and design (how the logic is split between classes, inheritance, composition, design patterns)
    - Method organization and design (how the logic is split between methods, parameters and return types)
    - Naming (class, method, API, configuration, HTTP endpoint, names of emitted metrics)
   -->
   
   
   <!-- It's good to describe an alternative design (or mention an alternative name) for every design (or naming) decision point and compare the alternatives with the designs that you've implemented (or the names you've chosen) to highlight the advantages of the chosen designs and names. -->
   
   <!-- If there was a discussion of the design of the feature implemented in this PR elsewhere (e. g. a "Proposal" issue, any other issue, or a thread in the development mailing list), link to that discussion from this PR description and explain what have changed in your final design compared to your original proposal or the consensus version in the end of the discussion. If something hasn't changed since the original discussion, you can omit a detailed discussion of those aspects of the design here, perhaps apart from brief mentioning for the sake of readability of this PR description. -->
   
   <!-- Some of the aspects mentioned above may be omitted for simple and small changes. -->
   
   <!-- Give your best effort to summarize your changes in a couple of sentences aimed toward Druid users. 
   
   If your change doesn't have end user impact, you can skip this section.
   
   For tips about how to write a good release note, see [Release notes](https://github.com/apache/druid/blob/master/CONTRIBUTING.md#release-notes).
   
   -->
   
   #### Fix:
   For the test `org.apache.druid.segment.virtual.ExpressionVirtualColumnTest#testRequiredColumns`, first convert the arraylists to hashSets and then compare the 2 HashSets using assertEquals().
   
   For all other tests in `NestedDataColumnSupplierTest` and `NestedDataColumnSupplierV4Test` classes, first create the JSON node trees and then compare the two trees using assertEquals().
   
   ##### Key changed/added classes in this PR
    * `org.apache.druid.segment.nested.NestedDataColumnSupplierV4Test`
    * `org.apache.druid.segment.nested.NestedDataColumnSupplierTest`
    * `org.apache.druid.segment.virtual.ExpressionVirtualColumnTest`
   
   <hr>
   
   <!-- Check the items by putting "x" in the brackets for the done things. Not all of these items apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items from the checklist below are strictly necessary, but it would be very helpful if you at least self-review the PR. -->
   
   This PR has:
   
   - [x] been self-reviewed.
   - [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
   


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "yashdeep97 (via GitHub)" <gi...@apache.org>.
yashdeep97 commented on code in PR #15418:
URL: https://github.com/apache/druid/pull/15418#discussion_r1428978984


##########
services/src/test/java/org/apache/druid/server/router/TieredBrokerHostSelectorTest.java:
##########
@@ -367,18 +368,18 @@ public void testGetAllBrokers()
   {
     Assert.assertEquals(
         ImmutableMap.of(
-            "mediumBroker", ImmutableList.of(),
-            "coldBroker", ImmutableList.of("coldHost1:8080", "coldHost2:8080"),
-            "hotBroker", ImmutableList.of("hotHost:8080")
+            "mediumBroker", ImmutableSet.of(),

Review Comment:
   So, it used `servers` which is a `ConcurrentHashMap` to create the list of Hostnames.(https://github.com/apache/druid/blob/e373f6269251655f5be93ce895aee8dee8cc67dd/services/src/main/java/org/apache/druid/server/router/TieredBrokerHostSelector.java#L63)
   
   And ConcurrentHashMap does not preserve the the ordering of the keys, which can result in the order of servers being different that what is expected in the Assert.
   For example: we might get ImmutableList.of("coldHost2:8080", "coldHost1:8080") instead of ImmutableList.of("coldHost1:8080", "coldHost2:8080") which will result in the test failing.
   Thus, the code incorrectly assumes deterministic ordering of the keys in ConcurrentHashMap.



-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "yashdeep97 (via GitHub)" <gi...@apache.org>.
yashdeep97 commented on PR #15418:
URL: https://github.com/apache/druid/pull/15418#issuecomment-1836907718

   Hi @cryptoe,
   I have merged all the pending PRs from students in the course, and updated the PR description.
   Please let me know if you need further clarification or if any changes are necessary.


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15418:
URL: https://github.com/apache/druid/pull/15418#issuecomment-1989680645

   This pull request has been marked as stale due to 60 days of inactivity.
   It will be closed in 4 weeks if no further activity occurs. If you think
   that's incorrect or this pull request should instead be reviewed, please simply
   write any comment. Even if closed, you can still revive the PR at any time or
   discuss it on the dev@druid.apache.org list.
   Thank you for your contributions.


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "cryptoe (via GitHub)" <gi...@apache.org>.
cryptoe commented on code in PR #15418:
URL: https://github.com/apache/druid/pull/15418#discussion_r1421972298


##########
services/src/test/java/org/apache/druid/server/router/TieredBrokerHostSelectorTest.java:
##########
@@ -367,18 +368,18 @@ public void testGetAllBrokers()
   {
     Assert.assertEquals(
         ImmutableMap.of(
-            "mediumBroker", ImmutableList.of(),
-            "coldBroker", ImmutableList.of("coldHost1:8080", "coldHost2:8080"),
-            "hotBroker", ImmutableList.of("hotHost:8080")
+            "mediumBroker", ImmutableSet.of(),

Review Comment:
   Why is this change required ?



##########
extensions-core/parquet-extensions/src/test/java/org/apache/druid/data/input/parquet/BaseParquetReaderTest.java:
##########
@@ -39,6 +39,7 @@
 class BaseParquetReaderTest extends InitializedNullHandlingTest
 {
   ObjectWriter DEFAULT_JSON_WRITER = new ObjectMapper().writerWithDefaultPrettyPrinter();
+  protected final ObjectMapper objectMapper = new ObjectMapper();

Review Comment:
   Nit: you can use the object mapper created on line 42 for line 41 and remove the inline instantiation of the object mapper.



##########
extensions-core/parquet-extensions/src/test/java/org/apache/druid/data/input/parquet/CompatParquetReaderTest.java:
##########
@@ -94,19 +94,16 @@ public void testBinaryAsString() throws IOException
                                 + "  \"field\" : \"hey this is &é(-è_çà)=^$ù*! Ω^^\",\n"
                                 + "  \"ts\" : 1471800234\n"
                                 + "}";
-    Assert.assertEquals(expectedJson, DEFAULT_JSON_WRITER.writeValueAsString(sampled.get(0).getRawValues()));
-
+    Assert.assertEquals(objectMapper.readTree(expectedJson), objectMapper.readTree(DEFAULT_JSON_WRITER.writeValueAsString(sampled.get(0).getRawValues())));
     final String expectedJsonBinary = "{\n"
                                 + "  \"field\" : \"aGV5IHRoaXMgaXMgJsOpKC3DqF/Dp8OgKT1eJMO5KiEgzqleXg==\",\n"
                                 + "  \"ts\" : 1471800234\n"
                                 + "}";
     Assert.assertEquals(
-        expectedJsonBinary,
-        DEFAULT_JSON_WRITER.writeValueAsString(sampledAsBinary.get(0).getRawValues())
-    );
+        objectMapper.readTree(expectedJsonBinary), 

Review Comment:
   Create a json test utils and then use that to do the assert everywhere. 
   You can put that class in druid core I think. 



-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in processing module (druid)

Posted by "yashdeep97 (via GitHub)" <gi...@apache.org>.
yashdeep97 commented on PR #15418:
URL: https://github.com/apache/druid/pull/15418#issuecomment-1831157677

   Hi @cryptoe, 
   Absolutely! I will reach out to others who have similar PRs and try to get them to merge into one.
   Also, would it be preferable to have 1 PR for each module (eg. processing, sql etc. ) or a combined PR for tests in all modules? 


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "kfaraz (via GitHub)" <gi...@apache.org>.
kfaraz commented on PR #15418:
URL: https://github.com/apache/druid/pull/15418#issuecomment-1886347338

   Have all of these tests been reported as flaky? I am not sure if that is really the case. These changes seem to be using sets instead of lists in a lot of places and changing equality assertions of json strings to JsonNode objects. Are these changes really needed?
   
   If there is indeed a flaky test, please share links in the PR description to one or more builds where these tests actually failed. For cases where there is a flake, we should try to figure out exactly what is causing the flakiness and update tests only when we fully understand the intent of the original test.
   
   cc: @yashdeep97 , @cryptoe 


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "yashdeep97 (via GitHub)" <gi...@apache.org>.
yashdeep97 commented on PR #15418:
URL: https://github.com/apache/druid/pull/15418#issuecomment-1836922435

   Merged changes from the following PRs:
   1. https://github.com/apache/druid/pull/15244
   2. https://github.com/apache/druid/pull/15392
   3. https://github.com/apache/druid/pull/15105


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15418:
URL: https://github.com/apache/druid/pull/15418#issuecomment-2043921073

   This pull request/issue has been closed due to lack of activity. If you think that
   is incorrect, or the pull request requires review, you can revive the PR at any time.


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in processing module (druid)

Posted by "yashdeep97 (via GitHub)" <gi...@apache.org>.
yashdeep97 commented on PR #15418:
URL: https://github.com/apache/druid/pull/15418#issuecomment-1823808800

   Hi @cryptoe, thanks for reviewing my previous Pull request (https://github.com/apache/druid/pull/15318)!
   Could you please review this one as well, it's quite similar to the previous one. (I found more flaky tests in other classes of the processing module)


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "yashdeep97 (via GitHub)" <gi...@apache.org>.
yashdeep97 commented on code in PR #15418:
URL: https://github.com/apache/druid/pull/15418#discussion_r1428977283


##########
extensions-core/parquet-extensions/src/test/java/org/apache/druid/data/input/parquet/CompatParquetReaderTest.java:
##########
@@ -94,19 +94,16 @@ public void testBinaryAsString() throws IOException
                                 + "  \"field\" : \"hey this is &é(-è_çà)=^$ù*! Ω^^\",\n"
                                 + "  \"ts\" : 1471800234\n"
                                 + "}";
-    Assert.assertEquals(expectedJson, DEFAULT_JSON_WRITER.writeValueAsString(sampled.get(0).getRawValues()));
-
+    Assert.assertEquals(objectMapper.readTree(expectedJson), objectMapper.readTree(DEFAULT_JSON_WRITER.writeValueAsString(sampled.get(0).getRawValues())));
     final String expectedJsonBinary = "{\n"
                                 + "  \"field\" : \"aGV5IHRoaXMgaXMgJsOpKC3DqF/Dp8OgKT1eJMO5KiEgzqleXg==\",\n"
                                 + "  \"ts\" : 1471800234\n"
                                 + "}";
     Assert.assertEquals(
-        expectedJsonBinary,
-        DEFAULT_JSON_WRITER.writeValueAsString(sampledAsBinary.get(0).getRawValues())
-    );
+        objectMapper.readTree(expectedJsonBinary), 

Review Comment:
   Hi @cryptoe, I'm unable to figure out where to place this new class that does assert after converting it to JSON tree, should I place it under the druid-testing-tools module in extensions-core? (I did not understand what druid core refers to)



-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "yashdeep97 (via GitHub)" <gi...@apache.org>.
yashdeep97 commented on code in PR #15418:
URL: https://github.com/apache/druid/pull/15418#discussion_r1428978984


##########
services/src/test/java/org/apache/druid/server/router/TieredBrokerHostSelectorTest.java:
##########
@@ -367,18 +368,18 @@ public void testGetAllBrokers()
   {
     Assert.assertEquals(
         ImmutableMap.of(
-            "mediumBroker", ImmutableList.of(),
-            "coldBroker", ImmutableList.of("coldHost1:8080", "coldHost2:8080"),
-            "hotBroker", ImmutableList.of("hotHost:8080")
+            "mediumBroker", ImmutableSet.of(),

Review Comment:
   So, it uses `servers` which is a `ConcurrentHashMap` to create the list of Hostnames.
   https://github.com/apache/druid/blob/e373f6269251655f5be93ce895aee8dee8cc67dd/services/src/main/java/org/apache/druid/server/router/TieredBrokerHostSelector.java#L63
   
   And ConcurrentHashMap does not preserve the the ordering of the keys, which can result in the order of servers being different that what is expected in the Assert.
   For example: we might get ImmutableList.of("coldHost2:8080", "coldHost1:8080") instead of ImmutableList.of("coldHost1:8080", "coldHost2:8080") which will result in the test failing.
   Thus, the code incorrectly assumes deterministic ordering of the keys in ConcurrentHashMap.



-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in processing module (druid)

Posted by "cryptoe (via GitHub)" <gi...@apache.org>.
cryptoe commented on PR #15418:
URL: https://github.com/apache/druid/pull/15418#issuecomment-1829610923

   @yashdeep97 Could you and the other folks working with the https://github.com/TestingResearchIllinois/NonDex raise a single PR with the changes. 


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "cryptoe (via GitHub)" <gi...@apache.org>.
cryptoe commented on PR #15418:
URL: https://github.com/apache/druid/pull/15418#issuecomment-1875033980

   Can you also include https://github.com/apache/druid/pull/15268 if not included already. 


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #15418: Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects
URL: https://github.com/apache/druid/pull/15418


-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

Re: [PR] Fix flaky tests in multiple modules due to non-deterministic specification of HashSet and JSON objects (druid)

Posted by "yashdeep97 (via GitHub)" <gi...@apache.org>.
yashdeep97 commented on code in PR #15418:
URL: https://github.com/apache/druid/pull/15418#discussion_r1427292111


##########
extensions-core/parquet-extensions/src/test/java/org/apache/druid/data/input/parquet/CompatParquetReaderTest.java:
##########
@@ -94,19 +94,16 @@ public void testBinaryAsString() throws IOException
                                 + "  \"field\" : \"hey this is &é(-è_çà)=^$ù*! Ω^^\",\n"
                                 + "  \"ts\" : 1471800234\n"
                                 + "}";
-    Assert.assertEquals(expectedJson, DEFAULT_JSON_WRITER.writeValueAsString(sampled.get(0).getRawValues()));
-
+    Assert.assertEquals(objectMapper.readTree(expectedJson), objectMapper.readTree(DEFAULT_JSON_WRITER.writeValueAsString(sampled.get(0).getRawValues())));
     final String expectedJsonBinary = "{\n"
                                 + "  \"field\" : \"aGV5IHRoaXMgaXMgJsOpKC3DqF/Dp8OgKT1eJMO5KiEgzqleXg==\",\n"
                                 + "  \"ts\" : 1471800234\n"
                                 + "}";
     Assert.assertEquals(
-        expectedJsonBinary,
-        DEFAULT_JSON_WRITER.writeValueAsString(sampledAsBinary.get(0).getRawValues())
-    );
+        objectMapper.readTree(expectedJsonBinary), 

Review Comment:
   Hi @cryptoe, Thanks for your feedback, I'll work on making these changes over this weekend



-- 
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@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org