[CI] testRandomTimeIntervalRounding Failure #56400
Comments
tvernum
added
:Core/Infra/Core
|
Pinging @elastic/es-core-infra (:Core/Infra/Core) |
Not sure why, but this does reproduce for me. |
|
Sorry folks! I'll dig as soon as I wake up for real. Feel free to revert if
it is loud.
…On Fri, May 8, 2020, 01:42 Ryan Ernst ***@***.***> wrote:
Reproduces locally?: No
Not sure why, but this *does* reproduce for me.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#56400 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABUXITVXFVMQ4DTVJUSJ6LRQOLTFANCNFSM4M34C23Q>
.
|
nik9000
added
:Analytics/Aggregations
|
Pinging @elastic/es-analytics-geo (:Analytics/Aggregations) |
elasticmachine
added
the
Team:Analytics
|
This doesn't reproduce for me, but I bet it is variable depending on the tzdb file that is in the distribution. Fun times. I was able to make some tests fail by running the random tests a ton of times. I'm going to fix those and hope that fixes this. |
|
OK! It looks like I made some assumptions about the shape of the "current transition list" vs the "future transition rules" that was incorrect. It caused duplicate transitions in my list which oddly, never broke with time unit rounding, but does break with time interval rounding. They do use different strategies, especially around overlaps. Anyway, the tzdb for I'll open a PR with assertions that'll trip if we hit this. |
|
Just an fyi, another instance of this failure: https://gradle-enterprise.elastic.co/s/2ua7hql4oeqec/tests/:server:test/org.elasticsearch.common.RoundingTests/testRandomTimeIntervalRounding#1 |
This fixes a bug in the interval rounding and two test bugs that showed up when I ran 1000s of iterations of the tests. The bug was that we could end up with duplicate transitions if we only needed the last transition from the list of fully defined transitions and the transitions overlapped with the rules. This happens. Its ok. We had defence against that if we needed more than one transition but forgot it in this case. Now we have it and assertions to make sure we catch any similar mistakes. One test bug had to do with the randomized test calling `round(round(min))` because sometimes the first `round` call will return a time less than min. Specifically if `min` is near daylight savings time. Anyway, this change fixes it by building an extra-wide prepared rounding just in case. The other test bug came up when adding a test for the real bug, namely that `assertRoundingAtOffset` made an assertion that wasn't true if the time to round ended up being in an overlap. This just drops that assertion. We have similar assertions in cases where we *know* what kind of offsets we're working with in other tests. Closes elastic#56400
|
I opened a PR to fix it. It looks like about 10% of the tests fail my machine. I'm kind of surprised I didn't bump into this. |
This fixes a bug in the interval rounding and two test bugs that showed up when I ran 1000s of iterations of the tests. The bug was that we could end up with duplicate transitions if we only needed the last transition from the list of fully defined transitions and the transitions overlapped with the rules. This happens. Its ok. We had defence against that if we needed more than one transition but forgot it in this case. Now we have it and assertions to make sure we catch any similar mistakes. One test bug had to do with the randomized test calling `round(round(min))` because sometimes the first `round` call will return a time less than min. Specifically if `min` is near daylight savings time. Anyway, this change fixes it by building an extra-wide prepared rounding just in case. The other test bug came up when adding a test for the real bug, namely that `assertRoundingAtOffset` made an assertion that wasn't true if the time to round ended up being in an overlap. This just drops that assertion. We have similar assertions in cases where we *know* what kind of offsets we're working with in other tests. Closes #56400
This fixes a bug in the interval rounding and two test bugs that showed up when I ran 1000s of iterations of the tests. The bug was that we could end up with duplicate transitions if we only needed the last transition from the list of fully defined transitions and the transitions overlapped with the rules. This happens. Its ok. We had defence against that if we needed more than one transition but forgot it in this case. Now we have it and assertions to make sure we catch any similar mistakes. One test bug had to do with the randomized test calling `round(round(min))` because sometimes the first `round` call will return a time less than min. Specifically if `min` is near daylight savings time. Anyway, this change fixes it by building an extra-wide prepared rounding just in case. The other test bug came up when adding a test for the real bug, namely that `assertRoundingAtOffset` made an assertion that wasn't true if the time to round ended up being in an overlap. This just drops that assertion. We have similar assertions in cases where we *know* what kind of offsets we're working with in other tests. Closes elastic#56400
Build scan: https://gradle-enterprise.elastic.co/s/plit7g4fdojhi/tests/
Repro line:
Reproduces locally?: No
Applicable branches: master
Failure excerpt:
The text was updated successfully, but these errors were encountered: