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