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

build: test parallelization ⚡️ #3319

Merged
merged 21 commits into from
Aug 29, 2023
Merged

build: test parallelization ⚡️ #3319

merged 21 commits into from
Aug 29, 2023

Conversation

erickzhao
Copy link
Member

@erickzhao erickzhao commented Aug 24, 2023

Summary

This PR streamlines our CircleCI configuration in two ways.

Build once, test everywhere

In main, our CircleCI job matrix runs two test jobs for each target platform and architecture. In practice, this means 8 test jobs:

  • fast-tests-arm64-node/macos
  • fast-tests-x64-node/macos
  • fast-tests-x64-node/windows
  • fast-tests-x64-node/linux
  • slow-tests-arm64-node/macos
  • slow-tests-x64-node/windows
  • slow-tests-x64-node/macos
  • slow-tests-x64-node/linux

Each one of these jobs currently the entire linting and building process for Forge separately. These are both wasteful:

  • Lint: Static analysis of the code only needs to be performed once.
  • Build: Being pure JS, Forge only needs to be compiled a single time.

Thus, this PR adds a separate lint-and-build job that runs on Linux because the runner on that platform is fast and cheap.

This will reduce total CI time from $t_{total} = t_{b1} + t_{t1} + ... + t_{b8} + t_{t_8}$ to $t_{total}' = t_{b} + t_{t1} + ... + t_{t8}$, where $t_{bn} + t_{tn}$ is the build and test time respectively for job ${n}$.

However, in more practical terms, this will only reduce time-to-green by $\Delta = t_{slowest\ build} - t_{fastest\ build}$.

CircleCI test splitting

CircleCI has a test splitting feature that relies on submitting JUnit format test reports to Circle. #3316 added that reporting, and this PR follows it up by adding the test splitting feature to our Slow test suites.

In an ideal world where tests can be split evenly $n$ ways, this theoretically reduces the runtime of slow test jobs from $t_{slow}$ to $t_{slow}'$, where $n$ is the parallelization factor for the runners.

$t_{slow} = t_{overhead} + t_{tests}$
$t_{slow}' = t_{overhead} + \dfrac{t_{tests}}{n}$

In order to do so, a few things needed to be changed:

  • test-globber.ts was removed for some tweaks in .mocharc.js. yarn test:fast and yarn test:slow should still work as before, though.
  • Windows slow test CI code was simplified and streamlined with the rest of the Linux and macOS setup.

Outcomes

In practice, the first green run in this PR reduced the time-to-green time from 32:42 -> 25:58 (-20.6%). There are a few future improvements to explore:

  • Windows slow tests are really slow due to setup time. Half of the 23-minute runtime is dedicated to installing node modules and setting up wixtoolset via Chocolatey.
  • Something's wrong with the JUnit format and test splitting doesn't seem to work via timing data. More investigation is required to figure out how to get this working. This means that the parallelization of slow tests is not optimized.
  • I only applied $n=2$ test runners in parallel in this iteration of the PR. The number of runners is a tradeoff between time-to-green and total CI runtime.

@erickzhao erickzhao changed the title testing build: slow test parallelization Aug 24, 2023
@erickzhao erickzhao force-pushed the test-refactor-circleci branch 3 times, most recently from d3bd02b to 676a34f Compare August 25, 2023 01:15
@erickzhao erickzhao changed the title build: slow test parallelization build: parallelization ⚡️ Aug 25, 2023
@erickzhao erickzhao marked this pull request as ready for review August 29, 2023 00:18
@erickzhao erickzhao requested a review from a team as a code owner August 29, 2023 00:18
@erickzhao erickzhao changed the title build: parallelization ⚡️ build: test parallelization ⚡️ Aug 29, 2023
Copy link
Member

@VerteDinde VerteDinde left a comment

Choose a reason for hiding this comment

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

Amazing - thanks so much for adding this! 👏

.circleci/config.yml Outdated Show resolved Hide resolved
Copy link
Member

@erikian erikian left a comment

Choose a reason for hiding this comment

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

🚀

@erickzhao erickzhao merged commit 478d4a4 into main Aug 29, 2023
3 checks passed
@erickzhao erickzhao deleted the test-refactor-circleci branch August 29, 2023 20:18
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.

4 participants