lianetm commented on code in PR #17244:
URL: https://github.com/apache/kafka/pull/17244#discussion_r1777270499
##########
core/src/test/scala/integration/kafka/api/PlaintextConsumerSubscriptionTest.scala:
##########
@@ -244,6 +245,9 @@ class PlaintextConsumerSubscriptionTest extends
AbstractConsumerTest {
}, waitTimeMs = 5000, msg = "An InvalidTopicException should be thrown.")
assertEquals(s"Invalid topics: [${invalidTopicName}]",
exception.getMessage)
+ assertDoesNotThrow(new Executable {
Review Comment:
I like the idea of including the unsubscribe in the test, but with this we
loose the coverage we initially had for subscribe(invalid) + close (if there's
a regression and releaseAssignment stops ignoring the error we won't catch it).
What about having 2 tests, mostly doing the same intially, but then one calls
unsubscribe and asserts does not throw (like we have now), and the other one
calls close explicitely and asserts the same, what do you think? (we could
reuse most of the code for the 2 tests)
--
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
