Skip to content

fix flaky tests in processing module#2

Open
yashdeep97 wants to merge 7 commits intomasterfrom
fix-processing-flaky-tests
Open

fix flaky tests in processing module#2
yashdeep97 wants to merge 7 commits intomasterfrom
fix-processing-flaky-tests

Conversation

@yashdeep97
Copy link
Owner

I found some flaky tests using the NonDex Plugin for maven in the processing module.

Description

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 tool.
The tests contain assertions that compare strings created from JSON objects(Assertion 1 and lists created from HashSets & Assertion 2).

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>

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

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.

@yashdeep97 yashdeep97 marked this pull request as ready for review November 15, 2023 06:50
@yashdeep97 yashdeep97 force-pushed the fix-processing-flaky-tests branch from ac7d6a4 to c56e856 Compare December 1, 2023 01:10
@yashdeep97 yashdeep97 force-pushed the fix-processing-flaky-tests branch 2 times, most recently from 38ede59 to 73929cb Compare December 1, 2023 01:30
@yashdeep97 yashdeep97 force-pushed the fix-processing-flaky-tests branch from 3f4261e to 9d5f403 Compare December 1, 2023 01:50
prathyushreddylpr and others added 4 commits November 30, 2023 19:54
…st and FlattenSpecParquetReaderTest

Co-authored-by: lakkidi2 <lakkidi2@fa23-cs527-035.cs.illinois.edu>
Co-authored-by: lxb007981 <lxb007981@hotmail.com>
Co-authored-by: Prathyush Reddy Lakkidi <prathyushreddylakkidi@Prathyushs-MacBook-Air.local>
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.

5 participants