-
-
Notifications
You must be signed in to change notification settings - Fork 84
Fix nondeterministic test failures #173
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
Comments
@AndreasArvidsson @brxck lets move the sleep discussion here. Here's @AndreasArvidsson's comment from the PR:
|
@AndreasArvidsson It's actually only macOS in CI, so we might be able to be even more specific by checking env var to see if CI as well as your os check It's quite odd it needs a sleep at all tbh because there's nothing async going on there. It's just setting some stuff in an object. |
Then that sounds like a good solution :) I've already pushed the operating system change. Which env var would be the easiest way to detect CI? |
I'll say that tests on Ubuntu occasionally had the same problem, including my own (not slow) machine. It is a weird problem... I had an alternative idea of adding retry logic to where we check the navigationMap? |
Yeah given @brxck 's comment I think exponential backoff is the way to go |
A retry logic sounds like a good idea. |
A retry logic is now implemented. Doing three tries with 100 mill seconds between each. |
Looks like that still failed; I switched it to exponential so hopefully that's more reliable |
Weird that didn't seem to help. Maybe it's not a timing thing? Maybe it's actually just nondeterministic for some reason? We should figure out how get more useful output from the assertion |
Yes if a three second sleep doesn't help it's not that. |
Yeah the first question to figure out is whether it is not doing the decorations or if it is doing them but they don't match our expectations |
I kind of have the feeling that the sleep makes no difference 😂 |
Well that's just great then :D |
Let's start by just retrying tests 3 times if they fail. This approach is terrible, because it swallows all nondeterministic bugs, but at least we can start requiring CI passes, unlike now where they're disabled because they fail
Originally posted by @pokey in #172 (comment)
The text was updated successfully, but these errors were encountered: