Run the full unit suite in CI instead of Jest's affected-tests mode

[jakebromberg]

Closes #2249.

The defect

The unit-tests job ran jest --changedSince=origin/<base> on PRs. Jest builds that selection from the module dependency graph, so a spec that reads a source file as text (fs.readFileSync) rather than importing it has no edge to that file and is never selected by a change to the file it guards.

The audit (#2249 acceptance criterion 2)

69 specs under tests/unit/ read source as text. 52 of them import nothing but fs and path — zero edges to the code they guard:

Guarded surface	Text-reading specs
shared/database/src/schema.ts	31
shared/authentication/src/auth.definition.ts	5
dev_env/docker-compose.yml, Dockerfile.*, .github/workflows/test.yml	8
apps/backend/services/*.ts, jobs/*/job.ts, scripts/*.mjs	~15

Measured against the real resolver:

--findRelatedTests shared/database/src/schema.ts                  → 250 suites, 3 of 31 schema.* guards
--findRelatedTests shared/authentication/src/auth.definition.ts   →   1 suite,  0 of 5 guards

Volume is not the signal. A change to schema.ts selects 250 suites and still misses 28 of the 31 specs written to guard schema.ts — a big green number is exactly what the failure mode looks like. That is how #2246 merged green while breaking 5 assertions in tests/unit/database/schema.fk-cascades.test.ts, the spec that holds #2239's per-constraint decision in place. The nightly full-suite run was the only backstop, one day late.

Proof the fix closes it (#2249 acceptance criterion 1)

Re-added { onDelete: 'cascade' } to show_djs.show_id — reverting #2250's fix — and ran both commands against that tree:

Command	Result
Old: jest --changedSince=origin/main --coverage --passWithNoTests	250 suites, 4978 tests, all pass — exit 0
New: npm run test:unit:coverage	1 failed suite, exit 1

The failure is the assertion that should have caught it:

● FK cascade/set-null rules in schema.ts › should NOT have onDelete (intentional NO ACTION) › show_djs.show_id → shows.id (#2239)
    Expected substring: not "onDelete"

schema.ts was restored afterwards; it is untouched in this diff.

Why option 1, and what it costs

unit-tests gates no other job — Integration-Tests needs [detect-changes, lint-and-typecheck] — and it runs beside lint-and-typecheck, which is consistently about twice as long:

unit-tests, recent runs	45–91 s
lint-and-typecheck, same runs	113–154 s
Largest observed affected run	252 suites / 5002 tests in 38.3 s
Full suite (this PR), locally	472 suites / 8181 tests in ~32 s

So the change adds roughly +30 s of runner time inside a minute of slack on a job that is not on the critical path, and the repo is public, so those minutes are free. The run still ends when Integration-Tests does, ~3 minutes later.

The issue's options 2 (union in a fixed list) and 3 (give the specs a real import edge) were rejected: both reintroduce a list somebody has to remember to maintain — the same rot that let a second set of five FKs drift underneath BS#1126's hardcoded regression spec, which is why #2239 shipped a general guard instead of another allowlist. Neither buys anything, because there is no time to protect.

Changes

• .github/workflows/test.yml — the job runs npm run test:unit:coverage unconditionally; drops fetch-depth: 0 (existed only to give --changedSince history) and --passWithNoTests (only meaningful under selection). A comment records the measurements so the optimization is not reintroduced as an obvious win.
• tests/unit/scripts/ci-unit-tests-full-suite.test.ts — new guard asserting the job carries no selection flag, does not branch on github.event_name, and that test:unit / test:unit:coverage are unfiltered. Verified it fails (3 assertions) when --changedSince is put back. It is itself a text-reading spec, which is now safe precisely because the full suite always runs.
• docs/testing.md — corrects the CI description, which still described affected-tests mode.

Local verification

format:check clean · lint 0 errors (917 pre-existing warnings) · typecheck clean · full unit suite 472 suites / 8181 tests green.

One unrelated observation worth its own ticket: Install Dependencies ran npm ci on 13 of 13 recent distinct branches, including branches that never touched package-lock.json. The node_modules cache never restores, costing 25–34 s per run — more than the entire optimization this PR removes was saving.

Related

• CI's affected-unit-test selection cannot see tests that read source as text #2249 — this issue
• fix(schema): stop declaring ON DELETE actions the database does not have (BS#2239) #2246 / test(schema): re-home the five FKs #2239 de-declared under the NO ACTION block #2250 / Five foreign keys declare an ON DELETE action the database does not have (drizzle-kit cannot see it) #2239 — the change whose breakage the gap hid, and its fix

[jakebromberg]

CI green on the first run, and the cost prediction held on the runner rather than my laptop:

	Before (largest observed affected run)	This PR
Jest step	38.3 s — 252 suites / 5002 tests	76 s — 472 suites / 8181 tests
unit-tests job total	45–91 s	122 s
lint-and-typecheck, same run	113–154 s	238 s
Slack before the critical path resumes	37–109 s	116 s

Integration-Tests still started immediately after lint-and-typecheck finished, so total run wall-clock is unchanged — unit-tests finished about two minutes before anything was waiting on it.

Dropping fetch-depth: 0 also cut Checkout Code from 5–6 s to 2 s, which claws back a little of the added test time.

[jakebromberg]

Review follow-ups pushed in 1037f6d. CI green.

Job-slicing boundary — confirmed by probing the extractor directly: the slice ended with Integration-Tests' "service containers but harmless" comment, so it really did run past the job. Every job here is introduced by its own two-space-indented # block, which the next-job-key terminator can't see. Now terminates on the first line at two-space indent that isn't part of the job body (every body line is indented four or more), with an it pinning the boundary. Verified in both directions:

• reintroducing --changedSince: still 3 assertions fail
• putting --changedSince and github.event_name == pull_request into the neighbouring job's comment: 9/9 pass, no spurious failure

Describe-time throw — the missing-job check now throws a named Error that points at the workflow path and reminds the reader that branch protection's required-check name has to move on a rename, rather than surfacing as a whole-file load failure.

Stale comment — # Unit tests - runs affected tests only sat directly above the new block saying the opposite; removed. Good catch, that one was mine.

On the out-of-scope note: the posthog-node bump and the lockfile license correction aren't in this PR — gh pr diff 2255 --name-only returns exactly .github/workflows/test.yml, docs/testing.md, and the new spec. Both landed on main in b86530c, which is this branch's base.