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

🏗 Cache Karma's babel transforms during CircleCI builds #32295

Merged
merged 1 commit into from
Jan 29, 2021
Merged

🏗 Cache Karma's babel transforms during CircleCI builds #32295

merged 1 commit into from
Jan 29, 2021

Conversation

rsimha
Copy link
Contributor

@rsimha rsimha commented Jan 29, 2021

AMP's Karma tests persist their babel transforms in .karma-cache (#28157). We lost the ability to cache the directory on Travis due to #28834 (comment). This PR reintroduces caching for the directory on CircleCI.

Results are great. We shave off ~30 secs from integration test runs, and ~5 mins from unit test runs.

Cold cache:
image

Warm cache:
image

Coming up: Caching for babel transforms in end-to-end tests. (Edit: Doesn't save more than a few seconds, so not worth it.)

@rsimha rsimha self-assigned this Jan 29, 2021
Copy link
Member

@samouri samouri left a comment

Choose a reason for hiding this comment

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

🔥

@rsimha rsimha merged commit 4443062 into ampproject:master Jan 29, 2021
@rsimha rsimha deleted the 2021-01-28-KarmaCacheCircleCI branch January 29, 2021 05:43
@rsimha
Copy link
Contributor Author

rsimha commented Jan 29, 2021

Question for you, @jridgewell. As things stand today, CircleCI uses a hash of package-lock.json as its cache key. This means it won't rewrite the cache if the lockfile hasn't changed (e.g. last step of this build).

However, we've configured browserify-persist-fs to bust the cache and re-transform all files when there's a change in either package-lock.json or a bunch of files in build-system.

const hashObject = {
deps: createHash(fs.readFileSync('./package-lock.json')),
build: globby
.sync([
'build-system/**/*.js',
'!build-system/eslint-rules',
'!**/test/**',
])
.map((f) => createHash(fs.readFileSync(f))),
};

This means if build-system changes but package-lock.json doesn't, unit tests will remain slow on CircleCI until the next change to package-lock.json. CircleCI docs don't seem to show a way to use globs to generate the cache key. Can you think of a workaround to optimize this further?

zaparent pushed a commit to zaparent/amphtml that referenced this pull request Jan 29, 2021
westonruter added a commit to westonruter/amphtml that referenced this pull request Jan 29, 2021
* 'master' of github.com:ampproject/amphtml: (1020 commits)
  🏗 Clean up CI config files (ampproject#32303)
  🏗 Remove Browser Installation from CI tasks and use smaller instances when possible (ampproject#32310)
  Cleanup fie-resources experiment (ampproject#32226)
  🏗  Fix bug in build target discovery logic (ampproject#32307)
  📖 Update TOC on TESTING.md (ampproject#32304)
  🏗 Reorganize browserify caching code (ampproject#32297)
  Fix <textarea> scrollHeight calculation (ampproject#32292)
  📖  Update testing docs (ampproject#32298)
  Fix missing space and mention `https` (ampproject#32293)
  ✨  [Story auto-analytics] Added validation and tests (ampproject#32288)
  🏗  Consolidate remaining CircleCI VM setup steps into separate scripts (ampproject#32290)
  🏗  Cache Karma's babel transforms during CircleCI builds (ampproject#32295)
  Add validation rules for aspect-ratio support via SSR (ampproject#32262)
  bump up viewer-messaging version (ampproject#32286)
  🖍🚀 Alternate `position: fixed/absolute` when docking/undocking (ampproject#32243)
  ✨ [Panning media] Transition siblings by ID (ampproject#32217)
  📦 Update dependency watchify to v4 (ampproject#32284)
  📦 Update dependency rollup to v2.38.1 (ampproject#32274)
  🐛♻️ ADS: XHRs race condtion and responsivnes fix (ampproject#32271)
  Add support for gdpr_conseented_providers, useCCPA_USPAPI, and _fw_us_privacy (ampproject#32275)
  ...
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.

None yet

3 participants