ableegoldman commented on a change in pull request #9858:
URL: https://github.com/apache/kafka/pull/9858#discussion_r695328381
##########
File path:
streams/src/test/java/org/apache/kafka/streams/integration/AdjustStreamThreadCountTest.java
##########
@@ -75,9 +76,19 @@
@Category(IntegrationTest.class)
public class AdjustStreamThreadCountTest {
- @ClassRule
public static final EmbeddedKafkaCluster CLUSTER = new
EmbeddedKafkaCluster(1);
+ @BeforeClass
+ public static void startCluster() throws IOException {
+ CLUSTER.start();
+ }
+
+ @AfterClass
+ public static void closeCluster() {
+ CLUSTER.stop();
Review comment:
Hey @chia7712 , sorry for never bothering to take a look at this PR
until just now, but I had a question about this. I know it was necessary to
remove the `ExternalResource` feature that used to be responsible for calling
`start()` and `stop()` for us in the integration tests since `@ClassRule` was
removed in JUnit5, but that was really quite a loss since this now leaves us
vulnerable to
1) resource leaks due to forgetting to clean up the EmbeddedKafkaCluster in
an integration test, or doing so in an incorrect way (eg such that a test
failure might skip the cleanup stage, a mistake that we've certainly
encountered in our tests in the past)
2) breaking compatibility of integration tests across older branches, so
that if we ever need to backport a fix that includes an integration test --
which many will/should do -- we can easily break the build of older branches by
accident. See for example [#11257](https://github.com/apache/kafka/pull/11257):
aka the reason I started digging into this 🙂 . Even if we remember to fix this
during the backport, it's just an extra hassle.
Now I'm certainly not an expert in all things JUnit, but a cursory glance
suggests we can replicate the old behavior in which the EmbeddedKafkaCluster is
automatically started/stopped without the need for this `@Before/AfterClass`
boilerplate code in every integration test. I believe it's possible to do so
using the `@Extension/ExtendWith` annotations. Can we try to port the
EmbeddedKafkaCluster back to an automated lifecycle with these so we can clean
up the Streams integration tests?
cc @ijuma @vvcephei who may be more familiar with these constructs and
how/when/why to use them
--
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
