Github user pnowojski commented on a diff in the pull request: https://github.com/apache/flink/pull/5500#discussion_r169055575 --- Diff: flink-tests/src/test/java/org/apache/flink/test/streaming/runtime/BroadcastStateITCase.java --- @@ -145,17 +147,16 @@ public Watermark checkAndGetNextWatermark(T lastElement, long extractedTimestamp private static final long serialVersionUID = 7616910653561100842L; private final Map<Long, String> expectedState; - - private final long timerTimestamp; + private final AtomicLong al = new AtomicLong(1000L); + // <key, time> + private final Map<Long, Long> expectedTimeAndKey = new HashMap<>(); + // <time, key> + private final Map<Long, Long> expectedKeyAndTime = new HashMap<>(); private transient MapStateDescriptor<Long, String> descriptor; - TestBroadcastProcessFunction( - final long timerTS, - final Map<Long, String> expectedBroadcastState - ) { + TestBroadcastProcessFunction(final Map<Long, String> expectedBroadcastState) { --- End diff -- Please keep the `timerTS` (renaming it to `initialTimerTimestamp`) parameter as an initial value for the `nextTimerTimestamp`/`al` field that you have added. Otherwise it's confusing where does this magic value `1000` come from and what are consequences of changing it. Especially that you have kept the original comment `// the timestamp should be high enough to trigger the timer after all the elements arrive.` in the `testConnectWithBroadcastTranslation()`.
---