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


##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2642,17 +2645,25 @@ class PlaintextAdminIntegrationTest extends 
BaseAdminIntegrationTest {
         val expectedOperations = 
AclEntry.supportedOperations(ResourceType.GROUP)
         assertEquals(expectedOperations, 
testGroupDescription.authorizedOperations())
 
-        // Test that the fake group is listed as dead.
+        // Test that the fake group throws GroupIdNotFoundException
         
assertTrue(describeWithFakeGroupResult.describedGroups().containsKey(fakeGroupId))
-        val fakeGroupDescription = 
describeWithFakeGroupResult.describedGroups().get(fakeGroupId).get()
-
-        assertEquals(fakeGroupId, fakeGroupDescription.groupId())
-        assertEquals(0, fakeGroupDescription.members().size())
-        assertEquals(GroupState.DEAD, fakeGroupDescription.groupState())
-        assertNull(fakeGroupDescription.authorizedOperations())
+        try {
+          describeWithFakeGroupResult.describedGroups().get(fakeGroupId).get()
+          fail("Fake group should throw GroupIdNotFoundException")
+        } catch {
+          case e: ExecutionException =>
+            assertTrue(e.getCause.isInstanceOf[GroupIdNotFoundException])
+        }
 
-        // Test that all() returns 2 results
-        assertEquals(2, describeWithFakeGroupResult.all().get().size())
+        // Test that all() also throws GroupIdNotFoundException
+        try {

Review Comment:
   ditto.



##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2664,18 +2675,18 @@ class PlaintextAdminIntegrationTest extends 
BaseAdminIntegrationTest {
         assertEquals(testGroupId, testGroupDescription.groupId)
         assertEquals(consumerSet.size, testGroupDescription.members().size())
 
-        // Describing a share group using describeConsumerGroups reports it as 
a DEAD consumer group
-        // in the same way as a non-existent group
+        // Describing a share group using describeConsumerGroups reports it as 
a non-existent group
+        // but the error message is different
         val describeConsumerGroupResult = 
client.describeConsumerGroups(Collections.singleton(testGroupId),
           new 
DescribeConsumerGroupsOptions().includeAuthorizedOperations(true))
-        assertEquals(1, describeConsumerGroupResult.all().get().size())
-
-        val deadConsumerGroupDescription = 
describeConsumerGroupResult.describedGroups().get(testGroupId).get()
-        assertEquals(testGroupId, deadConsumerGroupDescription.groupId())
-        assertEquals(0, deadConsumerGroupDescription.members().size())
-        assertEquals("", deadConsumerGroupDescription.partitionAssignor())
-        assertEquals(ConsumerGroupState.DEAD, 
deadConsumerGroupDescription.state())
-        assertEquals(expectedOperations, 
deadConsumerGroupDescription.authorizedOperations())
+        try {

Review Comment:
   ditto.



##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2209,18 +2205,25 @@ class PlaintextAdminIntegrationTest extends 
BaseAdminIntegrationTest {
           val expectedOperations = 
AclEntry.supportedOperations(ResourceType.GROUP)
           assertEquals(expectedOperations, 
testGroupDescription.authorizedOperations())
 
-          // Test that the fake group is listed as dead.
+          // Test that the fake group throws GroupIdNotFoundException
           
assertTrue(describeWithFakeGroupResult.describedGroups().containsKey(fakeGroupId))
-          val fakeGroupDescription = 
describeWithFakeGroupResult.describedGroups().get(fakeGroupId).get()
-
-          assertEquals(fakeGroupId, fakeGroupDescription.groupId())
-          assertEquals(0, fakeGroupDescription.members().size())
-          assertEquals("", fakeGroupDescription.partitionAssignor())
-          assertEquals(ConsumerGroupState.DEAD, fakeGroupDescription.state())
-          assertEquals(expectedOperations, 
fakeGroupDescription.authorizedOperations())
+          try {

Review Comment:
   Could we use `assertFutureThrows` here too?



##########
core/src/test/scala/integration/kafka/api/PlaintextAdminIntegrationTest.scala:
##########
@@ -2642,17 +2645,25 @@ class PlaintextAdminIntegrationTest extends 
BaseAdminIntegrationTest {
         val expectedOperations = 
AclEntry.supportedOperations(ResourceType.GROUP)
         assertEquals(expectedOperations, 
testGroupDescription.authorizedOperations())
 
-        // Test that the fake group is listed as dead.
+        // Test that the fake group throws GroupIdNotFoundException
         
assertTrue(describeWithFakeGroupResult.describedGroups().containsKey(fakeGroupId))
-        val fakeGroupDescription = 
describeWithFakeGroupResult.describedGroups().get(fakeGroupId).get()
-
-        assertEquals(fakeGroupId, fakeGroupDescription.groupId())
-        assertEquals(0, fakeGroupDescription.members().size())
-        assertEquals(GroupState.DEAD, fakeGroupDescription.groupState())
-        assertNull(fakeGroupDescription.authorizedOperations())
+        try {

Review Comment:
   ditto.



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

Reply via email to