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

Faster unit test CI job #1201

Merged
merged 1 commit into from
Dec 2, 2019
Merged

Faster unit test CI job #1201

merged 1 commit into from
Dec 2, 2019

Conversation

GoodForOneFare
Copy link
Member

@GoodForOneFare GoodForOneFare commented Dec 1, 2019

Description

Speed up CI's unit test job a bit. Right now, it's the bottleneck on shipping, and very slow to give feedback about failing tests:

Travis CI snapshot showing the unit test job taking 2 minutes longer than the build job

This is because a pretest build step was added. This makes sense for local test runs where e2e tests are part of the flow. However, CI's unit test job excludes tests that need build artefacts, so there's no need to build.

Is it fast, tho?

Faster.

quilt - Travis CI 2019-12-01 17-51-57

Getting the build step faster is probably going to require Node 12 + TypeScript support for threaded builds.

Type of change

CI / infrastructure.

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above

@@ -7,8 +7,10 @@
"build": "yarn tsc --build packages",
"lint": "eslint '**/*.{ts,tsx}'",
"ci:lint-docs": "yarn generate docs && test -z \"$(git status --porcelain)\" || echo 'The root README has not been updated. Run `yarn generate docs` in the root of your quilt directory and try again.'",
"pretest": "yarn run build",
"test": "NODE_ICU_DATA=node_modules/full-icu jest --maxWorkers=3 --coverage=false",
"_test": "NODE_ICU_DATA=node_modules/full-icu jest",
Copy link
Member Author

Choose a reason for hiding this comment

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

Yuck :/

Copy link
Contributor

@ismail-syed ismail-syed left a comment

Choose a reason for hiding this comment

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

Good find 💯 🙏

@GoodForOneFare GoodForOneFare merged commit 64c467d into master Dec 2, 2019
@GoodForOneFare GoodForOneFare deleted the faster-ci-tests branch December 2, 2019 18:22
@patsissons patsissons temporarily deployed to production December 4, 2019 19:12 Inactive
@theodoretan theodoretan temporarily deployed to gem December 10, 2019 21:19 Inactive
@newtack
Copy link

newtack commented Dec 29, 2019

For what it's worth, we switched from Jest to Zora and our tests (just 35K lines of Typescript) went from 30 seconds to 2 seconds. It is very fast and we love it.

@GoodForOneFare
Copy link
Member Author

Thanks for the tip, @newtack! Zora looks really interesting, and it's always great to learn about a new contender 😄

In case anyone else stumbles across this PR, I'd like to make it clear that Jest is not a bottleneck in Shopify's CI. The low-hanging fruit here is TypeScript compilation. To that end, we've been experimenting with improving TypeScript's caching performance in CI, and could probably just do a build with type checking disabled to really speed things up.

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.

None yet

5 participants