perf(ci): speed up PR feedback with transpile-only jest and parallel jobs#4374
Merged
Conversation
ts-jest re-ran a full type-check on every suite, which is redundant with the pipeline's type-check and was the dominant test cost. Enable transpile-only via tsconfig isolatedModules, add a project-wide type-check step so spec files stay type-checked (nest build excludes them), and split api-pr.yaml into parallel checks and test jobs. Every test still runs on every PR.
Transpile-only (isolatedModules) emits the emitDecoratorMetadata helpers differently, adding phantom uncovered branches on dependency-injected constructors that red the 100% Frick coverage gate. Compile the coverage run with full type info (tsconfig.coverage.json, isolatedModules: false) so the gate stays exact; the main suite keeps transpile-only.
Split the full suite into 3 disjoint jest shards and move the Frick coverage gate into its own parallel job, so PR feedback is ~max(shard, checks, coverage) instead of one ~9 min test job. Every suite still runs exactly once (shards are disjoint and cover all 300 suites).
Correct the coverageThreshold location (jest.frick.config.js, not package.json) and record why test:frick:cov compiles with full type info (tsconfig.coverage.json) while the main suite runs transpile-only.
Collaborator
Author
Review summaryReviewed in parallel across two lenses — logic/correctness and CONTRIBUTING conformance. Review passes to zero findings: the correctness lens came back clean; the conformance lens surfaced three minor items — a non-imperative PR title, a stale Verified:
CI green across all parallel jobs (Build and checks · Bank Frick coverage · Test shard 1–3). Whole-PR feedback ~4 min (from ~15 min). |
TaprootFreak
marked this pull request as ready for review
July 24, 2026 18:19
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
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.
What & why
PR feedback was dominated by the Jest step (~646 s for ~298 suites in a representative run). This PR makes it substantially faster without removing or deferring a single test — which matters here, because profiling showed the slowness is fixed per-suite overhead spread across the whole suite (incl. the auth / money / accounting suites), not a few skippable tests.
Two levers, both keep every test and weaken no gate:
isolatedModules). ts-jest was re-running a full TypeScript type-check on every suite. That work is redundant with the type-check the pipeline already does — so it's removed from the transform. Measured −42 % of total test time.api-pr.yamlis split into parallel jobs —checks(lint / format / build / type-check / audit),Bank Frick coverage, and a 3-way shardedtestmatrix — so PR feedback ≈ max(job) instead of one serial ~15-min run.Root cause (measured, clean local run, Node 20, 300 suites / 5223 tests, all green)
TestingModulebootstrap + ts-jest transform), not test logic (a 1-test controller suite still took ~39 s; the mean runtime of ≤3-test suites was 17.7 s).Type-safety is preserved (important)
isolatedModulesmakes ts-jest stop type-checking. That would have created a real gap:tsconfig.build.jsonexcludes**/*spec.ts, sonest buildnever type-checks test files — type errors in specs would go uncaught. So this PR adds a single project-widetype-checkstep (tsc --noEmit, base tsconfig, includes specs) to CI. Net effect: the whole project is type-checked once, instead of every spec being type-checked on every suite run. (Verified locally:npm run type-checkis green, ~62 s.)Measured before → after
This PR's own CI run (2-core
ubuntu-latest), all jobs running in parallel:Test (shard 1/3)Test (shard 2/3)Test (shard 3/3)Build and checks(lint / format / build / type-check / audit)Bank Frick coverageWhole PR feedback = max(all jobs) ≈ 247 s (~4.1 min), down from ~15 min — about −73 %. The former single ~10.8-min Jest step is now three disjoint ~2.5-min shards (verified: 100 + 100 + 100 = 300 suites, zero overlap, every suite runs exactly once). The new
type-checkstep is 30 s.Isolating just the transform change (m5me, 16 workers): full-suite wall-clock 413 s → 241 s (−42 %), per-suite work sum 6772 s → 3927 s. Everything else here is parallelism, which converts that work reduction into wall-clock.
Frick coverage under transpile-only
isolatedModulesemits theemitDecoratorMetadatahelpers differently, which adds phantom uncovered branches on dependency-injected constructors and would red the strict 100 % Frick coverage gate. So the coverage run (small, dedicated, not perf-sensitive) compiles with full type info viatsconfig.coverage.json(isolatedModules: false); the gate stays exact at 100 %, while the main suite keeps the fast transpile-only path.What this deliberately does NOT do
slow/full-citag; a review found several of them actually assert authentication (2FA-gated staff replies), authorization (ForbiddenExceptionon foreign notes/templates) and PII-leak prevention (Telegram group-bind guard). Deferring those would have removed real protection from the PR lane, so the tag approach was dropped entirely. This change excludes or defers no spec; every suite still runs on every PR. (One unrelated, pre-existingdescribe.skipin an EVM-delegation spec is left untouched.)CI / branch protection note
The single
Build and testjob is split into two parallel jobs,Build and checksandTest.develop/maincurrently define no required status checks, so this rename breaks nothing. If required checks are added later, require bothBuild and checksandTest.Follow-ups (optional)
@swc/jestis an alternative transform engine (not a missing piece): it could shave the remaining per-suite transform cost with no extra runner, but needs a full-suite verification of NestJS decorator-metadata / DI first. Deliberately not swapped in — ts-jest transpile-only already captured the transform win safely.Addresses the slow-PR-feedback problem from #4365 (approach differs deliberately: no tag, every test kept).