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