-
Notifications
You must be signed in to change notification settings - Fork 141
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
[ISSUE-472] Fix Flaky Test: LocalFileServerReadHandlerTest#testDataInconsistent #473
Conversation
Codecov Report
@@ Coverage Diff @@
## master #473 +/- ##
=========================================
Coverage 58.78% 58.78%
Complexity 1704 1704
=========================================
Files 206 206
Lines 11471 11471
Branches 1024 1024
=========================================
Hits 6743 6743
Misses 4317 4317
Partials 411 411 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
+1
I'm not sure if there is a more graceful way. But let's fix the problem first.
Thanks @kaijchen |
@@ -43,6 +43,8 @@ | |||
public class LocalFileServerReadHandlerTest { | |||
@Test | |||
public void testDataInconsistent() throws Exception { | |||
LocalFileHandlerTestBase.reset(); |
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.
is it necessary to put it under a beforeEach
block?
org.apache.uniffle.storage.handler.impl.LocalFileHandlerTest
also uses LocalFileHandlerTestBase, is it necessary to modify that class too?
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 think we should refactor LocalFileHandlerTestBase
in a later PR.
…stDataInconsistent (apache#473)" This reverts commit 7f89d6f.
### What changes were proposed in this pull request? Split `LocalFileHandlerTestBase#writeTestData()` into `generateBlocks()` and `writeTestData()`. Pass `List<Long> blockIds` into `LocalFileHandlerTestBase#calcSegmentBytes()`. ### Why are the changes needed? Followup #473. To make the expected result corresponding to the state of `LocalFileHandlerTestBase#ATOMIC_LONG`. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? CI.
…consistent (#473) ### What changes were proposed in this pull request? 1. Reset the static variable to fix LocalFileServerReadHandlerTest#testDataInconsistent ### Why are the changes needed? Fix flaky test ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? 1. Existing UTs
What changes were proposed in this pull request?
Why are the changes needed?
Fix flaky test
Does this PR introduce any user-facing change?
No
How was this patch tested?