dawidwys commented on a change in pull request #16415:
URL: https://github.com/apache/flink/pull/16415#discussion_r666084745
##########
File path:
flink-tests/src/test/java/org/apache/flink/test/streaming/api/FileReadingWatermarkITCase.java
##########
@@ -62,8 +64,17 @@ public void testWatermarkEmissionWithChaining() throws
Exception {
JobExecutionResult result = env.execute();
int actual = result.getAccumulatorResult(NUM_WATERMARKS_ACC_NAME);
-
- assertTrue("too few watermarks emitted: " + actual, actual >=
MIN_EXPECTED_WATERMARKS);
+ long expected =
+ result.<Long>getAccumulatorResult(RUNTIME_ACC_NAME) /
WATERMARK_INTERVAL_MILLIS;
+ checkState(expected > MIN_EXPECTED_WATERMARKS);
+
+ assertEquals(
+ "too few watermarks emitted in "
+ + result.<Long>getAccumulatorResult(RUNTIME_ACC_NAME)
+ + " ms",
+ expected,
+ actual,
+ .5 * expected);
Review comment:
Just to be clear, I am not criticizing the work here. Just wanted to say
it is quite a bit undeterministic.
Unfortunately I don't get the full purpose and corner cases it tries to
test. I also don't understand where do the minimum 5 expected watermarks come
from. Also don't know why we use 1 hour for bounded out of order watermarks,
what's the criteria for the number of lines etc.
I'll trust your judgement though. The changes definitely do not make it
worse and if you say it will improve the stability let's go with 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
