Skip to content
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

Make some executor tests more deterministic #1373

Merged
merged 3 commits into from
Mar 26, 2020

Conversation

imiric
Copy link
Contributor

@imiric imiric commented Mar 25, 2020

This refactors a couple of tests mentioned in #1357 to reduce their chances of failure. :) It doesn't get rid of the flakiness entirely, but merely shifts things around and uses safer timings. In my tests the behavior is much more stable now, though there could still be some failures on Windows during quick CPU usage bursts, which I'm hopeful we won't run into on GH Actions.

After these changes stress testing both for about an hour didn't produce failures on Linux nor Windows:
2020-03-25-145505_887x773_scrot
2020-03-25-145613_1208x1053_scrot

Whereas on new-schedulers both tests fail quickly:
2020-03-25-150045_888x772_scrot

It's likely these might be more susceptible to failure when run as part of the entire package instead of individually, and while I didn't see them fail when stress testing lib/executor, I did see others mentioned here, some of which I'll take a look at next.

Moving forward, it might be a good idea to setup stress testing of certain packages as a periodic CI task so we can catch the flakiness sooner.

Ivan Mirić added 3 commits March 25, 2020 11:39
This doesn't get rid of the flakiness entirely, but merely shifts things
around and uses safer timings to reduce the possibility of failures.
In my tests the behavior is much more stable now, though there are still
issues on Windows during quick CPU usage bursts, which I'm hopeful we
won't run into on GH Actions.
This doesn't get rid of the flakiness entirely, but merely shifts things
around and uses safer timings to reduce the possibility of failures.
In my tests the behavior is much more stable now, though there are still
issues on Windows during quick CPU usage bursts, which I'm hopeful we
won't run into on GH Actions.
This does the same functional check as before in a simpler way, while
also ensuring all values were covered, whereas in the previous version a
sequence of repeating values that doesn't decrement would erroneously
pass the test.
@imiric imiric requested review from mstoykov and na-- March 25, 2020 14:57
Copy link
Collaborator

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I like the newer one more as well :)

I don't think that catching flakiness with CI will be worth the time ... it is probably just better to remember to run the test a few hundred times if we ever change them, but maybe I am wrong

@imiric imiric merged commit 6507287 into new-schedulers Mar 26, 2020
@imiric imiric deleted the fix/1357-flaky-tests branch March 26, 2020 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants