cadonna commented on code in PR #15035: URL: https://github.com/apache/kafka/pull/15035#discussion_r1432322309
########## clients/src/main/java/org/apache/kafka/clients/consumer/internals/HeartbeatRequestManager.java: ########## @@ -234,7 +235,8 @@ public long maximumTimeToWait(long currentTimeMs) { * When consumer polls, we need to reset the pollTimer. If the poll timer has expired, we rejoin only when the * member is in the {@link MemberState#UNSUBSCRIBED} state. Review Comment: Is the state here correct? ########## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ########## @@ -169,7 +169,6 @@ class PlaintextConsumerTest extends BaseConsumerTest { startingTimestamp = startingTimestamp) } - // TODO: enable this test for the consumer group protocol when KAFKA-16008 has been fixed. Review Comment: Why did you remove this comment but not enable the test? ########## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ########## @@ -196,7 +195,6 @@ class PlaintextConsumerTest extends BaseConsumerTest { assertEquals(1, listener.callsToRevoked) } - // TODO: enable this test for the consumer group protocol when KAFKA-16009 has been fixed. Review Comment: Why did you remove this comment but not enable the test? ########## clients/src/test/java/org/apache/kafka/clients/consumer/internals/HeartbeatRequestManagerTest.java: ########## @@ -660,14 +672,14 @@ private HeartbeatRequestManager createHeartbeatRequestManager() { subscriptions = mock(SubscriptionState.class); membershipManager = mock(MembershipManager.class); backgroundEventHandler = mock(BackgroundEventHandler.class); - heartbeatState = new HeartbeatRequestManager.HeartbeatState(subscriptions, membershipManager, maxPollIntervalMs); - heartbeatRequestState = new HeartbeatRequestManager.HeartbeatRequestState( + heartbeatState = spy(new HeartbeatRequestManager.HeartbeatState(subscriptions, membershipManager, maxPollIntervalMs)); Review Comment: No, I don't. Why not passing mocks to this method. -- 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