-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Fix flaky test in TestLazyBinaryColumnarSerDe by using LinkedHashMap #5791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
@check-spelling-bot Report🔴 Please reviewSee the files view or the action log for details. Unrecognized words (1)ddd Previously acknowledged words that are now absentaarry bytecode HIVEFETCHOUTPUTSERDE timestamplocal yyyyTo accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands... in a clone of the [email protected]:seltepu/hive-forked.git repository
If the flagged items do not appear to be textIf items relate to a ...
|
@@ -165,7 +166,7 @@ public void testLazyBinaryColumnarSerDeWithEmptyBinary() throws SerDeException { | |||
outerStruct.mByte = 101; | |||
outerStruct.mShort = 2002; | |||
outerStruct.mInt = 3003; | |||
outerStruct.mLong = 4004l; | |||
outerStruct.mLong = 40041; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably, the original value is intended. Or we can use an uppercase.
outerStruct.mLong = 40041; | |
outerStruct.mLong = 4004L; |
@@ -234,7 +235,7 @@ public void testSerDeInnerNulls() throws SerDeException { | |||
outerStruct.mArray = new ArrayList<InnerStruct>(2); | |||
outerStruct.mArray.add(is1); | |||
outerStruct.mArray.add(is2); | |||
outerStruct.mMap = new HashMap<String, InnerStruct>(); | |||
outerStruct.mMap = new LinkedHashMap<String, InnerStruct>(); //fixed line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we don't need the additional comment.
…nerNulls()
What changes were proposed in this pull request?
This pull request fixes a flaky test in TestLazyBinaryColumnarSerDe.java. The test was using a HashMap to store values, which does not guarantee iteration order. This nondeterministic order caused the test to sometimes fail when the output order changed. The fix replaces HashMap with LinkedHashMap to ensure a consistent iteration order and deterministic test results.
Why are the changes needed?
The test fails nondeterministically because HashMap does not guarantee iteration order. This means the serialized output can vary between runs, leading to intermittent assertion failures. Using LinkedHashMap ensures the order is preserved, making the test reliable.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
The test was run multiple times using the NonDex plugin to randomize internal order.
Before the fix, the test failed intermittently, as shown in the attached failing-test.log. After the fix, the test passed consistently across multiple runs, as shown in the attached passing-test.log.
Reproduction steps:
mvn edu.illinois:nondex-maven-plugin:2.1.1:nondex -Dtest=org.apache.hadoop.hive.serde2.columnar.TestLazyBinaryColumnarSerDe#testSerDeInnerNulls -DnondexRuns=10