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


##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/RecordHelpersTest.java:
##########
@@ -383,4 +401,203 @@ public void testNewCurrentAssignmentTombstoneRecord() {
             "member-id"
         ));
     }
+
+    private static Stream<Arguments> metadataToExpectedGroupMetadataValue() {
+        return Stream.of(
+            Arguments.arguments(MetadataVersion.IBP_0_10_0_IV0, (short) 0),
+            Arguments.arguments(MetadataVersion.IBP_1_1_IV0, (short) 1),
+            Arguments.arguments(MetadataVersion.IBP_2_2_IV0, (short) 2),
+            Arguments.arguments(MetadataVersion.IBP_3_5_IV0, (short) 3)
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("metadataToExpectedGroupMetadataValue")
+    public void testNewGroupMetadataRecord(
+        MetadataVersion metadataVersion,
+        short expectedGroupMetadataValueVersion
+    ) {
+        Time time = new MockTime();
+
+        List<GroupMetadataValue.MemberMetadata> expectedMembers = new 
ArrayList<>();
+        expectedMembers.add(
+            new GroupMetadataValue.MemberMetadata()
+                .setMemberId("member-1")
+                .setClientId("client-1")
+                .setClientHost("host-1")
+                .setRebalanceTimeout(1000)
+                .setSessionTimeout(1500)
+                .setGroupInstanceId("group-instance-1")
+                .setSubscription(new byte[]{0, 1})
+                .setAssignment(new byte[]{1, 2})
+        );
+
+        expectedMembers.add(
+            new GroupMetadataValue.MemberMetadata()
+                .setMemberId("member-2")
+                .setClientId("client-2")
+                .setClientHost("host-2")
+                .setRebalanceTimeout(1000)
+                .setSessionTimeout(1500)
+                .setGroupInstanceId("group-instance-2")
+                .setSubscription(new byte[]{1, 2})
+                .setAssignment(new byte[]{2, 3})
+        );
+
+        Record expectedRecord = new Record(
+            new ApiMessageAndVersion(
+                new GroupMetadataKey()
+                    .setGroup("group-id"),
+                (short) 2),
+            new ApiMessageAndVersion(
+                new GroupMetadataValue()
+                    .setProtocol("range")
+                    .setProtocolType("consumer")
+                    .setLeader("member-1")
+                    .setGeneration(1)
+                    .setCurrentStateTimestamp(time.milliseconds())
+                    .setMembers(expectedMembers),
+            expectedGroupMetadataValueVersion));

Review Comment:
   nit: Indentation seems to be off here.



##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/RecordHelpers.java:
##########
@@ -357,6 +361,81 @@ public static Record newCurrentAssignmentTombstoneRecord(
         );
     }
 
+    /**
+     * Creates a GroupMetadata record.
+     *
+     * @param group              The generic group.
+     * @param metadataVersion    The metadata version.
+     * @return The record.
+     */
+    public static Record newGroupMetadataRecord(
+        GenericGroup group,
+        MetadataVersion metadataVersion
+    ) {
+        List<GroupMetadataValue.MemberMetadata> members = new ArrayList<>();

Review Comment:
   nit: Should we set the size based on the number of members in the group?



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