Skip to content
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

KAFKA-16654:Refactor kafka.test.annotation.Type and ClusterTestExtensions #15916

Merged
merged 18 commits into from May 21, 2024
Merged

KAFKA-16654:Refactor kafka.test.annotation.Type and ClusterTestExtensions #15916

merged 18 commits into from May 21, 2024

Conversation

TaiJuWu
Copy link
Contributor

@TaiJuWu TaiJuWu commented May 10, 2024 •
edited

*More detailed description of your change,
KAFKA-16654

*Summary of testing strategy (including rationale)
CI test.

Committer Checklist (excluded from commit message)

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

@TaiJuWu TaiJuWu marked this pull request as ready for review May 10, 2024 12:09
@TaiJuWu TaiJuWu marked this pull request as draft May 10, 2024 12:14
@TaiJuWu TaiJuWu marked this pull request as ready for review May 10, 2024 21:20
chia7712
chia7712 reviewed May 11, 2024
View reviewed changes
Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@TaiJuWu thanks for this patch. some comments are left.

core/src/test/java/kafka/test/annotation/Type.java Outdated Show resolved Hide resolved
core/src/test/java/kafka/test/annotation/Type.java Outdated Show resolved Hide resolved
core/src/test/java/kafka/test/annotation/Type.java Outdated Show resolved Hide resolved
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java Outdated Show resolved Hide resolved
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java Outdated Show resolved Hide resolved
@chia7712
Copy link
Contributor

@TaiJuWu please fix the conflicts :)

@TaiJuWu TaiJuWu marked this pull request as draft May 14, 2024 12:11
@TaiJuWu TaiJuWu marked this pull request as ready for review May 14, 2024 13:18
chia7712
chia7712 reviewed May 16, 2024
View reviewed changes
Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@TaiJuWu thanks for updated PR.

core/src/test/java/kafka/test/ClusterTestExtensionsTest.java Outdated Show resolved Hide resolved
tools/src/test/java/org/apache/kafka/tools/consumer/group/DeleteConsumerGroupsTest.java Outdated Show resolved Hide resolved
core/src/test/java/kafka/test/junit/ClusterTestExtensionsUnitTest.java Show resolved Hide resolved
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java Show resolved Hide resolved
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java Show resolved Hide resolved
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java Show resolved Hide resolved
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java Outdated Show resolved Hide resolved
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java Show resolved Hide resolved
@TaiJuWu TaiJuWu requested a review from chia7712 May 17, 2024 04:37
chia7712
chia7712 reviewed May 17, 2024
View reviewed changes
Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@TaiJuWu nice patch. it is almost prefect.

core/src/test/java/kafka/test/junit/ClusterTestExtensions.java Outdated Show resolved Hide resolved
core/src/test/java/kafka/test/ClusterTestExtensionsTest.java Outdated Show resolved Hide resolved
@chia7712
Copy link
Contributor

@TaiJuWu Please update all tests using ClusterTemplate. For example: zkClustersForAllMigrationVersions

chia7712
chia7712 approved these changes May 21, 2024
View reviewed changes
Copy link
Contributor

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

LGTM

@chia7712 chia7712 merged commit 8908352 into apache:trunk May 21, 2024
1 check failed
@TaiJuWu TaiJuWu deleted the KAFKA-16654 branch May 22, 2024 10:15
rreddy-22 pushed a commit to rreddy-22/kafka-rreddy that referenced this pull request May 24, 2024
@TaiJuWu @rreddy-22
KAFKA-16654 Refactor kafka.test.annotation.Type and ClusterTestExtens…
3d8522e 
…ions (apache#15916)

Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chia7712 chia7712 chia7712 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@TaiJuWu @chia7712