Skip to content

KAFKA-10070: parameterize Connect unit tests to remove code duplication#10299

Merged
mimaison merged 3 commits intoapache:trunkfrom
levzem:KAFKA-10070
Mar 19, 2021
Merged

KAFKA-10070: parameterize Connect unit tests to remove code duplication#10299
mimaison merged 3 commits intoapache:trunkfrom
levzem:KAFKA-10070

Conversation

@levzem
Copy link
Copy Markdown
Contributor

@levzem levzem commented Mar 11, 2021

Signed-off-by: Lev Zemlyanov lev@confluent.io

Parameterize Connect unit tests to remove code duplication. Some PRs added duplicate test classes to test topic creation. This fix parameterizes the topic creation to reduce code duplication.

The topic creation tests were copied over to the original test classes to capture any changes to reduce risk of test coverage.

Committer Checklist (excluded from commit message)

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

@levzem
Copy link
Copy Markdown
Contributor Author

levzem commented Mar 11, 2021

@C0urante @kkonstantine @rhauch would appreciate some 👀 , thanks!

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. That's a pretty nice cleanup!
I've only taken a quick look so far but there are a few small formatting issues that are failing checkstyle

Comment thread checkstyle/suppressions.xml Outdated
Lev Zemlyanov added 3 commits March 12, 2021 12:39
Signed-off-by: Lev Zemlyanov <lev@confluent.io>
Signed-off-by: Lev Zemlyanov <lev@confluent.io>
Signed-off-by: Lev Zemlyanov <lev@confluent.io>
@levzem
Copy link
Copy Markdown
Contributor Author

levzem commented Mar 12, 2021

tests failing are unrelated to the changes

Build / JDK 11 / kafka.api.PlaintextConsumerTest.testAutoCommitOnCloseAfterWakeup()
Build / JDK 15 / org.apache.kafka.connect.integration.RebalanceSourceConnectorsIntegrationTest.testMultipleWorkersRejoining

@mimaison thanks for the review, mind taking another pass?

Copy link
Copy Markdown
Contributor

@kkonstantine kkonstantine left a comment

Choose a reason for hiding this comment

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

Thanks @levzem
Glad to see there's a workaround to make parameterized tests work with powermock.

LGTM, but I'll wait for @mimaison's final approval.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants