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

🏗 Parallelizes execution of unit tests and e2e test on CircleCI #35227

Merged

Conversation

danielrozenberg
Copy link
Member

@danielrozenberg danielrozenberg commented Jul 14, 2021

This PR:

  • Parallelizes unit tests and e2e test (for both the base flavor and experimental flavors) into 16 shards, using CircleCI's build-in support for parallelizing tests
    • Parallelized jobs now have the ⛓️ emoji in their name to indicate that they're paralellized
  • Splits the --local_changes into a separate job which is a prerequisite to the (now renamed) All Unit Tests job
    • This job explicitly halts early and passes execution to All Unit Tests when running on push builds, as it has no meaning outside of pull requests
  • Splits experiment builds' e2e tests and integration tests into separate jobs to enable parallelization on the e2e tests

@danielrozenberg danielrozenberg force-pushed the circleci-unit-parallelism branch 30 times, most recently from 254e36f to fed1d52 Compare July 14, 2021 21:13
@danielrozenberg danielrozenberg force-pushed the circleci-unit-parallelism branch from d82996a to 0490bf6 Compare July 20, 2021 17:41
'All Unit Tests':
executor:
name: amphtml-large-executor
parallelism: 16
Copy link
Contributor

Choose a reason for hiding this comment

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

I was suggesting going below 16, not above. For posterity, could you share links that show the total duration and the total CPU usage for 4, 8, and 16 respectively (and any other parallelism factors you think are worth trying)?

@danielrozenberg danielrozenberg force-pushed the circleci-unit-parallelism branch from 66ee65b to 62c4369 Compare July 20, 2021 19:36
@danielrozenberg danielrozenberg force-pushed the circleci-unit-parallelism branch 2 times, most recently from c2083d1 to 044b444 Compare July 22, 2021 18:09
@danielrozenberg danielrozenberg force-pushed the circleci-unit-parallelism branch from 1f366da to e1273f6 Compare July 23, 2021 16:26
Copy link
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

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

This was a valiant effort, and the results in #35227 (comment) are very exciting. A ~30% reduction in running time for a ~10% increase in credits used is an excellent tradeoff.

LGTM!

@danielrozenberg danielrozenberg merged commit 3bd007b into ampproject:main Jul 23, 2021
@danielrozenberg danielrozenberg deleted the circleci-unit-parallelism branch July 23, 2021 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants