test(schema): re-home the five FKs #2239 de-declared under the NO ACTION block - #2250
Merged
Conversation
…ION block main is red. #2246 removed five false `onDelete` declarations from schema.ts but left `schema.fk-cascades.test.ts` asserting they were still there, so the suite fails on main at ddfa944 with 5 failures. CI did not catch it, and the reason generalizes. The unit-tests job runs Jest in affected-tests mode, and this spec reads schema.ts with fs.readFileSync rather than importing it — no edge in Jest's dependency graph, so changing schema.ts never marks the spec as related and it was never selected. #2246's run executed 244 suites / 4917 tests against a full suite of 462 / 8050. Filed as #2249. The five assertions are re-homed rather than deleted. The spec already had a "should NOT have onDelete (intentional NO ACTION)" block, which is now exactly where they belong, so the test keeps its protective value pointing the other way: it pins #2239's decision and fails if someone re-adds a cascade to schema.ts without also moving the deployed constraint. Deleting them would have left the decision unguarded at the unit level. `artist_library_crossreference.library_id` deliberately stays under "cascade" — #2239 did not touch it, and migration 0147 (BS#2112) repaired that constraint to CASCADE in the database, so the declaration is accurate. Adds an `expectNoOnDelete` helper mirroring the existing `expectOnDelete`, since the NO ACTION block was repeating the same four lines per case. Verified: 19/19 in this spec (unchanged count), and the full unit suite at 462 suites / 8050 tests passes, confirming this was the only breakage affected-mode was hiding. Refs #2239, #2246, #2249.
6 tasks
This was referenced Aug 24, 2026
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.
Fixes red
main.What broke
#2246 removed five false
onDeletedeclarations fromschema.ts(#2239) but lefttests/unit/database/schema.fk-cascades.test.tsasserting they were still present. The suite fails onmainatddfa9445with 5 failures.Why CI said green, and why that generalizes
The
unit-testsjob runs Jest in affected-tests mode — its log endsRan all test suites related to changed files. This spec reads the schema as text:There is no
import, so no edge in Jest's dependency graph, so editingschema.tsnever marks this spec as related and it was never selected. #2246's run executed 244 suites / 4917 tests; the full suite is 462 / 8050. Any test that guards a file it does not import sits in the same blind spot — filed separately as #2249, since fixing the selection mechanism is a bigger decision than fixing this spec.The fix re-homes rather than deletes
The spec already had a
should NOT have onDelete (intentional NO ACTION)block, which is precisely where these five belong now. Moving them there keeps the test's protective value, pointed the other way: it pins #2239's decision and fails if someone re-adds a cascade toschema.tswithout also moving the deployed constraint. Deleting the assertions would have left that decision unguarded at the unit level, which is how it drifted in the first place.A comment at the block records why the five are NO ACTION and what to do if one genuinely needs to cascade later (add it in the PR that introduces the delete path, pair it with a migration, update the case here).
artist_library_crossreference.library_iddeliberately stays undercascade— #2239 did not touch it, and migration0147(BS#2112) repaired that constraint toCASCADEin the database, so the declaration is accurate.Also adds an
expectNoOnDeletehelper mirroring the existingexpectOnDelete; the NO ACTION block was repeating the same four lines per case.Verification
Refs #2239, #2246. Related: #2249.