MINOR: Use ClusterInstance#createTopic in various integration tests by lytt925 · Pull Request #23513 · apache/kafka

lytt925 · 2026-09-18T16:40:41Z

Follow-up to apache/kafka#22874
(comment)

Currently many tests running on ClusterInstance create topics manually
using Admin#createTopics. The post-creation waiting behavior is not
inconsistent. Some tests implement custom waits, some call
.all().get(), and others don't wait.

Changes

This PR switch these calls to ClusterInstance#createTopic or
ClusterInstance#createTopicWithAssignment.
when the topic is for test setup and maps to one of the
helpers.

Also add a new overload of createTopicWithAssignment to convert more
tests.

I kept the original createTopics calls where the test needs the returned
topic ID

Reviewers: Ming-Yen Chung mingyen066@gmail.com

Reviewers: Ken Huang s7133700@gmail.com

mingyen066 · 2026-09-19T13:09:27Z

Thanks for the cleanup! The follow-up in apache/kafka#22874 (comment) was meant to clean up the whole codebase, and there are still many places using the same pattern, such as TopicCommandTest and EligibleLeaderReplicasIntegrationTest. Could we clean them all up in this PR?

lytt925 · 2026-09-20T02:37:50Z

Thanks for the cleanup! The follow-up in apache/kafka#22874 (comment) was meant to clean up the whole codebase, and there are still many places using the same pattern, such as TopicCommandTest and EligibleLeaderReplicasIntegrationTest. Could we clean them all up in this PR?

Thank you for the review! Sorry for missing other files in the first place. I have added them in the new commits. Please let me know if I missed any.

github-actions · 2026-09-29T03:22:29Z

A label of 'needs-attention' was automatically added to this PR in order to raise the
attention of the committers. Once this issue has been triaged, the triage label
should be removed to prevent this automation from happening again.

m1a2st

Could you also take a look at the following tests? They follow the same pattern:

DeleteTopicTest
ConnectInternalTopicsTest
BootstrapControllersIntegrationTest

There are also several tests that create a topic using only createTopics(...).all().get() (or .topicId(...).get()) without an explicit wait:

ConfigCommandIntegrationTest
DescribeConsumerGroupTest
DeleteOffsetsConsumerGroupCommandIntegrationTest
LeaderElectionCommandTest
ResetConsumerGroupOffsetTest
DeleteRecordsCommandTest
ConsumerIntegrationTest
ShareConsumerDLQTest
CordonedLogDirsIntegrationTest

lytt925 · 2026-10-03T06:48:38Z

@m1a2st Thanks for the review!

I've converted the tests you listed, plus a few more with the same pattern, and updated the PR description with the criteria I used.

Also few still create topics manually because they need a replica assignment together with configs. Would it make sense to add a createTopicWithAssignment(name, assignment, configs) overload?

m1a2st

Thanks @lytt925 for the update.

Also few still create topics manually because they need a replica assignment together with configs. Would it make sense to add a createTopicWithAssignment(name, assignment, configs) overload?

Yes, I think adding an overload makes sense. It would allow us to convert the remaining call sites that require both a replica assignment and topic configs.

m1a2st · 2026-10-03T08:15:31Z

@@ -219,15 +219,15 @@ public void testStaticCordonUncordonLogDirs() throws Exception {
            setCordonedLogDirs(admin, List.of(), BROKER_0);

            // We can't create topics again


This comment seems incorrect. Could you update it?

Thank you. the overload and related tests are updated in a932362

and the comment is fixed in 8d2df08

m1a2st

Thanks, LGTM

chia7712

@lytt925 thanks for this cleanup. I left a couple of comments. Please open a follow-up to address them

chia7712 · 2026-10-04T06:54:48Z

+    default void createTopicWithAssignment(String topicName, Map<Integer, List<Integer>> replicaAssignment, Map<String, String> props) throws InterruptedException {
        try (Admin admin = admin()) {
-            admin.createTopics(List.of(new NewTopic(topicName, replicaAssignment)));
+            admin.createTopics(List.of(new NewTopic(topicName, replicaAssignment).configs(props)));


We need to call get to have the correct error propagated.

chia7712 · 2026-10-04T06:55:01Z

    void testEnableRemoteLogOnExistingTopic() throws Exception {
        try (var admin = cluster.admin()) {
-            admin.createTopics(List.of(new NewTopic(testTopicName, numPartitions, numReplicationFactor).configs(Map.of()))).all().get();
+            cluster.createTopic(testTopicName, numPartitions, numReplicationFactor, Map.of());


cluster.createTopic(testTopicName, numPartitions, numReplicationFactor)

chia7712 · 2026-10-04T07:00:48Z

-        }
+        String topic1 = "kafka.testTopic1";
+        String hiddenConsumerTopic = Topic.GROUP_METADATA_TOPIC_NAME;
+        int partition = 2;


Those local variables were useful before, but now they look a bit verbose, right?

chia7712 · 2026-10-04T07:00:52Z

    @ClusterTest(brokers = 3)
    public void testAlterPartitionCount(ClusterInstance clusterInstance) throws Exception {
        String testTopicName = TestUtils.randomString(10);
+        int partition = 2;


chia7712 · 2026-10-04T07:00:56Z

    @ClusterTemplate("generate")
    public void testAlterAssignment(ClusterInstance clusterInstance) throws Exception {
        String testTopicName = TestUtils.randomString(10);
+        int partition = 2;


chia7712 · 2026-10-04T07:01:04Z

    @ClusterTest(brokers = 3)
    public void testAlterAssignmentWithMoreAssignmentThanPartitions(ClusterInstance clusterInstance) throws Exception {
        String testTopicName = TestUtils.randomString(10);
+        int partition = 2;


chia7712 · 2026-10-04T07:01:08Z

    @ClusterTemplate("generate")
    public void testAlterAssignmentWithMorePartitionsThanAssignment(ClusterInstance clusterInstance) throws Exception {
        String testTopicName = TestUtils.randomString(10);
+        int partition = 2;


chia7712 · 2026-10-04T07:05:41Z

@@ -645,7 +640,7 @@ private void updateAndCheckInvalidBrokerConfig(Optional<String> brokerIdOrDefaul
    public void testUpdateInvalidTopicConfigs() throws ExecutionException, InterruptedException {


We can scrape ExecutionException out

chia7712 · 2026-10-04T07:06:18Z

@@ -218,9 +217,6 @@ private String execute(LogDirsCommand.LogDirsCommandOptions options, Admin admin
    }

    private void createTopic(ClusterInstance clusterInstance, String topic) {


We could use clusterInstance.createTopicWithAssignment instead of this helper, right?

chia7712 · 2026-10-04T07:06:32Z

    @ClusterTest(brokers = 3)
    public void testCreateWithTopicNameCollision(ClusterInstance clusterInstance) throws Exception {
+        String topic = "foo_bar";
+        int partitions = 1;


github-actions Bot added triage PRs from the community core Kafka Broker tools tests Test fixes (including flaky tests) storage Pull requests that target the storage module clients small Small PRs labels Sep 18, 2026

chia7712 added the ci-approved label Sep 18, 2026

github-actions Bot added tiered-storage Related to the Tiered Storage feature and removed small Small PRs labels Sep 19, 2026

github-actions Bot added the needs-attention label Sep 29, 2026

lytt925 force-pushed the use-clusterinstance-createtopic branch from 5d03882 to 1c473c1 Compare September 29, 2026 04:32

m1a2st reviewed Sep 29, 2026

View reviewed changes

github-actions Bot removed needs-attention triage PRs from the community labels Sep 30, 2026

lytt925 added 12 commits October 3, 2026 13:46

Use createTopic in ClusterTestExtensionsTest

057f95c

Use createTopic in clients admin tests

af8ac39

Use createTopic in clients consumer tests

08b08a2

Use createTopic in clients producer tests

3301ff8

Use createTopic in rebootstrap and authorizer tests

21428a3

Use createTopic in server tests

2760211

Use createTopic in RemoteTopicCrudTest

28920f0

Use createTopic in LogAppendTimeTest and RLMM tests

892bd01

Use createTopic in TopicCommandTest

9697333

Use createTopic in ConsoleConsumerTest

fdaee7b

Use createTopic in consumer group command tests

15fe998

Use createTopic in other tools tests

cf7a92d

lytt925 force-pushed the use-clusterinstance-createtopic branch from 1c473c1 to cf7a92d Compare October 3, 2026 05:51

m1a2st reviewed Oct 3, 2026

View reviewed changes

lytt925 added 2 commits October 3, 2026 17:19

Add createTopicWithAssignment overload with configs

a932362

Fix incorrect comment in CordonedLogDirsIntegrationTest

8d2df08

m1a2st approved these changes Oct 3, 2026

View reviewed changes

m1a2st merged commit 62f0fef into apache:trunk Oct 4, 2026
20 checks passed

chia7712 reviewed Oct 4, 2026

View reviewed changes

lytt925 mentioned this pull request Oct 4, 2026

MINOR: Propagate ClusterInstance#createTopic errors and clean up callers #23699

Open

		@@ -219,15 +219,15 @@ public void testStaticCordonUncordonLogDirs() throws Exception {
		setCordonedLogDirs(admin, List.of(), BROKER_0);

		// We can't create topics again

		@@ -645,7 +640,7 @@ private void updateAndCheckInvalidBrokerConfig(Optional<String> brokerIdOrDefaul
		public void testUpdateInvalidTopicConfigs() throws ExecutionException, InterruptedException {

		@@ -218,9 +217,6 @@ private String execute(LogDirsCommand.LogDirsCommandOptions options, Admin admin
		}

		private void createTopic(ClusterInstance clusterInstance, String topic) {

Conversation

lytt925 commented Sep 18, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

mingyen066 commented Sep 19, 2026

Uh oh!

lytt925 commented Sep 20, 2026

Uh oh!

github-actions Bot commented Sep 29, 2026

Uh oh!

m1a2st left a comment

Choose a reason for hiding this comment

Uh oh!

lytt925 commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m1a2st left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m1a2st left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lytt925 commented Sep 18, 2026 •

edited by github-actions Bot

Loading

lytt925 commented Oct 3, 2026 •

edited

Loading