Skip to content

Commit f923f78

Browse files
authored
chore: improve unit test CI performance via shared deps cache and Jest cache (#28419)
--- ## **Description** If we start to see OOM failures (exit code 137, "JavaScript heap out of memory") after this change, the first thing to try would be lowering --max_old_space_size from 20480 to something more realistic like 8192, which forces Node's GC to be more aggressive rather than hoarding memory until the OS kills the process. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> Unit tests in CI were taking **~18+ minutes** due to several avoidable bottlenecks. This PR addresses three root causes without requiring paid infrastructure: **Problems fixed:** 1. **Redundant dependency installation** — Each of 10 unit test shards, 2 component-view test shards, and the coverage merge job independently ran `yarn install --immutable` + `yarn setup:github-ci --node` (~3–5 min each), totalling 13 redundant installs per CI run. 2. **Jest transform cache disabled** — `cache: false` in `jest.config.js` forced every shard to re-parse and re-transform all ~3,060 test files through `babel-jest` from scratch on every run. 3. **Low parallelism within shards** — `maxWorkers: '20%'` on a 4-vCPU `ubuntu-latest` runner meant Jest used ~1 worker per shard, leaving 3 cores idle. **Changes:** - New `prepare-unit-test-deps` CI job runs `yarn install` + `setup:github-ci --node` once and caches `node_modules` via `actions/cache`. All downstream jobs restore from this cache (~30s) instead of installing independently. - Jest transform cache persisted across CI runs via `actions/cache`, keyed on `yarn.lock` + `babel.config.tests.js`. Transforms are only redone when dependencies or transform config change. - `cache: false` → `cache: true` in `jest.config.js`, with a configurable `cacheDirectory` via `JEST_CACHE_DIRECTORY` env var so CI controls the cache path. - `maxWorkers` changed from `'20%'` (always) to `'50%'` in CI / `'20%'` locally, utilizing ~2 workers on the 4-core runner instead of ~1. **Expected impact (warm cache):** | Metric | Before | After | |---|---|---| | Setup time per shard | ~3–5 min | ~30s | | Transform time per shard | Full rebuild every run | Cached | | Jest workers per shard | ~1 | ~2 | | **Estimated wall-clock time** | **~18+ min** | **~8–12 min** | ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** N/A ## **Screenshots/Recordings** ### **Before** <img width="1079" height="381" alt="image" src="https://github.com/user-attachments/assets/4d2a32f5-6ab0-410d-838e-b902172567a2" /> N/A ### **After** <img width="976" height="318" alt="image" src="https://github.com/user-attachments/assets/25655008-6956-49b7-8a4f-db1bf72ea12c" /> N/A ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- Generated with the help of the pr-description AI skill --> Made with [Cursor](https://cursor.com) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes CI test execution by introducing cross-job workspace caching and enabling Jest caching, which could cause flaky runs or cache-miss failures if the cache keys/paths are incorrect. No production runtime code is affected. > > **Overview** > Improves CI test performance by introducing a new `prepare-unit-test-deps` job that installs once and caches a prepared workspace (`node_modules`, `.yarn`, and generated terms-of-use content) for all unit-test shards, component-view shards, and the coverage merge job. > > Enables and persists Jest’s transform cache across CI runs (shared `JEST_CACHE_DIRECTORY` at `/tmp/jest_rs` with an `actions/cache` key based on `yarn.lock` and `babel.config.tests.js`) and increases Jest parallelism in CI by setting `maxWorkers` to `50%` when `process.env.CI` is set. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit cafd2e6. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent bfc8e9a commit f923f78

2 files changed

Lines changed: 70 additions & 29 deletions

File tree

.github/workflows/ci.yml

Lines changed: 65 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -259,15 +259,14 @@ jobs:
259259
run: ${{ steps.download-actionlint.outputs.executable }} -color -config-file .github/actionlint.yaml
260260
shell: bash
261261

262-
unit-tests:
263-
name: Unit tests (${{ matrix.shard }})
262+
# === Unit Test Performance ===
263+
# See docs/ci-unit-test-performance.md for full rationale, applied optimizations, and future proposals.
264+
prepare-unit-test-deps:
265+
name: Prepare unit test dependencies
264266
runs-on: ubuntu-latest
265267
if: github.event_name != 'merge_group' || needs.check-skip-merge-queue.outputs.skip-merge-queue != 'true'
266268
needs:
267269
- check-skip-merge-queue
268-
strategy:
269-
matrix:
270-
shard: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
271270
steps:
272271
- uses: actions/checkout@v3
273272
- uses: actions/setup-node@v3
@@ -283,13 +282,54 @@ jobs:
283282
command: yarn install --immutable
284283
- name: Clean state and following up dependencies installation
285284
run: yarn setup:github-ci --node
285+
- name: Cache prepared workspace
286+
uses: actions/cache/save@v4
287+
with:
288+
path: |
289+
node_modules
290+
.yarn
291+
app/util/termsOfUse/termsOfUseContent.ts
292+
key: unit-test-deps-${{ github.sha }}
293+
294+
unit-tests:
295+
name: Unit tests (${{ matrix.shard }})
296+
runs-on: ubuntu-latest
297+
if: github.event_name != 'merge_group' || needs.check-skip-merge-queue.outputs.skip-merge-queue != 'true'
298+
needs:
299+
- check-skip-merge-queue
300+
- prepare-unit-test-deps
301+
strategy:
302+
matrix:
303+
shard: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
304+
steps:
305+
- uses: actions/checkout@v3
306+
- uses: actions/setup-node@v3
307+
with:
308+
node-version-file: '.nvmrc'
309+
- name: Restore prepared workspace
310+
uses: actions/cache/restore@v4
311+
with:
312+
path: |
313+
node_modules
314+
.yarn
315+
app/util/termsOfUse/termsOfUseContent.ts
316+
key: unit-test-deps-${{ github.sha }}
317+
fail-on-cache-miss: true
318+
- name: Restore Jest transform cache
319+
uses: actions/cache@v4
320+
with:
321+
path: /tmp/jest_rs
322+
key: jest-transform-${{ hashFiles('yarn.lock', 'babel.config.tests.js') }}-${{ matrix.shard }}
323+
restore-keys: |
324+
jest-transform-${{ hashFiles('yarn.lock', 'babel.config.tests.js') }}-
286325
- name: Prepare results directory
287326
run: mkdir -p tests/results
288327
# The "10" in this command is the total number of shards. It must be kept
289328
# in sync with the length of matrix.shard
290329
- run: yarn test:unit --shard=${{ matrix.shard }}/10 --forceExit --silent --coverageReporters=json --json --outputFile=tests/results/unit-test-results-${{ matrix.shard }}.json
291330
env:
292331
NODE_OPTIONS: --max_old_space_size=20480
332+
JEST_CACHE_DIRECTORY: /tmp/jest_rs
293333
- name: Rename coverage report and extract test count for this shard
294334
shell: bash
295335
run: |
@@ -317,23 +357,22 @@ jobs:
317357
# threshold calculation is accurate.
318358
merge-unit-and-component-view-tests:
319359
runs-on: ubuntu-latest
320-
needs: [unit-tests, component-view-tests]
360+
needs: [unit-tests, component-view-tests, prepare-unit-test-deps]
321361
if: ${{ !cancelled() && github.event_name != 'merge_group' }}
322362
steps:
323363
- uses: actions/checkout@v3
324364
- uses: actions/setup-node@v3
325365
with:
326366
node-version-file: '.nvmrc'
327-
cache: yarn
328-
- name: Install Yarn dependencies with retry
329-
uses: nick-fields/retry@ce71cc2ab81d554ebbe88c79ab5975992d79ba08 #v3.0.2
330-
with:
331-
timeout_minutes: 10
332-
max_attempts: 3
333-
retry_wait_seconds: 30
334-
command: yarn install --immutable
335-
- name: Clean state and following up dependencies installation
336-
run: yarn setup:github-ci --node
367+
- name: Restore prepared workspace
368+
uses: actions/cache/restore@v4
369+
with:
370+
path: |
371+
node_modules
372+
.yarn
373+
app/util/termsOfUse/termsOfUseContent.ts
374+
key: unit-test-deps-${{ github.sha }}
375+
fail-on-cache-miss: true
337376
- uses: actions/download-artifact@v4
338377
with:
339378
pattern: coverage-*
@@ -428,6 +467,7 @@ jobs:
428467
if: github.event_name != 'merge_group' || needs.check-skip-merge-queue.outputs.skip-merge-queue != 'true'
429468
needs:
430469
- check-skip-merge-queue
470+
- prepare-unit-test-deps
431471
strategy:
432472
matrix:
433473
shard: [1, 2]
@@ -436,16 +476,15 @@ jobs:
436476
- uses: actions/setup-node@v3
437477
with:
438478
node-version-file: '.nvmrc'
439-
cache: yarn
440-
- name: Install Yarn dependencies with retry
441-
uses: nick-fields/retry@ce71cc2ab81d554ebbe88c79ab5975992d79ba08 #v3.0.2
442-
with:
443-
timeout_minutes: 10
444-
max_attempts: 3
445-
retry_wait_seconds: 30
446-
command: yarn install --immutable
447-
- name: Clean state and following up dependencies installation
448-
run: yarn setup:github-ci --node
479+
- name: Restore prepared workspace
480+
uses: actions/cache/restore@v4
481+
with:
482+
path: |
483+
node_modules
484+
.yarn
485+
app/util/termsOfUse/termsOfUseContent.ts
486+
key: unit-test-deps-${{ github.sha }}
487+
fail-on-cache-miss: true
449488
- name: Prepare results directory
450489
run: mkdir -p tests/results
451490
- run: |

jest.config.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ const config = {
6060
],
6161
coverageReporters: ['text-summary', 'lcov'],
6262
coverageDirectory: '<rootDir>/tests/coverage',
63-
maxWorkers: process.env.NODE_ENV === 'production' ? '50%' : '20%',
63+
maxWorkers: process.env.CI ? '50%' : '20%',
6464
moduleNameMapper: {
6565
'\\.(svg)$': '<rootDir>/app/__mocks__/svgMock.js',
6666
'\\.(png)$': '<rootDir>/app/__mocks__/pngMock.js',
@@ -89,8 +89,10 @@ const config = {
8989
'<rootDir>/app/__mocks__/spinnerMock.js',
9090
'^rive-react-native$': '<rootDir>/app/__mocks__/rive-react-native.tsx',
9191
},
92-
// Disable jest cache
93-
cache: false,
92+
cache: true,
93+
...(process.env.JEST_CACHE_DIRECTORY && {
94+
cacheDirectory: process.env.JEST_CACHE_DIRECTORY,
95+
}),
9496
};
9597

9698
// eslint-disable-next-line import-x/no-commonjs

0 commit comments

Comments
 (0)