smattheis commented on a change in pull request #18991:
URL: https://github.com/apache/flink/pull/18991#discussion_r824852079
##########
File path:
flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/mailbox/TaskMailboxProcessorTest.java
##########
@@ -117,6 +119,8 @@ public void runDefaultAction(Controller controller) throws
Exception {
MailboxProcessor mailboxProcessor = start(mailboxThread);
mailboxProcessor.getMailboxExecutor(DEFAULT_PRIORITY).execute(() ->
stop.set(true), "stop");
mailboxThread.join();
+ Assert.assertTrue(counter.get() > 0);
Review comment:
Equals 1 won't work as it's time-dependent. I will change to hamcrest
assertThat(greater ...) - I was kind of on the track that we are restricted to
Junit which is not true. My bad, stupid me.
##########
File path:
flink-streaming-java/src/test/java/org/apache/flink/streaming/runtime/tasks/mailbox/TaskMailboxProcessorTest.java
##########
@@ -117,6 +119,8 @@ public void runDefaultAction(Controller controller) throws
Exception {
MailboxProcessor mailboxProcessor = start(mailboxThread);
mailboxProcessor.getMailboxExecutor(DEFAULT_PRIORITY).execute(() ->
stop.set(true), "stop");
mailboxThread.join();
+ Assert.assertTrue(counter.get() > 0);
+
Assert.assertTrue(mailboxProcessor.getNumMailsProcessedCounter().getCount() >
0);
Review comment:
counter.get() is not correct here as counter is the number of default
action executions, not the number of mails. At the moment, the number of mails
processed includes only the poison mail and manual the stop mail. So it would
be 2. However, if there are changes in future that some house-keeping mails are
generated and processed, one would need to adjust this test assertion. The > 0
proves that the counter is working in general. I can change it to > 1 which is
IMO here better than =2.
--
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]
