dajac commented on code in PR #14589:
URL: https://github.com/apache/kafka/pull/14589#discussion_r1373349480
##########
core/src/test/scala/unit/kafka/server/KafkaApisTest.scala:
##########
@@ -4385,15 +4451,39 @@ class KafkaApisTest {
.setGroupId("group-3")
.setErrorCode(Errors.INVALID_GROUP_ID.code)
- val expectedOffsetFetchResponse = new OffsetFetchResponseData()
- .setGroups(List(group1Response, group2Response, group3Response).asJava)
+ val group4Response = new
OffsetFetchResponseData.OffsetFetchResponseGroup()
+ .setGroupId("group-4")
+ .setErrorCode(Errors.INVALID_GROUP_ID.code)
+
+ val group5Response = new
OffsetFetchResponseData.OffsetFetchResponseGroup()
Review Comment:
I don't fully understand this test case either. My understanding is that we
could have a weird response only in one case: the coordinator returns a
response with an error and the kafka apis adds unauthorized topics to it. Did I
get it right? If so, it may be better to actually add a new (and separate) test
case for this one.
##########
core/src/test/scala/unit/kafka/server/KafkaApisTest.scala:
##########
@@ -4385,15 +4451,39 @@ class KafkaApisTest {
.setGroupId("group-3")
.setErrorCode(Errors.INVALID_GROUP_ID.code)
- val expectedOffsetFetchResponse = new OffsetFetchResponseData()
- .setGroups(List(group1Response, group2Response, group3Response).asJava)
+ val group4Response = new
OffsetFetchResponseData.OffsetFetchResponseGroup()
+ .setGroupId("group-4")
+ .setErrorCode(Errors.INVALID_GROUP_ID.code)
+
+ val group5Response = new
OffsetFetchResponseData.OffsetFetchResponseGroup()
+ .setGroupId("group-5")
+ .setTopics(List(
+ new OffsetFetchResponseData.OffsetFetchResponseTopics()
+ .setName("unknown")
+ .setPartitions(List(
+ new OffsetFetchResponseData.OffsetFetchResponsePartitions()
+ .setPartitionIndex(0)
+ .setCommittedOffset(-1)
+ ).asJava),
+ new OffsetFetchResponseData.OffsetFetchResponseTopics()
+ .setName("foo")
+ .setPartitions(List(
+ new OffsetFetchResponseData.OffsetFetchResponsePartitions()
+ .setPartitionIndex(1)
+ .setCommittedOffset(-1)
+ ).asJava)
+ ).asJava)
+
+ val expectedGroups = List(group1Response, group2Response,
group3Response, group4Response, group5Response)
Review Comment:
Is this change really necessary?
##########
core/src/test/scala/unit/kafka/server/KafkaApisTest.scala:
##########
@@ -4385,15 +4451,39 @@ class KafkaApisTest {
.setGroupId("group-3")
.setErrorCode(Errors.INVALID_GROUP_ID.code)
- val expectedOffsetFetchResponse = new OffsetFetchResponseData()
- .setGroups(List(group1Response, group2Response, group3Response).asJava)
+ val group4Response = new
OffsetFetchResponseData.OffsetFetchResponseGroup()
Review Comment:
Actually, how is this different from `group3Response`?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
