-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Feature: Adds parallel GitHub actions for e2e tests to get rid of flaky tests #4977
Feature: Adds parallel GitHub actions for e2e tests to get rid of flaky tests #4977
Conversation
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
p.s. I have no idea why I had to format the docs, but.. yea it's included as well! |
Signed-off-by: Wiard van Rij <wiard@outlook.com>
I wonder why this still has
Even though they have been removed :/ Failing |
Regarding docs, if you ran |
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.
Looks good! (Minus doc check). Nice find with the Gotesplit 👍
The docs are actually better, but there is some quirk that the In other words, the changed docs files are okay for now, even though it should not have been part of this PR. I think when we merged the other PR, it should have 0 diff anymore on it. As for Bingo. I was implementing this, but hit this issue that gets fixed as well: bwplotka/bingo#106
|
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Signed-off-by: Wiard van Rij <wiard@outlook.com>
Indeed it looks like you are correct - |
@wiardvanrij awesome work & a clever hack 🚀 |
Changes
I believe quite a lot of our e2e tests simply hit a deadline due to
-timeout 10m
Now, we could increase that, but I personally dislike builds > 10 min :)
This feature implements a pretty cool golang bin via gotesplit - which we fetch via scripts/gotesplit.sh. We place it in the /tmp/bin just like the Shellcheck bin.
What this tool does is it splits the tests based on the amount of parallelism you want. At the moment it does 0 on local dev work, which is exactly the same behavior as it already is. Mostly because our own computers do not benefit from the parallelism in our "runner", because that's our PC. We already do parallelism in the tests itself, which is perfect for that.
However for cloud based runners, it helps to have n-amount of runners, as it does not scale well vertically, but actually does it pretty good horizontally.
An example from my fork:
We increase the total amount of build minutes by roughly 40%, but we reduce the build time by 30-40% and actually remove quite a lot of flaky tests. Probably due to lack of GitHub runners and/or busy servers(?).
It can be further improved to actually cache the docker build stage so that each parallel testing job has access to the freshly prepared container image of Thanos.
Verification
Tested it locally and on my fork.