Re: [PR] MINOR: Various cleanups in clients tests [kafka]
mimaison merged PR #15877: URL: https://github.com/apache/kafka/pull/15877 -- 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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
mimaison commented on PR #15877: URL: https://github.com/apache/kafka/pull/15877#issuecomment-2109561702 Thanks for the reviews! Test failures are not related, merging to trunk -- 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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
mimaison commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1598589821
##
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##
@@ -2288,23 +2253,15 @@ public void testRebalanceException(GroupProtocol
groupProtocol) {
client.prepareResponseFrom(syncGroupResponse(singletonList(tp0),
Errors.NONE), coordinator);
// assign throws
-try {
-
consumer.updateAssignmentMetadataIfNeeded(time.timer(Long.MAX_VALUE));
-fail("Should throw exception");
-} catch (Throwable e) {
-assertEquals(partitionAssigned + singleTopicPartition,
e.getCause().getMessage());
-}
+Throwable t = assertThrows(Throwable.class, () ->
consumer.updateAssignmentMetadataIfNeeded(time.timer(Long.MAX_VALUE)));
+assertEquals(partitionAssigned + singleTopicPartition,
t.getCause().getMessage());
// the assignment is still updated regardless of the exception
assertEquals(singleton(tp0), subscription.assignedPartitions());
// close's revoke throws
-try {
-consumer.close(Duration.ofMillis(0));
-fail("Should throw exception");
-} catch (Throwable e) {
-assertEquals(partitionRevoked + singleTopicPartition,
e.getCause().getCause().getMessage());
-}
+t = assertThrows(Throwable.class, () ->
consumer.close(Duration.ofMillis(0)));
Review Comment:
Good call, I pushed an update.
Thanks for raising KAFKA-16737
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
lianetm commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1598491917
##
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##
@@ -2288,23 +2253,15 @@ public void testRebalanceException(GroupProtocol
groupProtocol) {
client.prepareResponseFrom(syncGroupResponse(singletonList(tp0),
Errors.NONE), coordinator);
// assign throws
-try {
-
consumer.updateAssignmentMetadataIfNeeded(time.timer(Long.MAX_VALUE));
-fail("Should throw exception");
-} catch (Throwable e) {
-assertEquals(partitionAssigned + singleTopicPartition,
e.getCause().getMessage());
-}
+Throwable t = assertThrows(Throwable.class, () ->
consumer.updateAssignmentMetadataIfNeeded(time.timer(Long.MAX_VALUE)));
+assertEquals(partitionAssigned + singleTopicPartition,
t.getCause().getMessage());
// the assignment is still updated regardless of the exception
assertEquals(singleton(tp0), subscription.assignedPartitions());
// close's revoke throws
-try {
-consumer.close(Duration.ofMillis(0));
-fail("Should throw exception");
-} catch (Throwable e) {
-assertEquals(partitionRevoked + singleTopicPartition,
e.getCause().getCause().getMessage());
-}
+t = assertThrows(Throwable.class, () ->
consumer.close(Duration.ofMillis(0)));
Review Comment:
Not introduced by this PR, but here we could be more specific and
`assertThrows(KafkaException.class, ...)`. Regardless of the type of exception
thrown in the rebalance callbacks, a `KafkaException` is what's always
propagated (legacy and new consumer).
Btw, reviewing this PR I noticed that this KafkaConsumerTest has lots of
tests not enabled for the new consumer (with lots of TODOs about it). I created
[KAFKA-16737](https://issues.apache.org/jira/browse/KAFKA-16737) to make sure
we address it, enable the tests that apply and clean up the code.
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
mimaison commented on code in PR #15877: URL: https://github.com/apache/kafka/pull/15877#discussion_r1598214121 ## clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java: ## @@ -3232,7 +3231,7 @@ public void testProducerBatchRetriesWhenPartitionLeaderChanges() throws Exceptio assertTrue(client.hasInFlightRequests()); client.respond(produceResponse(tp0, -1, Errors.NOT_LEADER_OR_FOLLOWER, 0)); sender.runOnce(); // receive produce response, batch scheduled for retry -assertTrue(!futureIsProduced.isDone(), "Produce request is yet not done."); +assertFalse(futureIsProduced.isDone(), "Produce request is yet not done."); Review Comment: Good catch, I updated the messages -- 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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
mimaison commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1598196898
##
clients/src/test/java/org/apache/kafka/test/Microbenchmarks.java:
##
@@ -78,34 +78,30 @@ public static void main(String[] args) throws Exception {
final Time time = Time.SYSTEM;
final AtomicBoolean done = new AtomicBoolean(false);
final Object lock = new Object();
Review Comment:
I'm not sure whether anyone uses this. I still think we should check before
deleting it.
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
mimaison commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1598194726
##
clients/src/test/java/org/apache/kafka/clients/MetadataTest.java:
##
@@ -1323,7 +1323,7 @@ public void
testConcurrentUpdateAndFetchForSnapshotAndCluster() throws Interrupt
} else { // Thread to read metadata snapshot, once its updated
try {
if (!atleastMetadataUpdatedOnceLatch.await(5,
TimeUnit.MINUTES)) {
Review Comment:
A bit unusual to chain assert calls but I'll accept it
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
mimaison commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1598184479
##
clients/src/test/java/org/apache/kafka/common/utils/ImplicitLinkedHashMultiCollectionTest.java:
##
@@ -114,11 +114,11 @@ public void testEnlargement() {
new TestElement(101),
new TestElement(105)
};
-for (int i = 0; i < testElements.length; i++) {
-assertTrue(multiSet.add(testElements[i]));
+for (TestElement testElement : testElements) {
+assertTrue(multiSet.add(testElement));
}
-for (int i = 0; i < testElements.length; i++) {
-assertFalse(multiSet.add(testElements[i]));
+for (TestElement testElement : testElements) {
+assertFalse(multiSet.add(testElement));
Review Comment:
Agreed, we can merge both loops
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
mimaison commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1598183774
##
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##
@@ -1840,28 +1840,28 @@ public void
testOperationsBySubscribingConsumerWithDefaultGroupId(GroupProtocol
// OK, expected
}
-try (KafkaConsumer consumer =
newConsumer(groupProtocol, (String) null)) {
+try (KafkaConsumer consumer =
newConsumer(groupProtocol, null)) {
consumer.subscribe(Collections.singleton(topic));
fail("Expected an InvalidGroupIdException");
} catch (InvalidGroupIdException e) {
// OK, expected
}
Review Comment:
Yes, I was able to simplify a few of these. Thanks!
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
mimaison commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1598182787
##
clients/src/test/java/org/apache/kafka/common/header/internals/RecordHeadersTest.java:
##
@@ -220,9 +220,7 @@ public void
shouldThrowNpeWhenAddingCollectionWithNullHeader() {
private int getCount(Headers headers) {
int count = 0;
-Iterator headerIterator = headers.iterator();
-while (headerIterator.hasNext()) {
-headerIterator.next();
+for (Header ignore : headers) {
Review Comment:
Nice suggestion
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
mimaison commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1598182133
##
clients/src/test/java/org/apache/kafka/clients/MetadataTest.java:
##
@@ -1335,7 +1335,7 @@ public void
testConcurrentUpdateAndFetchForSnapshotAndCluster() throws Interrupt
});
}
if (!allThreadsDoneLatch.await(5, TimeUnit.MINUTES)) {
Review Comment:
Yes that's even better!
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
mimaison commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1598165504
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -106,10 +103,9 @@ public void setup() {
commitRequestManager =
testBuilder.commitRequestManager.orElseThrow(IllegalStateException::new);
offsetsRequestManager = testBuilder.offsetsRequestManager;
coordinatorRequestManager =
testBuilder.coordinatorRequestManager.orElseThrow(IllegalStateException::new);
-heartbeatRequestManager =
testBuilder.heartbeatRequestManager.orElseThrow(IllegalStateException::new);
-memberhipsManager =
testBuilder.membershipManager.orElseThrow(IllegalStateException::new);
+HeartbeatRequestManager heartbeatRequestManager =
testBuilder.heartbeatRequestManager.orElseThrow(IllegalStateException::new);
Review Comment:
Right, these can be removed and as @lianetm pointed this allows getting rid
of the suppression. Thanks
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
lianetm commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1594231539
##
clients/src/test/java/org/apache/kafka/common/utils/ImplicitLinkedHashMultiCollectionTest.java:
##
@@ -114,11 +114,11 @@ public void testEnlargement() {
new TestElement(101),
new TestElement(105)
};
-for (int i = 0; i < testElements.length; i++) {
-assertTrue(multiSet.add(testElements[i]));
+for (TestElement testElement : testElements) {
+assertTrue(multiSet.add(testElement));
}
-for (int i = 0; i < testElements.length; i++) {
-assertFalse(multiSet.add(testElements[i]));
+for (TestElement testElement : testElements) {
+assertFalse(multiSet.add(testElement));
Review Comment:
could this be simplified to a single loop with the 2 asserts in it? we would
achieve the same, validate an elem can be added only if new, for all elems.
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
lianetm commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1594213570
##
clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java:
##
@@ -3492,9 +3491,9 @@ private void verifyErrorMessage(ProduceResponse response,
String expectedMessage
assertEquals(expectedMessage, e1.getCause().getMessage());
}
-class AssertEndTxnRequestMatcher implements MockClient.RequestMatcher {
+static class AssertEndTxnRequestMatcher implements
MockClient.RequestMatcher {
Review Comment:
seems could be private too
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
lianetm commented on code in PR #15877: URL: https://github.com/apache/kafka/pull/15877#discussion_r1594199948 ## clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java: ## @@ -3232,7 +3231,7 @@ public void testProducerBatchRetriesWhenPartitionLeaderChanges() throws Exceptio assertTrue(client.hasInFlightRequests()); client.respond(produceResponse(tp0, -1, Errors.NOT_LEADER_OR_FOLLOWER, 0)); sender.runOnce(); // receive produce response, batch scheduled for retry -assertTrue(!futureIsProduced.isDone(), "Produce request is yet not done."); +assertFalse(futureIsProduced.isDone(), "Produce request is yet not done."); Review Comment: Looks to me this error message is inverted too right? it's stating the desired state, not the failure: if the assert fails, it's because the produce request **is done** when it shouldn't (not because "is yet not done"). Maybe just "Produce request shouldn't complete yet"... -- 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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
lianetm commented on code in PR #15877: URL: https://github.com/apache/kafka/pull/15877#discussion_r1594200503 ## clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java: ## @@ -3253,13 +3252,13 @@ public void testProducerBatchRetriesWhenPartitionLeaderChanges() throws Exceptio assertTrue(client.hasInFlightRequests()); client.respond(produceResponse(tp0, -1, Errors.NOT_LEADER_OR_FOLLOWER, 0)); sender.runOnce(); // receive produce response, schedule batch for retry. -assertTrue(!futureIsProduced.isDone(), "Produce request is yet not done."); +assertFalse(futureIsProduced.isDone(), "Produce request is yet not done."); 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
lianetm commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1594156634
##
clients/src/test/java/org/apache/kafka/clients/consumer/KafkaConsumerTest.java:
##
@@ -1840,28 +1840,28 @@ public void
testOperationsBySubscribingConsumerWithDefaultGroupId(GroupProtocol
// OK, expected
}
-try (KafkaConsumer consumer =
newConsumer(groupProtocol, (String) null)) {
+try (KafkaConsumer consumer =
newConsumer(groupProtocol, null)) {
consumer.subscribe(Collections.singleton(topic));
fail("Expected an InvalidGroupIdException");
} catch (InvalidGroupIdException e) {
// OK, expected
}
Review Comment:
Since we're cleaning up this, maybe just `assertThrows`? (same for the
following 3 `try`)
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
lianetm commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1593116655
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -106,10 +103,9 @@ public void setup() {
commitRequestManager =
testBuilder.commitRequestManager.orElseThrow(IllegalStateException::new);
offsetsRequestManager = testBuilder.offsetsRequestManager;
coordinatorRequestManager =
testBuilder.coordinatorRequestManager.orElseThrow(IllegalStateException::new);
-heartbeatRequestManager =
testBuilder.heartbeatRequestManager.orElseThrow(IllegalStateException::new);
-memberhipsManager =
testBuilder.membershipManager.orElseThrow(IllegalStateException::new);
+HeartbeatRequestManager heartbeatRequestManager =
testBuilder.heartbeatRequestManager.orElseThrow(IllegalStateException::new);
Review Comment:
btw, removing all those unused might help us remove the suppression
ClassDataAbstractionCoupling, worth checking
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
lianetm commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1593098515
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -106,10 +103,9 @@ public void setup() {
commitRequestManager =
testBuilder.commitRequestManager.orElseThrow(IllegalStateException::new);
offsetsRequestManager = testBuilder.offsetsRequestManager;
coordinatorRequestManager =
testBuilder.coordinatorRequestManager.orElseThrow(IllegalStateException::new);
-heartbeatRequestManager =
testBuilder.heartbeatRequestManager.orElseThrow(IllegalStateException::new);
-memberhipsManager =
testBuilder.membershipManager.orElseThrow(IllegalStateException::new);
+HeartbeatRequestManager heartbeatRequestManager =
testBuilder.heartbeatRequestManager.orElseThrow(IllegalStateException::new);
Review Comment:
I don't think they are used to test the existence of the managers here, I
would say they were just left unused so we should remove them. Managers are
retrieved in this way in many other tests (ex
[HeartbeatRequestManagerTest](https://github.com/apache/kafka/blob/525b9b1d7682ae2a527ceca83fedca44b1cba11a/clients/src/test/java/org/apache/kafka/clients/consumer/internals/HeartbeatRequestManagerTest.java#L119)),
but only when needed.
--
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
Re: [PR] MINOR: Various cleanups in clients tests [kafka]
chia7712 commented on code in PR #15877:
URL: https://github.com/apache/kafka/pull/15877#discussion_r1591714910
##
clients/src/test/java/org/apache/kafka/clients/MetadataTest.java:
##
@@ -1323,7 +1323,7 @@ public void
testConcurrentUpdateAndFetchForSnapshotAndCluster() throws Interrupt
} else { // Thread to read metadata snapshot, once its updated
try {
if (!atleastMetadataUpdatedOnceLatch.await(5,
TimeUnit.MINUTES)) {
Review Comment:
How about using `assertDoesNotThrow`? For example:
```java
assertTrue(assertDoesNotThrow(() ->
atleastMetadataUpdatedOnceLatch.await(5, TimeUnit.MINUTES)),
"Test had to wait more than 5 minutes, something
went wrong.");
```
##
clients/src/test/java/org/apache/kafka/clients/MetadataTest.java:
##
@@ -1335,7 +1335,7 @@ public void
testConcurrentUpdateAndFetchForSnapshotAndCluster() throws Interrupt
});
}
if (!allThreadsDoneLatch.await(5, TimeUnit.MINUTES)) {
Review Comment:
How about using `assertTrue`?
```java
assertTrue(allThreadsDoneLatch.await(5, TimeUnit.MINUTES), "Test had to wait
more than 5 minutes, something went wrong.");
```
##
clients/src/test/java/org/apache/kafka/common/header/internals/RecordHeadersTest.java:
##
@@ -220,9 +220,7 @@ public void
shouldThrowNpeWhenAddingCollectionWithNullHeader() {
private int getCount(Headers headers) {
int count = 0;
-Iterator headerIterator = headers.iterator();
-while (headerIterator.hasNext()) {
-headerIterator.next();
+for (Header ignore : headers) {
Review Comment:
How about using `toArray`? for example: `headers.toArray().length`
##
clients/src/test/java/org/apache/kafka/test/Microbenchmarks.java:
##
@@ -78,34 +78,30 @@ public static void main(String[] args) throws Exception {
final Time time = Time.SYSTEM;
final AtomicBoolean done = new AtomicBoolean(false);
final Object lock = new Object();
Review Comment:
Maybe we should just delete this old class ...
##
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkThreadTest.java:
##
@@ -106,10 +103,9 @@ public void setup() {
commitRequestManager =
testBuilder.commitRequestManager.orElseThrow(IllegalStateException::new);
offsetsRequestManager = testBuilder.offsetsRequestManager;
coordinatorRequestManager =
testBuilder.coordinatorRequestManager.orElseThrow(IllegalStateException::new);
-heartbeatRequestManager =
testBuilder.heartbeatRequestManager.orElseThrow(IllegalStateException::new);
-memberhipsManager =
testBuilder.membershipManager.orElseThrow(IllegalStateException::new);
+HeartbeatRequestManager heartbeatRequestManager =
testBuilder.heartbeatRequestManager.orElseThrow(IllegalStateException::new);
Review Comment:
The variables `heartbeatRequestManager` and `membershipManager` are unused.
Are they used to test the existence of `heartbeatRequestManager` and
`membershipManager`? If so, could we rewrite them by `assertTrue`? For example:
`assertTrue(testBuilder.heartbeatRequestManager.isPresent());`
--
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
