KAFKA-19683: More cleanup and rewrite [4/N] #20777
Conversation
github-actions
bot
added
triage
|
@lucasbru tagging for review :) |
There was a problem hiding this comment.
Pull Request Overview
This PR is part 4 of a cleanup and rewrite effort (KAFKA-19683) that removes tagged-for-deletion tests and rewrites existing test cases in the TaskManagerTest file. The changes modernize test structure by replacing legacy StateMachineTask mock implementations with builder-based task creation and Mockito mocks, moving toward a more maintainable and clearer testing approach.
Key Changes:
- Removed legacy test methods that relied on custom
StateMachineTasksubclasses with inline behavior overrides - Rewrote tests using builder patterns (
statefulTask(),standbyTask()) and Mockito verification for cleaner test setup - Removed exactly-once-v2 (EOS V2) specific tests that are no longer needed
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @SuppressWarnings("removal") | ||
| @Test | ||
| public void shouldCommitAllActiveTasksThatNeedCommittingOnHandleRevocationWithEosV2() { |
There was a problem hiding this comment.
Why are you removing this without replacement?
There was a problem hiding this comment.
Honestly, I removed them blindly only since they were marked for removal. Another oversight by me 😓
I'll replace them in another commit.
|
|
||
| @SuppressWarnings("removal") | ||
| @Test | ||
| public void shouldCommitAllActiveTasksThatNeedCommittingOnHandleRevocationWithEosV2() { |
There was a problem hiding this comment.
Why are you removing this without replacement?
|
|
||
| @SuppressWarnings("removal") | ||
| @Test | ||
| public void shouldCommitViaProducerIfEosV2Enabled() { |
There was a problem hiding this comment.
Why are you removing this without replacement?
|
|
||
| @SuppressWarnings("removal") | ||
| @Test | ||
| public void shouldCommitAllActiveTasksThatNeedCommittingOnHandleRevocationWithEosV2() { |
There was a problem hiding this comment.
Why are you removing this without replacement?
There was a problem hiding this comment.
@shashankhs11 would you mind merging trunk to include #20786?
3 participants
Reviewers: Lucas Brutschy [email protected]