CI's affected-unit-test selection cannot see tests that read source as text

[jakebromberg]

The unit-tests CI job runs Jest in affected-tests mode ("Run affected unit tests" → Ran all test suites related to changed files). Any test that reads a source file with fs.readFileSync instead of importing it has no edge in Jest's dependency graph, so changing that source file never marks the test as related and the test silently does not run on the PR that breaks it.

Evidence

PR #2246 changed shared/database/src/schema.ts and reported unit-tests: success. It had in fact broken 5 assertions in tests/unit/database/schema.fk-cascades.test.ts, which opens with:

const schemaSource = fs.readFileSync(path.resolve(__dirname, "../../../shared/database/src/schema.ts"), "utf-8");

No import, no edge, never selected. The breakage reached main green and was found only by running the full suite locally.

The scale of the gap in that run: CI executed 244 suites / 4917 tests; the full suite is 462 suites / 8050 tests. Roughly 47% of suites were not considered, and the selection is content-blind in exactly the cases where a test guards a file it does not import.

Other tests with the same shape

Worth auditing — every test that asserts on source text rather than behaviour is in this blind spot. Grep for readFileSync under tests/unit/; schema.fk-cascades.test.ts is the one that already bit us, but it is unlikely to be alone. Doc-hygiene and schema-shape tests are the usual suspects.

End state

A PR that breaks a text-reading test fails CI on that PR.

Options (not a decision — pick one deliberately)

1. Run the full unit suite on PRs. Simplest and strictly correct. The full suite is ~31 s locally, so the affected-tests optimization may be buying very little; measure before assuming it is load-bearing.
2. Keep affected-mode but always union in a fixed list of text-reading suites.
3. Give the text-reading tests a real import edge so Jest can see them (e.g. import the module and read __filename-adjacent source, or move the assertion into a lint rule).

Option 1 is probably right if the timing holds up, because 2 and 3 both reintroduce a list somebody has to remember to maintain — the same rot that made #2239 a general guard rather than another allowlist.

Acceptance criteria

1. Reverting the fix in the companion PR (re-adding onDelete to any of Five foreign keys declare an ON DELETE action the database does not have (drizzle-kit cannot see it) #2239's five FKs in schema.ts) makes the unit-tests job fail.
2. The audit above is done and any other text-reading suites are covered by whatever mechanism is chosen.

Related

• Five foreign keys declare an ON DELETE action the database does not have (drizzle-kit cannot see it) #2239 / fix(schema): stop declaring ON DELETE actions the database does not have (BS#2239) #2246 — the change whose breakage this gap hid.
• The nightly full-suite run is the only thing that would have caught it, one day late.

[jakebromberg]

Fix in #2255 — option 1 (run the full suite), with both acceptance criteria demonstrated.

The audit (AC 2): the blind spot is wider than this issue guessed. 69 specs under tests/unit/ read source as text; 52 import nothing but fs and path. Measured against the real resolver, --findRelatedTests shared/database/src/schema.ts selects 250 suites and only 3 of the 31 tests/unit/database/schema.* specs written to guard that file. shared/authentication/src/auth.definition.ts selects 1 suite and none of its 5 guards.

AC 1: re-added onDelete: 'cascade' to show_djs.show_id (reverting #2250) and ran both commands — the old --changedSince invocation passes 250 suites at exit 0, the full suite exits 1 on the show_djs.show_id → shows.id (#2239) assertion.

The timing question this issue left open: the optimization was buying nothing. unit-tests gates no other job (Integration-Tests needs [detect-changes, lint-and-typecheck]) and runs beside lint-and-typecheck, which is consistently ~2x longer (45–91 s vs 113–154 s). Full suite is 472 suites / 8181 tests in ~32 s, so roughly +30 s inside a minute of existing slack — and the repo is public, so the minutes are free.

Options 2 and 3 were rejected for the reason this issue anticipated: both reintroduce a maintained list, the same rot that let a second set of five FKs drift under BS#1126's hardcoded spec.