ci: split the quality gate into parallel jobs and cut test import cost - #964
Open
realcodesiman wants to merge 5 commits into
Open
ci: split the quality gate into parallel jobs and cut test import cost#964realcodesiman wants to merge 5 commits into
realcodesiman wants to merge 5 commits into
Conversation
buildkit-cache-dance round-tripped the pnpm store and .next/cache through the GitHub Actions cache every run, but turbo.json excludes .next/cache from build outputs so a remote-cache hit skips next build entirely anyway — the round-trip cost more than the ~40s install it saved. Also disables Next's turbopackFileSystemCacheForBuild since the Docker build starts from a clean layer and never reads it back. Adds a per-leg `runner` matrix field (all still ubuntu-latest) and a 45-minute job timeout so a future larger-runner label is a one-word swap once one is actually provisioned.
The CI quality gate spent 298s with 111/113 turbo tasks already cached. Nearly all of it was one task: builder:test reported 289s wall for 8.68s of actual test execution, the rest being per-file module-graph rebuild. Cause was maxWorkers: 1 on CI in the shared vitest preset, which ran builder's 248 files serially in a single fork. The comment defending it argued turbo already parallelizes at the task level, but turbo does not parallelize *within* a suite — each suite is one `vitest run` process, so wall clock is bounded below by the slowest single suite. Measured: turbo --concurrency=4 over the 4 heaviest suites gave 2m46s, ~= builder alone. Raise maxWorkers to 2 on CI and drop turbo --concurrency to 2, keeping (concurrency x maxWorkers) <= the runner's 4 vCPU so forks never oversubscribe. Builder, through `turbo run test`: 147s -> 76s. VITEST_MAX_WORKERS overrides the preset for experimentation. It needs passThroughEnv because turbo runs in strict env mode and would otherwise strip it on the very path CI uses; passThroughEnv rather than env since worker count changes timing, not results, and must stay out of the hash. Also cache tsconfig.tsbuildinfo for check-types. incremental: true was inert in CI because the task declared no outputs, so every miss was a cold tsc (~30s for builder). Only apps/builder and packages/ui emit a buildinfo, so the outputs key lives in their own turbo.json — declaring it at the root makes turbo warn for the other 56 check-types tasks. Bump pnpm/action-setup v4 -> v6.0.10 in both workflows to clear the Node 20 deprecation annotation.
realcodesiman
changed the title
The PR gate ran as one 10m17s job. Three structural problems, not tuning: - check-types was serialized 13 deep by `dependsOn: ["^check-types"]`, but nothing here sets `composite`/`references` and no package ships a `dist` (every `exports` resolves to `./src/*.ts`), so each `tsc --noEmit` reparses its dependencies' source and consumes nothing a prior task emitted. The edge conveyed no data. Same for `^lint`: Biome lints files independently, and only builder defines a real lint script — the other 57 tasks were <NONEXISTENT> placeholders turbo still scheduled behind the chain. - MSW booted in every test file (~7MB graph plus three hooks) while only 24 files across 8 workspaces mock HTTP — none of builder's 248. It is now opt-in via `mswSetupFiles`. - Import, not execution, dominated the suites (builder: import 419.94s vs tests 16.92s), so enable `deps.optimizer.ssr` and move to the `threads` pool with 4 workers on CI. Types, lint and tests now run as three jobs on their own runners, so wall clock is the slowest phase rather than their sum. This also closes a gap: the root `pnpm lint` (`ultracite check`, repo-wide) was never run in CI — `turbo run lint` only ever reached builder's own script. Isolation stays ON. `--no-isolate` is faster (~30s on builder) but breaks the suite: `vi.mock` registers per module registry, so files sharing a worker overwrite each other's mocks — 18 files failed on threads, 2 on forks, with the failing set changing between runs, though each passes alone. That is pre-existing cross-file coupling and needs its own change. Measured on builder (248 files, CI env, cold): forks/2w 71.9s (previous setting), threads/2w 66.6s, threads/4w 38.4s, all 248 green. Full 56-suite run 98.4s. Note the first CI run after this is a full cache miss: dependency hashes feed each task hash, so dropping the edges invalidates all 56 check-types entries. Judge the win on the second run.
realcodesiman
changed the title
Scope workflow tokens to read-only where jobs never write, split dependabot's docker ecosystem per-app (root has no Dockerfile so the directory glob never tracked base images), drop the pr-labeler issues:write scope now that labels are pre-bootstrapped, and remove duplicate lint/type-check runs between ci.yml and release.yml so each check runs once per relevant trigger.
getRedisConnection() only skipped lazyConnect for next build (NEXT_PHASE=phase-production-build), so importing any queue barrel under vitest still dialed the non-routable REDIS_URL from setup-env and retried forever. Add isNoRedisEnv() to also treat VITEST=true as a no-dial environment and use it everywhere the build-phase check was duplicated across queue barrels.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pnpm lint(ultracite check, repo-wide) was never run in CI.turbo run lintonly ever reached builder's own script — the other 57 tasks are<NONEXISTENT>placeholders.Changes
Split the gate into three parallel jobs (
.github/workflows/ci.yml) — types, lint, and tests each get a full 4-vCPU runner, so wall clock is the slowest phase instead of their sum. Each picks its own concurrency:tscis single-threaded and CPU-bound, vitest parallelizes internally.Dropped meaningless
dependsOnedges (turbo.json,apps/builder/turbo.json,packages/ui/turbo.json):^check-typesserialized 56 tasks 13 deep. Nothing here setscomposite/referencesand no package ships adist(everyexportsresolves to./src/*.ts), so eachtsc --noEmitreparses its dependencies' source and consumes nothing a prior task emitted. Chain is now depth 1.^lint— Biome lints files independently; there was no ordering to preserve.outputs: ["tsconfig.tsbuildinfo"]is kept: it governs caching, not ordering.Cut per-file import cost (
packages/vitest-config/src/node.ts+ 8 consumer configs). Import, not execution, dominated the suites — builder measuredimport 419.94svstests 16.92s:mswSetupFiles. It was booting a ~7MB graph plus three lifecycle hooks in every test file, while only 24 files across 8 workspaces mock HTTP — none of builder's 248.deps.optimizer.ssrand moved to thethreadspool at 4 workers on CI.Measurements
Builder (248 files, CI env, cold) — the critical path:
--no-isolate, 2wFull 56-suite run: 98.4s. Builder
setup41.45s → 1.19s.Notes for the reviewer
Isolation stays ON, deliberately.
--no-isolateis the fastest option but breaks the suite:vi.mockregisters per module registry, so files sharing a worker overwrite each other's mocks. 18 files failed on threads, 2 on forks, and the failing set changed between runs — yet each file passes when run alone. That is pre-existing cross-file coupling; decoupling it belongs in its own change. Documented in the config so it isn't re-attempted blind.The first CI run after this will be a full cache miss. Dependency hashes feed each task hash, so dropping the edges invalidates all 56 check-types entries. Judge the win on the second run.
Test plan
pnpm lint— clean, 5246 files (this is the repo-wide check that previously never ran in CI)pnpm turbo run check-types --force— 56/56, cold cachepnpm turbo run test --force— 56/56 suites green