showuon commented on a change in pull request #9733:
URL: https://github.com/apache/kafka/pull/9733#discussion_r543892364



##########
File path: 
streams/src/test/java/org/apache/kafka/streams/integration/EosBetaUpgradeIntegrationTest.java
##########
@@ -938,11 +943,13 @@ public void close() {}
 
         final KafkaStreams streams = new KafkaStreams(builder.build(), config, 
new TestKafkaClientSupplier());
         streams.setUncaughtExceptionHandler((t, e) -> {
-            if (uncaughtException != null) {
+            // should only have our injected exception or commit exception
+            if (!(e instanceof RuntimeException) && 
!(e.getMessage().contains("test exception"))) {
+                // The exception won't cause the test fail since we actually 
"expected" exception thrown and failed the stream.
+                // So, log to stderr for debugging when the exception is not 
what we expected
                 e.printStackTrace(System.err);
                 fail("Should only get one uncaught exception from Streams.");

Review comment:
       > Seems this fail did not work as expected? Otherwise the test would 
have failed all the time? 
   
   Yes, the `fail()` is throwing `AssertionError` and fail the stream, but 
that's excatly what we expected to fail the stream, just not with our injected 
exception. So it won't failed the test.
   
   > we need to handle this differently for the error-injection and the "clean 
run" differently depending on the boolean test flag.
   
   I see, will update it. 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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to