Skip to content

finding(objectql): the ./registry module-mock is 12 hand-copies of one factory, and they have drifted — one copy omits getAllObjects (inert today, the #9002 shape if a path ever reaches it) #10551

Description

@os-zhuang

Filed unassigned by the dev seat measuring #9165, session session_01DdCnBGcHeufjrq7drTD3wt. Measured on origin/main @ 4544be2600. ⛔ No domain:* — triage's field.

This is NOT a claim that any test is vacuous today. The omission below is inert, and that is a measured statement, not an assumption — see "What was measured". It is filed because it is the exact substrate the #9002 instance grew on, and because the drift is invisible by construction.

The measurement

vi.mock('./registry', …) appears in 12 test files, all in packages/objectql/src/. All 12 are hand-copies of one factory — same const instance: any = {…}, same function SchemaRegistry() { return instance; }, same Object.assign(SchemaRegistry, instance), same trailing computeFQN / parseFQN / RESERVED_NAMESPACES.

They have drifted. Eleven declare 12 members; one declares 11:

filemembersdelta
the other 11 (engine.test.ts, engine-filter-tokens, engine-multivalue-normalize, engine-validation-locale, engine-middleware-operation-vocabulary, engine-autonumber-* ×6)12
packages/objectql/src/engine-count-read-filter.test.ts11no getAllObjects

getAllObjects is called at 13 sites in packages/objectql/src non-test source, and several of those calls sit under a catch that recovers — including engine.ts:7049 in announceOpenMigrationGates(), whose catch is parameterless and whose whole body is the comment // An advisory must never be the reason a boot fails. That is the shape that made #9002 silent: a missing member raises a TypeError, a recovering catch absorbs it, and an incomplete double reads as "a registry with nothing in it".

What was measured, and what it says

A recording Proxy was placed on all 12 mock instances (records a get for any member the double does not declare, out-of-band, then returns undefined so behaviour is unchanged), and the whole objectql suite was run:

Test Files 224 passed (224)
Tests 3956 passed (3956)
missing-member accesses recorded: 0

⇒ No registry double in this package is accessed for a member it does not declare, on any path its suite drives. engine-count-read-filter.test.ts's omission is latent, not live.

Positive control, in the same 224-file configuration (⛔ a zero-hit without one proves nothing): deleting getAllObjects: vi.fn(() => []) from engine-middleware-operation-vocabulary.test.ts — reintroducing #9002 exactly — makes the same run print

OS_PROBE_MISSING engine-middleware-operation-vocabulary.test.ts::getAllObjects
Test Files 1 failed | 223 passed (224)

so the instrumentation does reach the real path and does name the member. Restored byte-clean afterwards (git diff --exit-code = 0).

Why file it, given it is inert

  1. The drift is invisible. Nothing compares the 12 copies. Whether a given copy is complete is decided by which paths its own suite happens to drive — so an omission is inert until an unrelated test is added, and then it is silent rather than red.
  2. The finding(objectql): the delete-cascade path's two registry-read swallows are the #8895 shape one layer up — catch → return disables every referential guard at once, silently #9002 lesson did not propagate.engine-middleware-operation-vocabulary.test.ts carries a 12-line comment explaining the finding(objectql): the delete-cascade path's two registry-read swallows are the #8895 shape one layer up — catch → return disables every referential guard at once, silently #9002 swallow and why getAllObjects: vi.fn(() => []) is the truthful body. That comment sits in one of the 12 copies. The other 11 never learned it, because there is no shared factory to learn it in.
  3. It is the cheapest thing that could remove a whole defect class from this package: one shared factory, 12 call sites.

Candidate shapes, none costed

  1. Collapse the 12 into one shared factory in a test-support module (vi.mock('./registry', async () => …) accepts an async factory, so the import is legal despite hoisting). Removes the drift by construction.
  2. Add the missing getAllObjects: vi.fn(() => []) to the one outlier only. One line, and it leaves the drift mechanism in place.
  3. Make the doubles self-checking — the recording-Proxy shape above, kept rather than thrown away, with an afterEach failing the run on the record. This is the prior art in scripts/check-cross-repo-closer-outcome.mjs (unstubbed(), cross-repo-issue-closer.yml: an already-closed foreign issue is skipped whole, so it never gets the backlink the workflow exists to leave #9643), and its measured first-run cost in this package is zero reds. ⛔ Deliberately not recommended here: choosing it is a verification-strategy act that belongs with A partial test double is only ever discovered by deleting the catch that hides it — twice this shift. check:engine-double-contract pins engine doubles; nothing pins registry doubles #9165's disposition, not with this file.

Reproduce

git worktree add ../os-9165 origin/main && cd ../os-9165 && pnpm install
pnpm --filter '@objectstack/objectql^...' build
# member-set diff across the 12: AST-read `const instance` from each file that greps `vi.mock('./registry'`
pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 # 224 files green, both with and without the outlier's omission

Related

#9165 (the card this was measured under) · #9002 / PR #9163 (the instance this shape produced) · #8896 / PR #9067 (the same defect in a metadata-protocol engine double) · #9285 (three getAllObjects?.() ?? [] seams in plugin.ts)

Metadata

Metadata

Assignees

No one assigned

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions