mjsax commented on code in PR #16745:
URL: https://github.com/apache/kafka/pull/16745#discussion_r1701351629


##########
streams/src/test/java/org/apache/kafka/streams/integration/EosIntegrationTest.java:
##########
@@ -1170,7 +1171,9 @@ public void process(final Record<Long, Long> record) {
         final KafkaStreams streams = new KafkaStreams(builder.build(), config);
 
         streams.setUncaughtExceptionHandler((t, e) -> {
-            if (uncaughtException != null || 
!e.getMessage().contains("Injected test exception")) {
+            if (uncaughtException != null ||
+                !(e instanceof StreamsException) ||
+                !e.getCause().getMessage().equals("Injected test exception.")) 
{

Review Comment:
   This change is required only, because the original test conditions was not 
"correct" anyway. The outer exception was already a `StreamsException` wrapping 
the `RuntimeException` the test throws. Testing the output exception for type 
`RuntimeException` does not make any sense anyway, as all non-checked exception 
are `RuntimeException` and the check passes...
   
   The only actual change we get in this PR, is that the error message of the 
outer `StreamsException` changes -- originally, it was not set explicitly, and 
thus was set as `"RuntimeException: Injected test exception"` -- thus, even if 
we did check the "wrong/outer" error message the test passed. This PR now sets 
the `StreamsException` error message explicitly (what I don't consider a 
backward compatibility issue), but to this check, we need to check the error 
message of the wrapped exception (bonus, we can test for `equals` instead of 
`contains`).



-- 
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]

Reply via email to