-
Notifications
You must be signed in to change notification settings - Fork 14.6k
KAFKA-19683: Remove dead tests and modify tests in TaskManagerTest [1/N] #20501
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: trunk
Are you sure you want to change the base?
Conversation
@@ -1853,14 +1812,20 @@ public void shouldReleaseLockForUnassignedTasksAfterRebalanceWithStateUpdater() | |||
} | |||
|
|||
@Test | |||
public void shouldReportLatestOffsetAsOffsetSumForRunningTask() throws Exception { |
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.
as suggested in #19275 (comment)
final StreamTask runningStatefulTask = statefulTask(taskId00, taskId00ChangelogPartitions) | ||
.inState(State.RUNNING).build(); | ||
final StreamTask restoringStatefulTask = statefulTask(taskId01, taskId01ChangelogPartitions) | ||
.inState(State.RESTORING).build(); | ||
final StandbyTask restoringStandbyTask = standbyTask(taskId02, taskId02ChangelogPartitions) | ||
.inState(State.RUNNING).build(); | ||
final long changelogOffsetOfRunningTask = 42L; | ||
final long changelogOffsetOfRunningTask = Task.LATEST_OFFSET; |
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.
as suggested in #19275 (comment)
@@ -1943,14 +1908,26 @@ public void shouldComputeOffsetSumForRunningStatefulTaskAndRestoringTaskWithStat | |||
} | |||
|
|||
@Test | |||
public void shouldSkipUnknownOffsetsWhenComputingOffsetSum() throws Exception { |
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.
as suggested in #19275 (comment)
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.
LGTM
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.
Pull Request Overview
This PR cleans up test code in TaskManagerTest
by removing dead tests and adding new ones to improve test coverage and maintainability.
- Removes three obsolete test methods that are no longer needed
- Adds a new utility method for setting up TaskManager instances
- Refactors two existing test methods to use a more direct testing approach with mocked dependencies
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@shashankhs11 please rebase this on trunk, CI keeps failing on flaky tests that are already fixed |
141e09a
to
d58bbe0
Compare
This is the first part of cleaning up of the tests in
TaskManagerTest
Reviewers: Lucas Brutschy [email protected]