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

Stacked MRs

Part MR Scope Reviewable LOC Target
1/3 !2066 (merged) (merged) schema, generated companions, and the delete-path claims 917 main
2/3 !2067 (merged) (merged) the shape and constraint suite 1,116 main
3/3 this MR the absences and the rows they admit 1,081 main

All three parts merge in order, and 1/3 and 2/3 are in. The order was 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 flowed the other way. Both merged on 2026-08-28, each retargeted to main when its parent branch was deleted, and this MR now targets main too.

The 2/3 figure is measured at 7b4aa0abb, being that branch's own two commits, which is what a reviewer read while it was open. The 3/3 figure is measured at 5931abb0f, this MR's head after its branch was rebased onto main and the review fixes landed on top: with 1/3 and 2/3 both merged, the three-dot diff against main is this part's single test file and nothing else, so the figure is now the whole diff rather than a subset of it. The 1/3 figure is carried from when it was open and is not re-derived here.

Total 3,114 reviewable LOC across the three parts, split as evenly as the atomic units allow, with the widest part about 12% off an equal third. 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.


The absences the schema deliberately leaves open, and every row shape they admit. One CHECK and no trigger is the whole of the narrowing, so this suite pins both halves first, then asserts each shape the pair lets through.

The absences: the columns a transcription from npm_remote_repositories would have brought along, the CHECK count on the parent and its inherited clone on all 64 partitions, and the trigger count. That last assertion names every shape below, because a BEFORE INSERT trigger narrowing any of them passes every catalog test in this file and would be caught by nothing else.

The rejections, each constructing a row that only one constraint refuses: a duplicate upstream inside one list, a duplicate position, a negative position, an upstream in another namespace, and a virtual parent in another namespace. The last two are the tenancy control, and each builds a row where every id exists and only the pairing is wrong, which is the exact row a single-column foreign key would admit. The reorder pair asserts the deferrable key from both sides: one transaction commits and the swap is read back off the rows, two autocommit statements fail.

One refusal is not a row at all. The duplicate-upstream message states that an immediate unique is the only kind that can arbitrate ON CONFLICT, and the migration repeats it twice as the reason a position write is UPDATE-then-INSERT rather than an upsert. OnConflictArbiters tests both halves of that claim: naming the deferrable position key as the arbiter is refused with 55000 and a message naming deferrability, because inference skips a non-immediate index and does so while planning, and the same statement naming the immediate no-duplicate-upstream key is accepted. The refusal is what the write path meets first, and nothing else in either suite fails when someone reaches for an upsert.

The admitted shapes, twelve of them: an upstream at every kind, at every format and at every visibility, all as positive hits rather than one value standing for the enumeration; an already soft-deleted upstream and an association under an already soft-deleted virtual repository, both directions, because naming one would read as the whole list; a virtual repository listing its own parent, whose delete part 2/3 proves is refused; a non-contiguous position list, which is what an upstream removal leaves behind; a list longer than ADR-004's cap of 20, which is enforced at write time and by nothing here; one upstream shared by two virtual repositories at the same position; a namespace already out of service at each of its five stamps; any id shape, including a v4 id and an id equal to each of the other three uuid columns; and an id reused across namespaces, which is what the composite primary key admits.

The last two had no test before this and rested on the column catalog alone.

Two of those shapes are admitted by the schema and still have to be refused by the association route, and each test says so rather than stopping at why no constraint expresses it. A self-referencing upstream leaves the repository undeletable while empty, because the upstream key refuses the delete of the repositories row and the reap finalizer retries it forever. A soft-deleted upstream keeps RepositoryReaper.finalize mapping a 23503 to ErrRepositoryNotEmpty on every attempt, so the tombstone is never reaped and its storage is never reclaimed.

Reviewable LOC 1,081, all of it one test file, which cannot land in halves without separating the absence assertions from the shapes they admit.

Depends on part 2/3, which must merge first: this file uses fifteen identifiers defined there, among them seedNPMVirtualUpstreamTree, npmVirtualUpstreamsInsert and assertDeferredUniqueViolation.

Review notes

  • e2e catalog: no scenario is added or affected; Step 19 owns the catalog.
  • Integration-tagged lint: as part 2/3, run with the prescribed flags, no new finding, and every //nolint token measured firing.
  • Enumerated columns are covered as positive hits at every value rather than one value standing for the set: all three kinds including virtual, all four formats, and all three visibility values.
  • Absence assertions name what they find. The column check aggregates the offending names rather than counting them, because the widest group holds thirteen and a bare count would not say which one arrived.
Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading