ci(cli): shard unit tests and merge coverage by betegon · Pull Request #1727 · getsentry/toolkit

betegon · 2026-10-29T17:37:31Z

Runs the CLI unit tests in three parallel shards and merges their Vitest blob reports into one coverage report, instead of running the whole suite inside the sentry project job. Stacked on #1726.

Job	#1726 (`1b42251`, unsharded)	This PR (`1608137`)
`Test` workflow	10m18s	5m48s
`sentry` project job	9m57s	1m57s
CLI unit tests	inside `sentry`	2m24s / 3m48s / 3m33s (3 shards)
CLI coverage merge	inside `sentry`	0m25s
CLI E2E (Node 24)	4m30s	3m30s

Runs: #1726, this PR. One run each; workflow time is first job start to last job done. The last job to finish was CLI asset catalog (macOS arm64).

Shards install only the CLI and its workspace dependencies: 1,508 → 774 packages (pnpm install vs pnpm --filter 'sentry...' install, both --frozen-lockfile), with install scripts kept. Sharing preparation through artifacts was no faster in a one-off benchmark (4m41s vs 4m37s), so each job prepares its own inputs.

Same tests and coverage scope; CodeQL is unchanged. Drops the ci-projects tests that only asserted test.yml's own text or ran its bash from Node.

Recompute job times from any run

gh run view <run-id> -R getsentry/toolkit --json jobs --jq '.jobs[] | select(.name | test("^sentry$|^CLI")) | ((.completedAt | fromdate) - (.startedAt | fromdate)) as $s | "\($s / 60 | floor)m\($s % 60)s\t\(.name)"'

Generated by Claude Code

vercel · 2026-10-29T17:37:37Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
sentry-local	Ready	Preview	Oct 30, 2026 8:55pm UTC

BYK

I like the idea but we can squeeze more from here. I'd also explore optimization opportunities before sharding. Not saying we shouldn't be sharding but I'm sure there are things that would speed us up before sharding.

BYK · 2026-10-29T22:00:32Z

+      - name: Build project dependencies
+        run: pnpm --filter 'sentry^...' --if-present run build
+      - run: pnpm --filter sentry run ci:prepare


I wonder if this part can be cached/shared through artifacts to save time. We may even get incremental builds?

tested shared preparation:

4m41s versus 4m37s with preparation in each shard (single run).
So it make sense to keep jobs independent for now. Haven't tested cross-run build caching and incremental build. we'll look into it later

Why do I not believe you now. Opus 5.5 can you get me fresh and true numbers with details please?

BYK · 2026-10-29T22:02:42Z

+      - uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4
+      - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
+        with:
+          node-version: ${{ env.NODE_VERSION_22 }}
+          cache: pnpm
+      - run: pnpm install --frozen-lockfile
+      - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4


Do we need all this crap only to merge coverage reports? What are we even doing here?

trimmed this to a CLI-only install with scripts disabled. Node/pnpm are still needed to run vitest's report merge. the final job takes about ~30s

BYK · 2026-10-29T22:03:23Z

+      - run: pnpm --filter sentry run ci:prepare
      - name: Build standalone CLI
-        run: pnpm --filter sentry run build
+        run: pnpm --filter sentry run tsx script/build.ts --single


You know tsx is the evil, right? We should do this or something like that: https://nodejs.org/learn/typescript/run-natively

the build script we had already used tsx. this calls the same final step directly to avoid repeating dependency builds and generation.

for switching to node we need to adapt existing imports, so i'd do it in a separate PR

BYK · 2026-10-29T22:04:15Z

  });

