florin-akermann commented on code in PR #15189:
URL: https://github.com/apache/kafka/pull/15189#discussion_r1493061626
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamKStreamJoinTest.java:
##########
@@ -1363,6 +1365,16 @@ public void testWindowing() {
new KeyValueTimestamp<>(2, "L2+l2", 2002L),
new KeyValueTimestamp<>(3, "L3+l3", 2003L)
);
+
+ //push two items with timestamp at grace edge; this should produce
one join item, M0 is 'too late'
+ final long currentStreamTime = 2104;
+ final long lowerBound = currentStreamTime -
timeDifference.toMillis() - grace.toMillis();
+ inputTopic1.pipeInput(0, "M0", lowerBound - 1);
+ inputTopic1.pipeInput(1, "M1", lowerBound + 1);
Review Comment:
I can see now that the naming is misleading.
The lowerbound is with regards to the grace period, 1900
However the lowerbound of the winow `1:l1` is 1901
So the +1 was there to make sure it is still within the window.
In general I start to wonder whether it wouldn't make more sense to test
these two concerns (grace & windowing) separatley. E.g. with grace 150, `M0` is
just a test case to assert that late records get dropped and `M1` is just
another window bound test. With grace 104 we get the 'grace bound' and the 'l0
lower window bound' to overlap but it might be confusing. In other words, as
you said 'this test aims to test window bounds'. So maybe I should move grace
period tests into a separate test class?
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
