test(npm): npm_virtual_repository_upstreams shape 2/3 (S31 plan: 2/19)

Stacked MRs

Part	MR	Scope	Reviewable LOC	Target
1/3	!2066 (merged)	schema, generated companions, and the delete-path claims	1,045	main (merged)
2/3	this MR	the shape and constraint suite	1,143	main
3/3	!2068 (merged)	the absences and the rows they admit	1,077	dmeshcharakou/s31-npm-virtual-step-2b

Part 1/3 has merged, squashed onto main as e739e728b, which is why this MR targets main rather than dmeshcharakou/s31-npm-virtual-step-2a. Merge 2/3 before 3/3. That order is forced, not a preference: part 2/3 defines the fifteen fixtures part 3/3 consumes, among them seedNPMVirtualUpstreamTree, npmVirtualUpstreamsInsert and assertDeferredUniqueViolation, so part 3/3 does not compile without it. Nothing flows the other way. Part 3/3 still targets this branch, and because this branch was rebased to drop part 1/3's now-squashed commit, part 3/3 needs git rebase --onto <this branch's new tip> 7b4aa0abb once this merges rather than a plain rebase onto main.

Total 3,265 reviewable LOC, split as evenly as the atomic units allow (1,045 / 1,143 / 1,077, within about 5% of equal thirds). The migration is one file and cannot land in halves, and neither test suite can be cut without separating the fixtures from the tests that use them, so no part clears the 500-line ceiling and each states its own justification below.

All three figures count additions plus deletions, and part 1/3's excludes the generated companions it regenerates: migrations/structure.sql and the three internal/datastore/jet/** files, 2,060 lines between them, which is what takes its diff from 3,105 to 1,045. Re-derived rather than copied: every row of this table was stale, part 1/3 at the 917 an earlier revision recorded and part 3/3 at the 962 it was opened with. Measured 1/3 at e739e728b, 2/3 at b71613a9a, 3/3 at bd25110a6.

---

The suite that asserts the shape part 1/3 declares: the partitioned parent and its 64 partitions, the column catalog, both unique keys and the reverse-lookup index with their ordered key columns, the position CHECK, the deferrable constraint's condeferrable and condeferred flags, that it is the only deferrable constraint on the table, every identifier against the 63-character limit, index inheritance on every partition, and hash routing on namespace_id.

It also carries the fixtures the absences suite in part 3/3 consumes, which is why it lands first of the two: that file uses fifteen identifiers defined here, among them seedNPMVirtualUpstreamTree, npmVirtualUpstreamsInsert and assertDeferredUniqueViolation. Nothing here depends on anything there.

Four delete-behavior tests sit here rather than with the absences, because each is about a key's declared action rather than about a row shape the schema admits:

• CascadesOnVirtualRepositoryDelete and TwoLevelCascadeFromRepositoriesDelete, the two links of the cascade, the second deliberately using a different repositories row as the upstream so the delete reaches the association only through the virtual child.
• ListedUpstreamDeleteIsRefused, the 23503 on deleting an upstream some other virtual repository lists, with the referencing row asserted to survive by id rather than merely by count.
• SelfReferencingUpstreamDeleteIsRefused, which settles the one row the cascade and the NO ACTION key both reach. Neither of the other two reached it, and the outcome was asserted nowhere before this: the key wins, so the repository is undeletable while being empty and the reap finalizer retries it forever. internal/datastore/repositories.md named this test as the thing that would pin the wedge, and now names it as the thing that does.

Both delete-refusal tests also pin which key raised the 23503, through assertRefusedByTheUpstreamKey. See the review note below for why that is an assertion on PgError.TableName and not on the constraint name.

Three static tests parse the migration text rather than the database: the Down's lock budget, that the Down reverses every object the Up creates, and that the Up is replayable, meaning every CREATE carries IF NOT EXISTS, the one ADD CONSTRAINT that cannot is preceded by a guarded DROP that sorts ahead of it, and the Up section carries -- +goose NO TRANSACTION, which is the premise the other two rest on.