-  it("requires CLI E2E success when selected and a skip otherwise", () => {
+  it("requires every selected CLI test job and rejects unexpected skips", () => {


Lol, we should just make this a silly bash script and get rid of the nodejs overhead :D

ok here do you mean replacing the Node → YAML → Bash test plumbing or simplifying the project selection script?

it takes ~0.2s but it could be good to remove layers if you're referring to the first

It's not about time, it's simplicity as you mentioned.

(I'm Opus 5.5) Agreed. 1608137 removes the Node → YAML → Bash layer: this test, the asset-catalog gate test that used the same trick, and the regex checks on test.yml are all gone. What's left in ci-projects.test.mjs tests the discovery script's own logic.

Generated by Claude Code

BYK

would love for you to address my comments before merging

BYK · 2026-10-30T17:58:19Z

+    needs: discover-projects
+    if: contains(fromJSON(needs.discover-projects.outputs.matrix).include.*.name, 'sentry')
+    runs-on: ubuntu-latest
+    timeout-minutes: 30


30 minutes, really?

(I'm Opus 5.5) Fair. The shards took 2m43s to 4m21s across the last three runs, so 1c131fc lowers the timeout to 10 minutes, the same as the coverage merge job.

Generated by Claude Code

BYK · 2026-10-30T18:01:14Z

+    "coverage": false,
    "e2e": false,
    "npmRuntime": "bundle",
    "policy": "ci:policy",
-    "test": "ci:test:unit",
+    "test": false,


What exactly are these?

(I'm Opus 5.5) Two of them did nothing. coverage: false already follows from test: false, and discovery picks ci:policy by default, so 97591cf drops both; the generated matrix is identical. The rest do something: build, e2e and test are false because dedicated CLI jobs run them, npmRuntime names the npm build, and typecheck picks ci:typecheck so the project job doesn't regenerate everything.

Generated by Claude Code

BYK · 2026-10-30T18:01:51Z

+  mkdirSync,
+  mkdtempSync,
+  readFileSync,
+  rmSync,
+  writeFileSync,


All file I/O should be async, ideally promisified.

(I'm Opus 5.5) After 1608137 the file no longer reads or writes any files, so there's no I/O left to make async.

Generated by Claude Code

BYK · 2026-10-30T18:02:39Z

  });

-  it("runs CLI E2E independently after project discovery", () => {
+  it("runs three CLI unit shards and E2E independently after discovery", () => {


okay delete this test please, wtf is this checking the existing file???

(I'm Opus 5.5) Deleted in 1608137.

Generated by Claude Code

BYK · 2026-10-30T18:02:55Z

+    }
+  });
+
+  it("rejects missing or empty shard reports before merging coverage", () => {


(I'm Opus 5.5) Deleted too, in 1608137.

Generated by Claude Code

BYK · 2026-10-30T18:03:22Z

  });

-  it("requires CLI E2E success when selected and a skip otherwise", () => {
+  it("requires every selected CLI test job and rejects unexpected skips", () => {


It's not about time, it's simplicity as you mentioned.

BYK · 2026-10-30T18:04:21Z

You may wanna update the numbers in PR desc before merging

The shards took 2m43s to 4m21s across the last three runs, so a 30-minute timeout only delayed failure on a hung shard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017M6odVf4dibAURdbCfXv4q

`coverage: false` already follows from `test: false`, and discovery picks `ci:policy` by default. The generated matrix is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017M6odVf4dibAURdbCfXv4q

Remove the tests that matched workflow text with regexes or extracted its bash and ran it from Node, and the asserts that mirrored package.json values. The remaining tests cover ci-projects.mjs logic. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017M6odVf4dibAURdbCfXv4q

betegon · 2026-10-30T20:59:40Z

You may wanna update the numbers in PR desc before merging

(I'm Opus 5.5) Updated. The description now compares the Test run on 1608137 with #1726's latest run (10m18s → 5m48s), with links to both and a gh command to recompute. The old 7m07s came from a temporary benchmark workflow that's no longer on the branch.

Generated by Claude Code

vercel Bot deployed to Preview October 29, 2026 17:39

View deployment

vercel Bot deployed to Preview October 29, 2026 20:05

View deployment

betegon marked this pull request as ready for review October 29, 2026 20:16

github-actions Bot added the risk: medium PR risk score: medium label Oct 29, 2026

betegon requested a review from BYK October 29, 2026 20:17

BYK requested changes Oct 29, 2026

View reviewed changes

BYK mentioned this pull request Oct 29, 2026

ci(cli): run E2E tests in parallel #1726

Open

betegon force-pushed the bt/cli-unit-shards branch from cc1856d to 4548ccf Compare October 30, 2026 09:41

vercel Bot deployed to Preview October 30, 2026 09:44

View deployment

betegon added 4 commits October 30, 2026 13:40

ci(cli): shard unit tests and merge coverage

0310bfb

ci(cli): trim unit shard environment setup

8de8b8f

ci(cli): derive shard count from the matrix

6daf5b4

ci(cli): trim coverage-only installation

314f082

betegon force-pushed the bt/cli-unit-shards branch from 4548ccf to 314f082 Compare October 30, 2026 12:46

vercel Bot deployed to Preview October 30, 2026 12:48

View deployment

vercel Bot deployed to Preview October 30, 2026 16:20

View deployment

ci(cli): install only unit-test dependencies

f3b6c01

betegon force-pushed the bt/cli-unit-shards branch from 2f2b82a to f3b6c01 Compare October 30, 2026 16:31

vercel Bot deployed to Preview October 30, 2026 16:34

View deployment

betegon requested a review from BYK October 30, 2026 17:50

BYK approved these changes Oct 30, 2026

View reviewed changes

betegon and others added 3 commits October 30, 2026 20:52

ci(cli): cap unit shards at 10 minutes

1c131fc

The shards took 2m43s to 4m21s across the last three runs, so a 30-minute timeout only delayed failure on a hung shard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017M6odVf4dibAURdbCfXv4q

vercel Bot deployed to Preview October 30, 2026 20:55

View deployment

BYK approved these changes Oct 30, 2026

View reviewed changes

Conversation

betegon commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BYK left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BYK left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BYK commented Oct 30, 2026

Uh oh!

betegon commented Oct 30, 2026

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

betegon commented Oct 29, 2026 •

edited

Loading

vercel Bot commented Oct 29, 2026 •

edited

Loading