-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
test: Fix and improve the test QuickDegradeAndRestoreCommandTopicIntegrationTest #8275
Changes from 2 commits
3454a96
4afc2b9
c2f61a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -576,8 +576,9 @@ private Properties buildBrokerConfig(final String logDir) { | |
// Need to know where ZK is: | ||
config.put(KafkaConfig.ZkConnectProp(), zookeeper.connectString()); | ||
config.put(AclAuthorizer.ZkUrlProp(), zookeeper.connectString()); | ||
// Do not require tests to explicitly create tests: | ||
config.put(KafkaConfig.AutoCreateTopicsEnableProp(), true); | ||
// Create topics explicitly when needed to avoid a race which | ||
// automatically recreates deleted command topic: | ||
config.put(KafkaConfig.AutoCreateTopicsEnableProp(), false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm worried that there may be many tests that rely on automatically creating topics. Is there way to set this config but at the test level instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you look at private static final IntegrationTestHarness TEST_HARNESS = IntegrationTestHarness.builder()
.withKafkaCluster(
EmbeddedSingleNodeKafkaCluster.newBuilder()
.withoutPlainListeners()
.withSaslSslListeners()
).build(); Can we do something similar here and create a function to remove the auto topic creation for just this test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, seem like few tests do depend on that setting to be true. I've addressed this in my latest commit. For now, only QuickDegradeAndRestoreCommandTopicIntegrationTest will set automatically creating topics to be false. |
||
// Default to small number of partitions for auto-created topics: | ||
config.put(KafkaConfig.NumPartitionsProp(), 1); | ||
// Allow tests to delete topics: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good check