Reviewable LOC 1,143 counting additions plus deletions, all but 5 of it one test file. Splitting it would have to cut a suite in half and would strand the fixtures away from the tests that define them. The 5 lines outside it are in internal/datastore/repositories.md, which now points at the test by name. Measured at b71613a9.

This branch does not touch the migration, and an earlier revision of it did. Part 1/3 committed 20260828102654_create_npm_virtual_repository_upstreams.sql and it is now on main, so lint:migration-immutability refuses any change to it from here, and an Allow-Migration-Edit trailer would spend a deliberate override on a comment. The sidecar is the unfrozen home for a test name that can move. The file here is byte-identical to main, so it carries part 1/3's own later correction to that comment: cd24fa120 replaced the forward reference with the trigger-level explanation of why the NO ACTION check wins, and the corrected text names no test at all.

Part 1/3 merged as e739e728b; nothing in this MR is still waiting on it.

Review notes

• e2e catalog: no scenario is added or affected; Step 19 owns the catalog.
• Integration-tagged lint: run with --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false. It reports three unused findings, one per part-3 fixture: seedNPMVirtualUpstreamPair, upstreamPositionOf and assertDeferredUniqueViolation have no caller until !2068 (merged) supplies them. They are accepted for the life of the stack rather than suppressed, because a //nolint:unused added here would still be here after !2068 (merged) lands. Note that CI cannot see any of this: .golangci.yaml sets no run.build-tags, so the golangci_lint job never compiles this file. Every //nolint token was measured firing: dropping the paralleltest directive produces two findings, so it is earned, and no dupl or thelper token was carried over from the sibling suites where those do fire.
• Which key fires is asserted; over what columns is not. RejectsAbsentVirtualRepository and RejectsAbsentUpstreamRepository pin PgError.ConstraintName to the declared key, and the two delete-refusal tests pin PgError.TableName to npm_virtual_repository_upstreams, which declares exactly one key into repositories. RejectsAbsentNamespace is deliberately left unpinned, because all three ids are absent there and which key raises it is not determined. What none of them prove is a key's columns: RejectsAbsentUpstreamRepository passes a random uuid, so it would still pass against a single-column REFERENCES repositories (id), which is the shape the migration calls out as the tenancy hole. !2068 (merged)'s ForeignKeyDeleteActions and RejectsCrossNamespaceUpstream are what close that, so read "the shape suite" as excluding the keys' columns until part 3/3 lands.
• Why the delete-refusal tests assert TableName rather than the constraint name. What PostgreSQL reports depends on the statement's direction, and test:integration runs the PG_VERSION: ["16", "17", "18"] matrix, so an assertion has to hold on all three. Measured on postgres:{16,17,18}-alpine with the CI service options, at server_version 16.15, 17.11 and 18.6: an insert into the partitioned table reports the declared constraint name on all three, while a delete from the referenced repositories table reports the partition clone — a wholly auto-generated name truncated to the identifier limit on 16 and 17 (npm_virtual_repository_upstr_upstream_repository_id_name_fkey61, and ..._names_fkey2 for another partition, so the truncation point itself moves), and the declared name plus a partition number on 18 (fk_nvru_upstream_repository_id_repositories_44). So equality against the declared name fails on every version for a delete, and a prefix match fails on 16 and 17. TableName is the referencing relation, unsuffixed and unpartitioned on all three. The survival counts those two tests already carried do not substitute for this: a 23503 aborts the whole statement whichever key raised it, so the row survives either way and the count is entailed by the delete having failed rather than by which key failed it.
• Every assertion added here was mutation-tested per version. Aiming the two constraint-name pins at npmVirtualUpstreamsNamespaceFK fails both, and aiming assertRefusedByTheUpstreamKey at npm_virtual_repositories fails both delete tests, on 16, 17 and 18 alike; unmutated, the full TestNPMVirtualUpstreams suite passes on all three.