-
Notifications
You must be signed in to change notification settings - Fork 15
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
E2E Tests parallel execution #2833
base: main
Are you sure you want to change the base?
Conversation
cabbeac
to
70aa966
Compare
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.
Thanks for this! I wonder what is the expected/observed speed-up with these changes ?
That's a good question! I observed an increase of speed of about 5 to 7 minutes, but this is just the first step, when we refactor the e2e tests, in order to make them less flaky and without some useless costly operations (mostly photofinish dump when not needed), this pr should make a lot more sense |
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.
Perfect! Really really appreciated!
Just a comment about putting the 4
as a variable
PD: I don't know if we would need some change in the artifact upload in failure cases, otherwise they might overwrite. Maybe we can add some sort of index to the folder name
id: prepare | ||
uses: bahmutov/gh-build-matrix@main | ||
with: | ||
n: 4 |
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.
Can we have this 4
in some variable on top?
So we can change if needed, as it is used in 2 places
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.
Also: where does this number come from? Should it be the number of cores?
steps: | ||
- name: Create matrix ⊹ | ||
id: prepare | ||
uses: bahmutov/gh-build-matrix@main |
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.
question: As far as I get it, this action just provides a range for enumeration, right? Like you say 4
and it spits [1,2,3,4]
SPLIT: ${{ strategy.job-total }} | ||
SPLIT_INDEX: ${{ strategy.job-index }} |
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.
question: Where do those variables come out? Shouldn't be something like strategy.matrix...
or prepare.output.matrix...
?
70aa966
to
1d1db69
Compare
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.
Thank you @CDimonaco, this PR is very valuable for me as it barely halves the CI execution time.
There are some unanswered questions in comments, besides that I think we should merge.
7ef888b
to
8dbec90
Compare
8dbec90
to
1ce4c97
Compare
Description
This pr splits the cypress tests execution across multiple CI jobs, we use the
cypress-split
plugin.This PR does not fix any issue on flakiness or test logic, it just speeds up the execution of test pipeline.