Skip to content

KAFKA-19683: Clean up TaskManagerTest #20994

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from
Open

KAFKA-19683: Clean up TaskManagerTest #20994

wants to merge 6 commits into from

Conversation

@shashankhs11
Copy link
Contributor

@shashankhs11 shashankhs11 commented Nov 25, 2025 •
edited by github-actions bot
Loading

Final cleanup of TaskManagerTest.java

  • Renamed setUpTaskManagerWithStateUpdater to setUpTaskManager
  • Removed WithStateUpdaterEnabled from test names
  • Removed the method setUpTaskManagerWithoutStateUpdater
  • Rewrote the newly added test
    shouldCloseTasksIfStateUpdaterTimesOutOnRemove to use new mocking
    patterns
  • Completely removed the utility class StateMachineTask and also
    handleAssignment method

Reviewers: Lucas Brutschy lbrutschy@confluent.io

@github-actions github-actions bot added triage PRs from the community streams labels Nov 25, 2025
@github-actions github-actions bot added the tests Test fixes (including flaky tests) label Nov 26, 2025
@shashankhs11 shashankhs11 marked this pull request as ready for review November 26, 2025 14:31
@shashankhs11
Copy link
Contributor Author

shashankhs11 commented Nov 26, 2025

@lucasbru, tagging for review. Should hopefully be the last one for the test file :)

lucasbru
lucasbru reviewed Nov 28, 2025
View reviewed changes
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. There are few tests that are still called "...WithStateUpdater", I suppose we can drop this suffix now

@lucasbru lucasbru self-assigned this Nov 28, 2025
@github-actions github-actions bot removed the triage PRs from the community label Nov 29, 2025
@shashankhs11
Copy link
Contributor Author

shashankhs11 commented Nov 29, 2025

LGTM. There are few tests that are still called "...WithStateUpdater", I suppose we can drop this suffix now

Done! Removed them in 7b07885

Thank you for your patience and all the reviews, Lucas :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucasbru lucasbru Awaiting requested review from lucasbru

At least 1 approving review is required to merge this pull request.

Assignees

@lucasbru lucasbru

Labels

ci-approved streams tests Test fixes (including flaky tests)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shashankhs11 @lucasbru