test(npm): npm_virtual_upstream_rules shape 2/3 (S31 plan: 3/19)
Stacked MRs
| Part | MR | Scope | Reviewable LOC | Target |
|---|---|---|---|---|
| 1/3 | !2099 (merged) (merged) | schema, generated companions, and the cascade guard | 709 | main |
| 2/3 | this MR | the catalog and DDL suite, and the sidecar's first two sections | 1,021 | main |
| 3/3 | !2101 (merged) | the absences and the rows they admit | 1,208 | dmeshcharakou/s31-npm-virtual-step-3b |
Merge 1/3, then 2/3, then 3/3. The order is forced, not a preference. Part
2/3 asserts a table part 1/3 creates, and it also owns the fixtures part 3/3
consumes (npmVirtualRule, seedNPMVirtualRule, seedNPMVirtualRuleTree,
npmVirtualRulesInsert and the constraint-name constants), so part 3/3 does not
compile without it. Nothing flows the other way.
Part 1/3 additionally cannot be split from its cascade-guard companion: the
moment its migration lands, TestRepositoryStore_DeleteFKAssumption_SchemaGuard
fails until wantCascadeTargets names the new table, so separating them would
leave main red between merges.
Each part was compiled with -tags=integration and run on its own branch, not
only at the tip: 0, 19 and 39 TestNPMVirtualRules* functions respectively, all
passing, against PostgreSQL 16 with the full migration chain applied.
Total 2,938 reviewable LOC, split as evenly as the atomic units allow (709 / 1,021 / 1,208). Part 3/3 carries the overshoot and its "Diff size" section says why shrinking it would cost more than the imbalance does.
What this does
The catalog and DDL half of the suite for the table part 1/3 creates:
partitioned-table shape, the six columns and their types, the upstream index's
columns and uniqueness, the three CHECK expressions, that nothing is deferrable,
that every constraint name fits the 63-character identifier limit, that all 64
partitions inherit the parent's indexes, and hash routing by namespace_id. The
delete-path arms cover the absent-parent refusals and the one, two and three
level cascades.
It also reads the migration file itself: the Down lock budget, that the Down
reverses every Up object, that the Up carries no ADD CONSTRAINT, and that the
Up is replayable.
Why the order is forced
This file owns the fixtures part 3/3 consumes, among them npmVirtualRule,
seedNPMVirtualRule, seedNPMVirtualRuleTree, npmVirtualRulesInsert and the
constraint-name constants, so part 3/3 does not compile without it. Nothing
flows the other way: this file uses no declaration from the row-shape suite,
which is why it stands alone here. Verified by compiling the package with
-tags=integration on this branch with the row-shape file absent.
The sidecar starts here
npm_virtual_upstream_rules_schema_notes.md arrives in this MR because this is
its first caller. It carries the intro plus the two sections whose reasoning
this file's assertions rest on: the nvur identifier arithmetic and why nothing
on the table is deferrable.
The three remaining sections name row-shape tests and land with them in part
3/3. Keeping them here would forward-reference that part, which is what
AGENTS.md guardrail 19 rules out. The intro likewise names only the suite that
exists at this point, and part 3/3 widens it.
Two things this MR deliberately does not assert
An accepted row at every discriminator slot. This suite inserts
rule_type=1 with target_field=0, and reaches
npmVirtualRuleTypeAllow and npmVirtualRuleTargetScope only inside an insert
it requires to fail. The accepted-row coverage for every slot the two CHECKs
admit belongs to the range tests part 3/3 adds, which insert one at each. That
is a coverage split, not an omission, and the enumerated-column divider in this
file says so. While part 3/3 is unmerged, main therefore holds a suite that
pins how the two CHECK expressions render without proving the schema accepts
every value they list.
A fourth CHECK. TestNPMVirtualRulesSchema_CheckShapes asserts the table
declares exactly three, which is what part 1/3's migration declares. ADR-007
requires a UUIDv7 version check on every uuid id column with no server-side
default, and this table's id is one; while the table carries no such check the
count here is three, and when the retrofit tracked in
#983 (closed)
reaches this table the count becomes four. The assertion message says as much,
so the 3 does not read as the settled shape. npm_virtual_upstream_rules is
not in !2106 (merged)'s migration, because it was created after that MR's table list was
written, so the retrofit for this table is still owed.
Testing
Verified against PostgreSQL 16 on top of part 1/3: 19 TestNPMVirtualRules*
functions in the package, all passing, re-run after the review-fix commit.
golangci-lint with --build-tags=integration and the uncapped flags reports
no finding in either file this MR adds. The package as a whole reports 130, the
same total as before the fixes and all of them pre-existing.
assertNoMatchInUp was checked in the failing direction as well as the passing
one, by pointing it at a pattern the Up does carry: it names the matched
statement, its line in the comment-stripped Up section, and the rule, rather
than printing the whole 12 KB section the way assert.NotRegexpf did.
Diff size
1,021 reviewable LOC, past the 500 docs/dev/development-model.md asks a split or a justification for. This MR is one third of that split. The suite is one file and cannot land in halves without separating fixtures from the assertions that use them: schema suite 908, sidecar 113.