mjsax commented on code in PR #18405:
URL: https://github.com/apache/kafka/pull/18405#discussion_r1906275452
##########
streams/src/test/java/org/apache/kafka/streams/KafkaStreamsTest.java:
##########
@@ -982,10 +983,14 @@ public void
shouldThrowOnCleanupWhileShuttingDownStreamClosedWithCloseOptionLeav
closeOptions.timeout(Duration.ZERO);
closeOptions.leaveGroup(true);
- streams.close(closeOptions);
- assertThat(streams.state() == State.PENDING_SHUTDOWN,
equalTo(true));
- assertThrows(IllegalStateException.class, streams::cleanUp);
- assertThat(streams.state() == State.PENDING_SHUTDOWN,
equalTo(true));
+ if (streams.close(closeOptions)) {
Review Comment:
Not sure if I agree -- `CleanupThread` seem to be fine, but it seem to
rather be a test setup issue, ie, we need a clean way to "pause" the
`CleanupThread` for test -- for regular production code, we don't need to pause
it though, so it's only a test setup thing.
As mentioned, I think if we change `prepareTerminableThread` helper method
of this test, we can make it work. If it's unclear to you what I have in mind,
happy to do a PR myself. Just let me know how you want to proceeed.
--
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]
