KAFKA-14683 Migrate WorkerSinkTaskTest to Mockito (3/3)

[hgeraldino]

Last batch for migrating WorkerSinkTaskTest tests to Mockito

[divijvaidya]

As part of this last batch, please also removed this test from the list here:

kafka/build.gradle

Line 424 in 5cfcc52

"**/WorkerSinkTaskTest.*"

[hgeraldino]

Final batch of the WorkerSinkTaskTest migration 🎉 🎉 🎉

This one is probably the largest of the 3 (sorry).
We can either do one final commit renaming the test from WorkerSinkTaskTestMockito back to WorkerSinkTastTest, or just open a new PR once this gets merged to do the cleanup

Tagging @divijvaidya @gharris1727 @C0urante

[gharris1727]

Thanks @hgeraldino, that last test is especially complex. Thank you for your attention to detail here!

[gharris1727]

in the original test:

This confuses me. The earlier 3rd poll assigned topic-partition 3, and iteration 3 paused the newAssignment, including topic-partition 3. Yet in the 5th poll, this record is returned for a partition which should be paused.

The 5th iteration has the put succeed, and then the partitions are unpaused. But this unpause should only allow for new records to appear in the 6th poll, right?

[hgeraldino]

You're right, the test was a bit off mainly because the first call to task.put(..) shouldn't have thrown an exception.

Here's the sequence now (which matches the original WorkerSinkTaskTest):

1. Iteration#1: partitions on INITIAL_ASSIGNMENT are assigned
2. Iteration#2: task.put(...) throws, partitions are paused
3. Iteration#3: P3 is assigned, task.put(...) throws (task is already paused so it's a noop)
4. Iteration#4: task.put(...) throws, noop
5. Iteration#5: initial assignment is revoked (pending messages for P1 are removed); consumer.poll(...) returns a record for P3, which is successfully processed.

I've added the corresponding verifications after each step. Hope it makes sense now

[hgeraldino]

Thanks @gharris1727 for your review. I think I addressed most of your comments.

If anyone has any other suggestions, please let me know. Ideally we should rename this file back to WorkerSinkTaskTest (not sure if as part of this PR or a separate one) and have checkstyle, build.gradle, etc. cleaned up

[gharris1727]

LGTM, Thanks @hgeraldino for all your hard work!

We can straighten out the build.gradle and move back to the old class name in a follow-up.

[gharris1727]

Test failures appear unrelated, and the runtime tests pass locally for me.