-
Notifications
You must be signed in to change notification settings - Fork 84
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
Parallelize conformance test runs in CI #1110
Conversation
780326a
to
9755053
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.
Nice improvement!
There are still two cases of sequential runs:
- node runs server and client tests in sequence.
- web runs promise client and callback client in sequence.
Parallelizing both can potentially bring overall runtime below 3min. Considering that we want to move to turborepo, it may not be worth it to further split up the jobs.
But I do wonder why chrome tests takes significantly longer than the two other browsers. Looking at these two runs, chrome takes as long as safari and firefox combined: A, B. Is there something wrong with our setup?
Our setup should be OK. I checked it against webdriver's docs and it looks fine to me. It's a bit variable too. In this run, the safari tests seem to have taken a bit longer and chrome's tests were around 3m. So, this may be a function of how long it takes the chrome driver to start? Not entirely sure. Also updated to further parallelize so now we're around 4 minutes or so. I don't love that proliferation of checks now and how the Makefile has kind of blown up, so if you know of a better way around this, lmk. Just feels kind of unwieldy and hard to maintain as-is. |
Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
chore: fix some comments Signed-off-by: sjtucoder <[email protected]> Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
e5ac897
to
a278f48
Compare
Thanks for looking into chrome.
I agree. It's a lot of boilerplate to save 1 minute. IMO we should punt on it, and parallelize further with #798. |
a278f48
to
b36703b
Compare
Signed-off-by: Steve Ayers <[email protected]>
Signed-off-by: Steve Ayers <[email protected]>
@timostamm sounds good. I merged in your changes from #1111 and reverted the commits that further parallelized callback vs. promise client and Node clients and servers. |
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.
LGTM!
This parallelizes the various test runs for conformance in CI to minimize the duration of the entire process.
Previously, the repo was basically only running the web and conformance tests in parallel. However, each of these have various runs which were running serially. Now, each conformance suite run is split into its own Github workflow. In addition, Node version matrices were added to the Node-related conformance runs to add further coverage without affecting overall time. This results in the following all running concurrently:
Node tests:
Web tests:
Note that conformance tests are still run as part of the
all
default goal, so local runs of justmake
are unaffected. However, the conformance tests are no longer a pre-req of thetest
target, so simply runningtest
will not run the conformance tests. To do so, runmake testconformance
.The above enhancements result in an improvement of approximately 8 minutes on CI runs. The overall run duration is tied to the longest individual workflow, which happens to be connect-web conformance in Chrome at 5 minutes. Therefore, the overall run is around 5 minutes, where previously it was 13 minutes or more.