KAFKA-19683: More test replacements [5/N]

[shashankhs11]

Replaced more tests in TaskManagerTest.java

Reviewers: Lucas Brutschy [email protected]

[shashankhs11]

This test tests the same behaviour as the above test shouldNotCommitIfNoRevokedTasksNeedCommitting.
I wasn't able to understand/figure out on how the behavior varies through the different processing modes.

Just for my understanding, it would differ only if the revoked tasks needs committing right?

[lucasbru]

yes. We could consider parametrizing on ProcessingMode here.

[shashankhs11]

done in ff6b3b0

[shashankhs11]

@lucasbru, tagging for review :)

[Copilot]

Pull Request Overview

This PR refactors test methods in TaskManagerTest.java to use mocked StreamTask and StandbyTask objects created via a builder pattern instead of the custom StateMachineTask test helper class. The changes improve test maintainability by using more realistic mocks and explicit verification of task interactions.

• Replaces StateMachineTask with mocked StreamTask/StandbyTask using the builder pattern
• Updates test setup to use mocked TasksRegistry instead of relying on task creation and state management
• Converts verification from state assertions to explicit mock verification calls

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lucasbru]

thanks a lot, but it seems there is a little gap in the state updater code here, since we do not handle timeout exceptions during initializations as before. I wonder if we could fix it in this PR, as it shouldn't be a huge change?

[lucasbru]

why did you remove verifyNoInteractions(consumer)?

[shashankhs11]

added back in 7d74633

[lucasbru]

Wait. We should actually also handle TimeoutException in the production code. I suppose you removed TimeoutException because state updater doesn't handle it.

We should fix that, because initializeIfNeeded may throw a timeout exception.

If we get a TimeoutException inside addTasksToStateUpdater, we should

            task.maybeInitTaskTimeoutOrThrow(now, timeoutException);
            tasks.addPendingTasksToInit(Collections.singleton(task));
            updateOrCreateBackoffRecord(task.id(), nowMs);

and otherwise, we should

                task.clearTaskTimeout();

after the initialization.

And then we should bring back this behavior in the test

[shashankhs11]

Opened KAFKA-19864 to track and isolate this change -- #20829

[lucasbru]

yes. We could consider parametrizing on ProcessingMode here.

[shashankhs11]

@lucasbru

I wonder if we could fix it in this PR, as it shouldn't be a huge change?

I created KAFKA-19864 to track and isolate this change -- #20829 is ready for review.

The thinking behind this was that since the commits are squashed and merged to trunk and this PR also updates few other tests, I thought it would be better if we file isolate this change in case we need to revert for any reason in the future.