showuon commented on a change in pull request #9202:
URL: https://github.com/apache/kafka/pull/9202#discussion_r486161428
##########
File path:
core/src/test/scala/unit/kafka/coordinator/group/GroupMetadataManagerTest.scala
##########
@@ -931,6 +932,42 @@ class GroupMetadataManagerTest {
assertTrue(group.has(memberId))
}
+ @Test
+ def testShouldThrowExceptionForUnsupportedGroupMetadataVersion(): Unit = {
+ val generation = 1
+ val protocol = "range"
+ val memberId = "memberId"
+ val unSupportedVersion = Short.MinValue
+
+ // put the un-supported version as the version value
+ val groupMetadataRecordValue =
buildStableGroupRecordWithMember(generation, protocolType, protocol, memberId)
+ .value().putShort(unSupportedVersion)
+ // reset the position to the starting position 0 so that it can read the
data in correct order
+ groupMetadataRecordValue.position(0)
+
+ val e = assertThrows(classOf[KafkaException],
+ () => GroupMetadataManager.readGroupMessageValue(groupId,
groupMetadataRecordValue, time))
+ assertEquals(s"Unknown group metadata version ${unSupportedVersion}",
e.getMessage)
+ }
+
+ @Test
+ def testCurrentStateTSForAllGroupMetadataVersion(): Unit = {
+ val generation = 1
+ val protocol = "range"
+ val memberId = "memberId"
+
+ for (apiVersion <- ApiVersion.allVersions) {
+ val groupMetadataRecord = buildStableGroupRecordWithMember(generation,
protocolType, protocol, memberId, apiVersion = apiVersion)
+
+ val deserializedGroupMetadata =
GroupMetadataManager.readGroupMessageValue(groupId,
groupMetadataRecord.value(), time)
+ // GROUP_METADATA_VALUE_SCHEMA_V2 or higher should correctly set the
currentStateTimestamp
+ if (apiVersion >= KAFKA_2_1_IV0)
+ assertEquals(time.milliseconds(),
deserializedGroupMetadata.currentStateTimestamp.get)
Review comment:
Correct! Also, after your question, I tried and found the message when
test failed will be:
```
kafka.coordinator.group.GroupMetadataManagerTest >
testCurrentStateTimestampForAllGroupMetadataVersions FAILED
java.util.NoSuchElementException: None.get
```
It's not clear and not helpful for debugging. So I added failed message and
use `getOrElse(-1)` to get an unexpected timestamp. Now, the failed message
will be:
```
kafka.coordinator.group.GroupMetadataManagerTest >
testCurrentStateTimestampForAllGroupMetadataVersions FAILED
java.lang.AssertionError: the apiVersion 2.3-IV0 doesn't set the
currentStateTimestamp correctly. expected:<1599726297785> but was:<-1>
```
It'll be more clear to show which version cause this error and what error it
is.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
