feat: parallelize jest tests with sharding #1550
Conversation
WalkthroughAdds a new CI workflow (.github/workflows/test.yml) that shards Jest tests and runs a dependent build; updates package.json exports and build/test scripts; removes an existing workflow (.github/workflows/build-rad-ui.yml). Changes
Sequence Diagram(s)sequenceDiagram autonumber actor Dev as Push/PR to main participant GH as GitHub Actions participant Runner as ubuntu-latest participant Node as actions/setup-node@v4 (Node 20) participant npm as npm participant Jest as Jest Dev->>GH: push / open PR targeting main rect rgba(230,240,255,0.6) note over GH: concurrency per workflow+ref\ncancel in-progress runs end GH->>Runner: start test job (matrix shards 1/2 & 2/2) Runner->>Runner: actions/checkout@v4 Runner->>Node: setup Node (v20, npm cache) Runner->>npm: npm ci Runner->>Jest: npm run test:shard (JEST_SHARD=1/2 or 2/2) Jest-->>Runner: test results Runner-->>GH: report shard status GH->>Runner: start build job (after tests) Runner->>Runner: checkout + setup-node Runner->>npm: npm ci Runner->>npm: npm run build:rollup (clean -> parallel builds -> generate-exports) Runner-->>GH: build status GH-->>Dev: workflow result Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews: pre_merge_checks: custom_checks: - name: "Undocumented Breaking Changes" mode: "warning" instructions: | Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
package.json (1)
15-15: Make shard script cross‑platformpackage.json has "test:shard": "jest --shard=$JEST_SHARD" (line 15) and .github/workflows/test.yml sets JEST_SHARD via matrix. $JEST_SHARD won't expand on Windows shells — either use cross-env-shell or inject the matrix value in CI.
Apply:
- "test:shard": "jest --shard=$JEST_SHARD", + "test:shard": "cross-env-shell \"jest --shard=$JEST_SHARD\"",And add devDep (outside this hunk):
"devDependencies": { + "cross-env": "^7.0.3",If you prefer no new deps, call Jest directly in the workflow with the matrix value instead (workflow already sets JEST_SHARD).
.github/workflows/test.yml (3)
38-41: Pass shard value directly; avoid env expansion dependencySimplifies CI and works regardless of package script/env quirks.
- - name: Run tests - env: - JEST_SHARD: ${{ matrix.shard }}/${{ matrix.total }} - run: npm run test:shard + - name: Run tests + run: npx jest --shard=${{ matrix.shard }}/${{ matrix.total }}
13-15: Concurrency key: use PR number when availablePrevents accidental grouping across different refs for PR events.
-concurrency: - group: ${{ github.workflow }}-${{ github.ref }} +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true
18-26: Optional: add a job timeoutAvoids hung jobs blocking CI.
test: + timeout-minutes: 20
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/test.yml(1 hunks)package.json(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: coverage
🔇 Additional comments (1)
package.json (1)
7-10: Add "." export and types — verify built entry paths firstWhen "exports" is defined consumers cannot import the package root; add a "." export mapping to the built JS and typings. dist/ is not present in the workspace — confirm the built entry and .d.ts path (e.g. dist/index.js or dist/index.cjs and dist/index.d.ts) before merging. Verify with: fd -a 'index.@(js|mjs|cjs|d.ts)' .
Apply (if those files exist):
- "exports": { - "./themes/default.css": "./dist/themes/default.css", - "./themes/tailwind-presets/default.js": "./dist/themes/tailwind-presets/default.js" - }, + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js" + }, + "./themes/default.css": "./dist/themes/default.css", + "./themes/tailwind-presets/default.js": "./dist/themes/tailwind-presets/default.js" + },
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/test.yml (2)
29-34: Upgrade to supported Node LTS (Node 18 is EOL).Use Node 20 (or 22 LTS) to avoid security and tooling issues. Mirror this in all jobs.
- node-version: '18' + node-version: '20'
49-54: Also bump Build job to Node 20.- node-version: '18' + node-version: '20'
🧹 Nitpick comments (5)
.github/workflows/test.yml (2)
38-41: Pass shard via CLI arg; avoid env interpolation dependency.This makes the command portable and removes reliance on shell expansion. Adjust workflow and simplify the script.
Workflow change:
- - name: Run tests - env: - JEST_SHARD: ${{ matrix.shard }}/${{ matrix.total }} - run: npm run test:shard + - name: Run tests + run: npm test -- --shard=${{ matrix.shard }}/${{ matrix.total }}Then in package.json (see separate comment), update "test:shard" to just "jest" or drop it.
19-26: Optional: Add timeouts to prevent hung jobs.Helpful for stuck shards/builds.
test: name: Jest ${{ matrix.shard }}/${{ matrix.total }} runs-on: ubuntu-latest + timeout-minutes: 20 ... build: name: Build runs-on: ubuntu-latest needs: test + timeout-minutes: 20package.json (3)
7-10: Verify published files align with new subpath exports.Ensure these files exist post-build and are included in the npm package. Consider a "files" whitelist to guarantee inclusion.
{ "name": "@radui/ui", ... - "exports": { + "exports": { "./themes/default.css": "./dist/themes/default.css", "./themes/tailwind-presets/default.js": "./dist/themes/tailwind-presets/default.js" }, + "files": [ + "dist/**" + ], ... }
15-15: Make sharded test script portable or drop it.Env interpolation
$JEST_SHARDisn’t portable on Windows shells. Simplest: let CI pass the arg and keep script as plain jest.Option A (portable, keep script):
- "test:shard": "jest --shard=$JEST_SHARD", + "test:shard": "jest"Then run in CI:
npm run test:shard -- --shard=${{ matrix.shard }}/${{ matrix.total }}.Option B (keep env-based UX, add cross-env-shell):
+ "test:shard": "cross-env-shell \"jest --shard=\\${JEST_SHARD:-1/1}\"",And add devDependency:
cross-env.
33-33: Cross-platform parallel build.
& ... & waitis bash-specific; use a cross-platform runner to avoid Windows dev friction.Using npm-run-all:
- "build:rollup:process": "npm run clean && (npm run build:css & npm run build:components & wait) && npm run build:generate-exports", + "build:rollup:process": "npm run clean && npm-run-all -p build:css build:components && npm run build:generate-exports",Add devDependency:
+ "npm-run-all": "^4.1.5",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/test.yml(1 hunks)package.json(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-14T18:13:36.606Z
Learnt from: GoldGroove06 PR: rad-ui/ui#1214 File: package.json:64-64 Timestamp: 2025-07-14T18:13:36.606Z Learning: In npm, lifecycle scripts with `pre` and `post` prefixes automatically run before and after their corresponding main scripts. For example, a `prebuild` script will automatically run before the `build` script when executing `npm run build`, so there's no need to explicitly call `npm run prebuild` in the build script. Applied to files:
package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: coverage
🔇 Additional comments (1)
.github/workflows/test.yml (1)
43-59: Build currently fails (Rollup dts). Decide gating strategy.If the failure is known and being fixed separately, consider temporarily marking build as non-blocking, or keep it blocking to enforce green PRs. Your call—just confirming intent.
Option (temporary, not generally recommended):
build: name: Build runs-on: ubuntu-latest needs: test + continue-on-error: true
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/test.yml (1)
29-34: Node 20 LTS + npm cache: LGTM (resolves prior comment).Consider explicitly keying the cache to all lockfiles in a workspace.
Apply:
uses: actions/setup-node@v4 with: node-version: '20' cache: 'npm' + cache-dependency-path: | + package-lock.json + **/package-lock.json
🧹 Nitpick comments (5)
.github/workflows/test.yml (5)
18-26: Add job timeouts to avoid hanging runs.Set conservative timeouts for matrix shards.
test: name: Jest ${{ matrix.shard }}/${{ matrix.total }} runs-on: ubuntu-latest + timeout-minutes: 20
42-46: Add timeout to build job.Prevents indefinite hangs on type generation/build.
build: name: Build runs-on: ubuntu-latest needs: test + timeout-minutes: 20
58-59: Fail faster with a pre-build typecheck.Catches d.ts/TS errors before Rollup (use --if-present to avoid breaking).
- - name: Build package - run: npm run build:rollup + - name: Type-check + run: npm run typecheck --if-present + + - name: Build package + run: npm run build:rollup
3-7: Optional: Add manual trigger.Allows on-demand runs without a push/PR.
on: push: branches: [ main ] pull_request: branches: [ main ] + workflow_dispatch:
49-54: Mirror cache path tweak in build job.Keep cache keys consistent across jobs.
uses: actions/setup-node@v4 with: node-version: '20' cache: 'npm' + cache-dependency-path: | + package-lock.json + **/package-lock.json
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: coverage
🔇 Additional comments (2)
.github/workflows/test.yml (2)
9-12: Principle-of-least-privilege looks good.Read-only perms for contents and PRs are appropriate for this workflow.
13-15: Good use of concurrency to auto-cancel superseded runs.Grouping by workflow and ref is correct for both pushes and PRs.
CoverageThis report compares the PR with the base branch. “Δ” shows how the PR affects each metric.
Coverage improved or stayed the same. Great job! Run |
Summary
Testing
JEST_SHARD=1/2 npm run test:shardnpm run build:rollup(fails: [plugin dts] RollupError: src/components/ui/Select/fragments/SelectItem.tsx: Failed to compile)Summary by CodeRabbit
New Features
Tests
Chores