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.

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading