MINOR: Use ClusterInstance#createTopic in various integration tests

[lytt925]

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]

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]

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]

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]

@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]

This comment seems incorrect. Could you update it?

[lytt925]

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

and the comment is fixed in 8d2df08

[m1a2st]

Thanks, LGTM