Skip to content

Conversation

shashankhs11
Copy link
Contributor

@shashankhs11 shashankhs11 commented Sep 8, 2025

This is the first part of cleaning up of the tests in TaskManagerTest

  • Removed dead tests
  • Added new tests as suggested earlier

Reviewers: Lucas Brutschy [email protected]

@github-actions github-actions bot added triage PRs from the community streams tests Test fixes (including flaky tests) labels Sep 8, 2025
@@ -1853,14 +1812,20 @@ public void shouldReleaseLockForUnassignedTasksAfterRebalanceWithStateUpdater()
}

@Test
public void shouldReportLatestOffsetAsOffsetSumForRunningTask() throws Exception {
Copy link
Contributor Author

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;
Copy link
Contributor Author

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 {
Copy link
Contributor Author

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)

@github-actions github-actions bot removed the triage PRs from the community label Sep 8, 2025
@shashankhs11 shashankhs11 changed the title KAFKA-19683: Clean up TaskManagerTest KAFKA-19683: Remove dead tests and modify tests in TaskManagerTest [1/N] Sep 8, 2025
@lucasbru lucasbru self-assigned this Sep 10, 2025
Copy link
Member

@lucasbru lucasbru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lucasbru lucasbru requested a review from Copilot September 11, 2025 14:06
Copy link
Contributor

@Copilot Copilot AI left a 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.

@lucasbru
Copy link
Member

@shashankhs11 please rebase this on trunk, CI keeps failing on flaky tests that are already fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-approved streams tests Test fixes (including flaky tests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants