dajac commented on code in PR #13240:
URL: https://github.com/apache/kafka/pull/13240#discussion_r1121506569


##########
clients/src/test/java/org/apache/kafka/common/requests/OffsetCommitResponseTest.java:
##########
@@ -17,37 +17,64 @@
 package org.apache.kafka.common.requests;
 
 import org.apache.kafka.common.TopicPartition;
+import org.apache.kafka.common.Uuid;
+import org.apache.kafka.common.errors.UnsupportedVersionException;
 import org.apache.kafka.common.message.OffsetCommitResponseData;
 import 
org.apache.kafka.common.message.OffsetCommitResponseData.OffsetCommitResponsePartition;
 import 
org.apache.kafka.common.message.OffsetCommitResponseData.OffsetCommitResponseTopic;
 import org.apache.kafka.common.protocol.ApiKeys;
 import org.apache.kafka.common.protocol.Errors;
 import org.apache.kafka.common.protocol.MessageUtil;
+import org.apache.kafka.common.utils.annotation.ApiKeyVersionsSource;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
 
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
+import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
+import static java.util.function.Function.identity;
 import static 
org.apache.kafka.common.requests.AbstractResponse.DEFAULT_THROTTLE_TIME;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class OffsetCommitResponseTest {
 
     protected final int throttleTimeMs = 10;
 
-    protected final String topicOne = "topic1";
+    protected final String topic1 = "topic1";
+    protected final Uuid topic1Id = Uuid.randomUuid();
     protected final int partitionOne = 1;
+    protected final int partitionTwo = 2;
     protected final Errors errorOne = Errors.COORDINATOR_NOT_AVAILABLE;
     protected final Errors errorTwo = Errors.NOT_COORDINATOR;
-    protected final String topicTwo = "topic2";
-    protected final int partitionTwo = 2;
-
-    protected TopicPartition tp1 = new TopicPartition(topicOne, partitionOne);
-    protected TopicPartition tp2 = new TopicPartition(topicTwo, partitionTwo);
+    protected final String topic2 = "topic2";
+    protected final int p3 = 3;
+    protected final int p4 = 4;
+    protected final String topic3 = "topic3";
+    protected final int p5 = 5;
+    protected final int p6 = 6;
+    protected final String topic4 = "topic4";
+    protected final Uuid topic4Id = Uuid.randomUuid();
+    protected final int p7 = 7;
+    protected final int p8 = 8;
+    protected final Uuid topic5Id = Uuid.randomUuid();
+    protected final int p9 = 9;
+    protected final int p10 = 10;
+    protected final String topic6 = "topic6";
+    protected final Uuid topic6Id = Uuid.randomUuid();
+    protected final int p11 = 11;
+    protected final int p12 = 12;

Review Comment:
   I am not a fan of all those attributes in test. One or two are fine if they 
are really re-used on all the tests. Otherwise, it may be better to check 
define what you need in tests. I would also use `TopicIdPartition` when 
relevant so you can basically group the name, id, and partition together.



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

Reply via email to