chia7712 commented on code in PR #15489:
URL: https://github.com/apache/kafka/pull/15489#discussion_r1564099443
##########
tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java:
##########
@@ -48,7 +53,7 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
@ExtendWith(value = ClusterTestExtensions.class)
-@ClusterTestDefaults(clusterType = Type.ZK)
+@ClusterTestDefaults(clusterType = Type.ALL)
@Tag("integration")
public class GetOffsetShellTest {
private final int topicCount = 4;
Review Comment:
`topicName` could be a local variable
##########
tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java:
##########
@@ -81,28 +86,57 @@ private void setUp() {
}
Properties props = new Properties();
- props.put(CommonClientConfigs.BOOTSTRAP_SERVERS_CONFIG,
cluster.config().producerProperties().get("bootstrap.servers"));
+ props.put(CommonClientConfigs.BOOTSTRAP_SERVERS_CONFIG,
cluster.bootstrapServers());
props.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG,
StringSerializer.class);
props.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG,
StringSerializer.class);
try (KafkaProducer<String, String> producer = new
KafkaProducer<>(props)) {
IntStream.range(0, topicCount + 1)
.forEach(i -> IntStream.range(0, i * i)
- .forEach(msgCount -> producer.send(
- new ProducerRecord<>(getTopicName(i), msgCount
% i, null, "val" + msgCount)))
+ .forEach(msgCount -> {
+ try {
Review Comment:
we can leverage `assertDoesNotThrow` to simplify the code
```java
assertDoesNotThrow(() -> producer.send(
new ProducerRecord<>(getTopicName(i),
msgCount % i, null, "val" + msgCount)).get());
```
##########
tools/src/test/java/org/apache/kafka/tools/GetOffsetShellTest.java:
##########
@@ -48,7 +53,7 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
@ExtendWith(value = ClusterTestExtensions.class)
-@ClusterTestDefaults(clusterType = Type.ZK)
+@ClusterTestDefaults(clusterType = Type.ALL)
@Tag("integration")
Review Comment:
Could you use `serverProperties` to define default server configs?
```java
cluster.config().serverProperties().put("auto.create.topics.enable",
false);
cluster.config().serverProperties().put("offsets.topic.replication.factor",
"1");
cluster.config().serverProperties().put("offsets.topic.num.partitions",
String.valueOf(offsetTopicPartitionCount));
```
could be replaced by
```java
@ClusterTestDefaults(clusterType = Type.ALL, serverProperties = {
@ClusterConfigProperty(key = "auto.create.topics.enable", value =
"false"),
@ClusterConfigProperty(key = "offsets.topic.replication.factor", value =
"1"),
@ClusterConfigProperty(key = "offsets.topic.num.partitions", value = "4")
})
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
