New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BEAM-3317] Use fixed system time for testing #4723
[BEAM-3317] Use fixed system time for testing #4723
Conversation
@kennknowles @aromanenko-dev could you please take a look? |
@pawel-kaczmarczyk |
@aromanenko-dev It's enough to slow down the |
@pawel-kaczmarczyk Great, thanks! |
assertThat(reader.getWatermark()).isEqualTo(new Instant(timestampMs)); | ||
try { | ||
Instant now = Instant.now(); | ||
DateTimeUtils.setCurrentMillisFixed(now.getMillis()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is OK with me. I think even better would be to inject a clock into the reader (just pass to constructor) if it depends on such a service. I don't really know much about this connector, though, so it is just a vague suggestion.
Can you please squash the commits together? Then I will merge. |
3dfd930
to
d0f272f
Compare
@kennknowles commits squashed, thanks for your review! |
The problem here was caused by unfortunate timing during watermark retrieval. KinesisReader uses MovingFunction to decide if watermark should be advanced or not, parametrized with
numSignificantBuckets=2
andnumSignificantSamples=10
. The test relied only onnumSignificantSamples
and did not take into consideration the time buckets. Now it sets the system time to a fixed value before execution so there will be no unfortunate time interatctions. I've also applied the same changes to other test that was likely to fail as it followed the same scheme.Follow this checklist to help us incorporate your contribution quickly and easily:
[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue.mvn clean verify
to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.