Skip to content

KAFKA-14684 Replace EasyMock and PowerMock with Mockito in WorkerSinkTaskThreadedTest#14505

Merged
mimaison merged 2 commits intoapache:trunkfrom
hgeraldino:WorkerSinkTaskThreadedTest-mockito
Oct 16, 2023
Merged

KAFKA-14684 Replace EasyMock and PowerMock with Mockito in WorkerSinkTaskThreadedTest#14505
mimaison merged 2 commits intoapache:trunkfrom
hgeraldino:WorkerSinkTaskThreadedTest-mockito

Conversation

@hgeraldino
Copy link
Copy Markdown
Contributor

@hgeraldino hgeraldino commented Oct 5, 2023 •
edited
Loading

JIRA: https://issues.apache.org/jira/browse/KAFKA-14684

Previous PR got closed somehow

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@hgeraldino
Copy link
Copy Markdown
Contributor Author

hgeraldino commented Oct 5, 2023

Hey @C0urante @gharris1727 @yashmayya @clolov

I was pleasantly surprised to find out that only 4! of the connect:runtime tests are pending to be migrated. Here's my attempt to migrate WorkerSinkTaskThreadedTest; once the PR is settled (it's pretty similar to what I did on the AbstractSourceTaskTest and WorkerSourceTaskTest migrations) I'll submit the final WorkerSinkTaskTest PR

I look forward for your feedback

@divijvaidya divijvaidya added the tests Test fixes (including flaky tests) label Oct 6, 2023
@clolov clolov assigned clolov and unassigned clolov Oct 9, 2023
clolov
clolov reviewed Oct 10, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@clolov clolov left a comment

Choose a reason for hiding this comment

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

Heya and thank you for the contribution! Apologies, but I am not too knowledgeable with Connect which makes it quite difficult for me to confirm the correctness of the changes in this pull request. Is it possible that you break this down (purely for review purposes) in a way similar to the one proposed by Bruno here (#12607 (comment))?

Comment thread connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerSinkTaskThreadedTest.java Outdated
Comment thread connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerSinkTaskThreadedTest.java Outdated
mimaison
mimaison reviewed Oct 13, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@mimaison mimaison left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. It looks good overall. Can you address the small comments @clolov and I left?

Comment thread connect/runtime/src/test/java/org/apache/kafka/connect/runtime/WorkerSinkTaskThreadedTest.java Outdated
mimaison
mimaison approved these changes Oct 16, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@mimaison mimaison left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

@mimaison mimaison merged commit 4150595 into apache:trunk Oct 16, 2023
AnatolyPopov pushed a commit to aiven/kafka that referenced this pull request Feb 16, 2024
@hgeraldino @AnatolyPopov
KAFKA-14684: Replace EasyMock/PowerMock with Mockito in WorkerSinkTas…
c074114 
…kThreadedTest (apache#14505)


Reviewers: Mickael Maison <mickael.maison@gmail.com>, Christo Lolov <christololov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clolov clolov clolov left review comments

@mimaison mimaison mimaison approved these changes

Assignees

No one assigned

Labels

connect tests Test fixes (including flaky tests)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@hgeraldino @mimaison @clolov @divijvaidya @yashmayya