From fb79cfb249e6879fb3488cfd5975a115e2f89c66 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 04:23:02 +0000 Subject: [PATCH] test(objectql): collapse the twelve `./registry` module-mocks into one factory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `vi.mock('./registry', …)` was hand-copied into twelve test files in `packages/objectql/src`, and the copies had drifted: eleven declared twelve members, `engine-count-read-filter.test.ts` declared eleven and omitted `getAllObjects` — the #9002 shape, inert only because no path its suite drives reaches one of the thirteen `getAllObjects` call sites in this package. The deciding evidence for collapsing rather than adding the missing line is the two lesson comments themselves: the #9002 explanation lived in exactly one copy and the #9154 explanation in nine others, and neither could reach the rest because there was no shared factory to write them in. Both now live in `registry-module-mock.ts`, which every call site inherits. `engine.test.ts` keeps its stateful in-memory registry as per-member overrides over the shared member set, so it too fails when the shared factory loses a member. The `async` factory form is what makes importing the shared module legal under `vi.mock` hoisting. Test infrastructure only — no production source is touched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --- .../engine-autonumber-default-format.test.ts | 41 +--- .../src/engine-autonumber-defer.test.ts | 41 +--- .../src/engine-autonumber-resync.test.ts | 41 +--- .../src/engine-autonumber-seed-outage.test.ts | 41 +--- .../src/engine-autonumber-seed-scan.test.ts | 41 +--- .../src/engine-autonumber-seed-suffix.test.ts | 41 +--- .../src/engine-count-read-filter.test.ts | 30 +-- .../objectql/src/engine-filter-tokens.test.ts | 41 +--- ...ne-middleware-operation-vocabulary.test.ts | 42 +--- .../src/engine-multivalue-normalize.test.ts | 41 +--- .../src/engine-validation-locale.test.ts | 41 +--- packages/objectql/src/engine.test.ts | 84 ++++---- packages/objectql/src/registry-module-mock.ts | 182 ++++++++++++++++++ 13 files changed, 288 insertions(+), 419 deletions(-) create mode 100644 packages/objectql/src/registry-module-mock.ts diff --git a/packages/objectql/src/engine-autonumber-default-format.test.ts b/packages/objectql/src/engine-autonumber-default-format.test.ts index 5e224ab7ab..1674b2601d 100644 --- a/packages/objectql/src/engine-autonumber-default-format.test.ts +++ b/packages/objectql/src/engine-autonumber-default-format.test.ts @@ -46,41 +46,12 @@ import { ObjectQL } from './engine'; import { SchemaRegistry } from './registry'; import type { IDataDriver } from '@objectstack/spec/contracts'; -vi.mock('./registry', () => { - const instance: any = { - getObject: vi.fn(), - resolveObject: vi.fn((n: string) => instance.getObject(n)), - // [#9154] This double used to OMIT `getAllObjects`, and every test here - // passed anyway: the engine's roll-up summary index read it as - // `getAllObjects?.() ?? []`, so a double that does not model the method - // was indistinguishable from a registry with nothing in it — the write - // path silently skipped the insert-time roll-up seed (#5749) and the - // post-write recompute. With the optional call gone the omission is a - // hard `TypeError`, which is the point: the double now has to model the - // method the engine actually calls. Empty is the truthful body for THIS - // suite — it declares no `summary` field, so the roll-up index over it is - // empty either way, and now it says so instead of the engine inventing it. - getAllObjects: vi.fn(() => []), - registerObject: vi.fn(), - getObjectOwner: vi.fn(), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn(), - reset: vi.fn(), - metadata: { get: vi.fn(() => new Map()) }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, - computeFQN: (_ns: string | undefined, name: string) => name, - parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; +vi.mock('./registry', async () => { + // [#10551] The one shared factory — see `registry-module-mock.ts` for the + // member set, the #9002 / #9154 lessons it carries, and why the async factory + // form is what makes this import legal under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); }); /** Date tokens render from the wall clock, so the clock is pinned. Only `Date`. */ diff --git a/packages/objectql/src/engine-autonumber-defer.test.ts b/packages/objectql/src/engine-autonumber-defer.test.ts index 588dbbf478..b307fcf615 100644 --- a/packages/objectql/src/engine-autonumber-defer.test.ts +++ b/packages/objectql/src/engine-autonumber-defer.test.ts @@ -18,41 +18,12 @@ import type { IDataDriver } from '@objectstack/spec/contracts'; * A `required` autonumber must pass insert-validation in BOTH cases (the value * is runtime-owned, assigned after validation in the native-driver case). */ -vi.mock('./registry', () => { - const instance: any = { - getObject: vi.fn(), - resolveObject: vi.fn((n: string) => instance.getObject(n)), - // [#9154] This double used to OMIT `getAllObjects`, and every test here - // passed anyway: the engine's roll-up summary index read it as - // `getAllObjects?.() ?? []`, so a double that does not model the method - // was indistinguishable from a registry with nothing in it — the write - // path silently skipped the insert-time roll-up seed (#5749) and the - // post-write recompute. With the optional call gone the omission is a - // hard `TypeError`, which is the point: the double now has to model the - // method the engine actually calls. Empty is the truthful body for THIS - // suite — it declares no `summary` field, so the roll-up index over it is - // empty either way, and now it says so instead of the engine inventing it. - getAllObjects: vi.fn(() => []), - registerObject: vi.fn(), - getObjectOwner: vi.fn(), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn(), - reset: vi.fn(), - metadata: { get: vi.fn(() => new Map()) }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, - computeFQN: (_ns: string | undefined, name: string) => name, - parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; +vi.mock('./registry', async () => { + // [#10551] The one shared factory — see `registry-module-mock.ts` for the + // member set, the #9002 / #9154 lessons it carries, and why the async factory + // form is what makes this import legal under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); }); function makeDriver(supportsAutonumber: boolean): IDataDriver & { created: any[] } { diff --git a/packages/objectql/src/engine-autonumber-resync.test.ts b/packages/objectql/src/engine-autonumber-resync.test.ts index 0df5eac42f..2edd76d097 100644 --- a/packages/objectql/src/engine-autonumber-resync.test.ts +++ b/packages/objectql/src/engine-autonumber-resync.test.ts @@ -91,41 +91,12 @@ import { ObjectQL } from './engine'; import { SchemaRegistry } from './registry'; import type { IDataDriver } from '@objectstack/spec/contracts'; -vi.mock('./registry', () => { - const instance: any = { - getObject: vi.fn(), - resolveObject: vi.fn((n: string) => instance.getObject(n)), - // [#9154] This double used to OMIT `getAllObjects`, and every test here - // passed anyway: the engine's roll-up summary index read it as - // `getAllObjects?.() ?? []`, so a double that does not model the method - // was indistinguishable from a registry with nothing in it — the write - // path silently skipped the insert-time roll-up seed (#5749) and the - // post-write recompute. With the optional call gone the omission is a - // hard `TypeError`, which is the point: the double now has to model the - // method the engine actually calls. Empty is the truthful body for THIS - // suite — it declares no `summary` field, so the roll-up index over it is - // empty either way, and now it says so instead of the engine inventing it. - getAllObjects: vi.fn(() => []), - registerObject: vi.fn(), - getObjectOwner: vi.fn(), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn(), - reset: vi.fn(), - metadata: { get: vi.fn(() => new Map()) }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, - computeFQN: (_ns: string | undefined, name: string) => name, - parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; +vi.mock('./registry', async () => { + // [#10551] The one shared factory — see `registry-module-mock.ts` for the + // member set, the #9002 / #9154 lessons it carries, and why the async factory + // form is what makes this import legal under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); }); /** Date tokens render from the wall clock, so the clock is pinned. */ diff --git a/packages/objectql/src/engine-autonumber-seed-outage.test.ts b/packages/objectql/src/engine-autonumber-seed-outage.test.ts index b4aecfad62..8920dab313 100644 --- a/packages/objectql/src/engine-autonumber-seed-outage.test.ts +++ b/packages/objectql/src/engine-autonumber-seed-outage.test.ts @@ -37,41 +37,12 @@ import { ObjectQL } from './engine'; import { SchemaRegistry } from './registry'; import type { IDataDriver } from '@objectstack/spec/contracts'; -vi.mock('./registry', () => { - const instance: any = { - getObject: vi.fn(), - resolveObject: vi.fn((n: string) => instance.getObject(n)), - // [#9154] This double used to OMIT `getAllObjects`, and every test here - // passed anyway: the engine's roll-up summary index read it as - // `getAllObjects?.() ?? []`, so a double that does not model the method - // was indistinguishable from a registry with nothing in it — the write - // path silently skipped the insert-time roll-up seed (#5749) and the - // post-write recompute. With the optional call gone the omission is a - // hard `TypeError`, which is the point: the double now has to model the - // method the engine actually calls. Empty is the truthful body for THIS - // suite — it declares no `summary` field, so the roll-up index over it is - // empty either way, and now it says so instead of the engine inventing it. - getAllObjects: vi.fn(() => []), - registerObject: vi.fn(), - getObjectOwner: vi.fn(), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn(), - reset: vi.fn(), - metadata: { get: vi.fn(() => new Map()) }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, - computeFQN: (_ns: string | undefined, name: string) => name, - parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; +vi.mock('./registry', async () => { + // [#10551] The one shared factory — see `registry-module-mock.ts` for the + // member set, the #9002 / #9154 lessons it carries, and why the async factory + // form is what makes this import legal under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); }); const DOC_SCHEMA = { diff --git a/packages/objectql/src/engine-autonumber-seed-scan.test.ts b/packages/objectql/src/engine-autonumber-seed-scan.test.ts index 563c366765..f3f403791a 100644 --- a/packages/objectql/src/engine-autonumber-seed-scan.test.ts +++ b/packages/objectql/src/engine-autonumber-seed-scan.test.ts @@ -41,41 +41,12 @@ import { ObjectQL } from './engine'; import { SchemaRegistry } from './registry'; import type { IDataDriver } from '@objectstack/spec/contracts'; -vi.mock('./registry', () => { - const instance: any = { - getObject: vi.fn(), - resolveObject: vi.fn((n: string) => instance.getObject(n)), - // [#9154] This double used to OMIT `getAllObjects`, and every test here - // passed anyway: the engine's roll-up summary index read it as - // `getAllObjects?.() ?? []`, so a double that does not model the method - // was indistinguishable from a registry with nothing in it — the write - // path silently skipped the insert-time roll-up seed (#5749) and the - // post-write recompute. With the optional call gone the omission is a - // hard `TypeError`, which is the point: the double now has to model the - // method the engine actually calls. Empty is the truthful body for THIS - // suite — it declares no `summary` field, so the roll-up index over it is - // empty either way, and now it says so instead of the engine inventing it. - getAllObjects: vi.fn(() => []), - registerObject: vi.fn(), - getObjectOwner: vi.fn(), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn(), - reset: vi.fn(), - metadata: { get: vi.fn(() => new Map()) }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, - computeFQN: (_ns: string | undefined, name: string) => name, - parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; +vi.mock('./registry', async () => { + // [#10551] The one shared factory — see `registry-module-mock.ts` for the + // member set, the #9002 / #9154 lessons it carries, and why the async factory + // form is what makes this import legal under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); }); /** The page size the seeding walk uses — a PAGE, not a cap, since #6249. */ diff --git a/packages/objectql/src/engine-autonumber-seed-suffix.test.ts b/packages/objectql/src/engine-autonumber-seed-suffix.test.ts index a240e95e74..0a01ddee57 100644 --- a/packages/objectql/src/engine-autonumber-seed-suffix.test.ts +++ b/packages/objectql/src/engine-autonumber-seed-suffix.test.ts @@ -38,41 +38,12 @@ import { ObjectQL } from './engine'; import { SchemaRegistry } from './registry'; import type { IDataDriver } from '@objectstack/spec/contracts'; -vi.mock('./registry', () => { - const instance: any = { - getObject: vi.fn(), - resolveObject: vi.fn((n: string) => instance.getObject(n)), - // [#9154] This double used to OMIT `getAllObjects`, and every test here - // passed anyway: the engine's roll-up summary index read it as - // `getAllObjects?.() ?? []`, so a double that does not model the method - // was indistinguishable from a registry with nothing in it — the write - // path silently skipped the insert-time roll-up seed (#5749) and the - // post-write recompute. With the optional call gone the omission is a - // hard `TypeError`, which is the point: the double now has to model the - // method the engine actually calls. Empty is the truthful body for THIS - // suite — it declares no `summary` field, so the roll-up index over it is - // empty either way, and now it says so instead of the engine inventing it. - getAllObjects: vi.fn(() => []), - registerObject: vi.fn(), - getObjectOwner: vi.fn(), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn(), - reset: vi.fn(), - metadata: { get: vi.fn(() => new Map()) }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, - computeFQN: (_ns: string | undefined, name: string) => name, - parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; +vi.mock('./registry', async () => { + // [#10551] The one shared factory — see `registry-module-mock.ts` for the + // member set, the #9002 / #9154 lessons it carries, and why the async factory + // form is what makes this import legal under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); }); /** diff --git a/packages/objectql/src/engine-count-read-filter.test.ts b/packages/objectql/src/engine-count-read-filter.test.ts index 47997d74b9..89dda56854 100644 --- a/packages/objectql/src/engine-count-read-filter.test.ts +++ b/packages/objectql/src/engine-count-read-filter.test.ts @@ -17,30 +17,12 @@ import { SchemaRegistry } from './registry'; * These tests assert on what the DRIVER receives: the middleware's filter * must be present in the ast that reaches driver.count / driver.aggregate. */ -vi.mock('./registry', () => { - const instance: any = { - getObject: vi.fn(), - resolveObject: vi.fn((n: string) => instance.getObject(n)), - registerObject: vi.fn(), - getObjectOwner: vi.fn(), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn(), - reset: vi.fn(), - metadata: { get: vi.fn(() => new Map()) }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, - computeFQN: (_ns: string | undefined, name: string) => name, - parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; +vi.mock('./registry', async () => { + // [#10551] The one shared factory — see `registry-module-mock.ts` for the + // member set, the #9002 / #9154 lessons it carries, and why the async factory + // form is what makes this import legal under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); }); const NOTE_SCHEMA = { diff --git a/packages/objectql/src/engine-filter-tokens.test.ts b/packages/objectql/src/engine-filter-tokens.test.ts index 88b27791b6..f6d5480766 100644 --- a/packages/objectql/src/engine-filter-tokens.test.ts +++ b/packages/objectql/src/engine-filter-tokens.test.ts @@ -17,41 +17,12 @@ import { SchemaRegistry } from './registry'; * These tests assert on what the DRIVER receives: no `{token}` may survive to * the driver AST, and an unresolvable one must throw rather than pass through. */ -vi.mock('./registry', () => { - const instance: any = { - getObject: vi.fn(), - resolveObject: vi.fn((n: string) => instance.getObject(n)), - // [#9154] This double used to OMIT `getAllObjects`, and every test here - // passed anyway: the engine's roll-up summary index read it as - // `getAllObjects?.() ?? []`, so a double that does not model the method - // was indistinguishable from a registry with nothing in it — the write - // path silently skipped the insert-time roll-up seed (#5749) and the - // post-write recompute. With the optional call gone the omission is a - // hard `TypeError`, which is the point: the double now has to model the - // method the engine actually calls. Empty is the truthful body for THIS - // suite — it declares no `summary` field, so the roll-up index over it is - // empty either way, and now it says so instead of the engine inventing it. - getAllObjects: vi.fn(() => []), - registerObject: vi.fn(), - getObjectOwner: vi.fn(), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn(), - reset: vi.fn(), - metadata: { get: vi.fn(() => new Map()) }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, - computeFQN: (_ns: string | undefined, name: string) => name, - parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; +vi.mock('./registry', async () => { + // [#10551] The one shared factory — see `registry-module-mock.ts` for the + // member set, the #9002 / #9154 lessons it carries, and why the async factory + // form is what makes this import legal under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); }); const DEAL_SCHEMA = { diff --git a/packages/objectql/src/engine-middleware-operation-vocabulary.test.ts b/packages/objectql/src/engine-middleware-operation-vocabulary.test.ts index 00852f9ec9..de44f8853d 100644 --- a/packages/objectql/src/engine-middleware-operation-vocabulary.test.ts +++ b/packages/objectql/src/engine-middleware-operation-vocabulary.test.ts @@ -96,42 +96,12 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { ObjectQL } from './engine'; import { SchemaRegistry } from './registry'; -vi.mock('./registry', () => { - const instance: any = { - getObject: vi.fn(), - resolveObject: vi.fn((n: string) => instance.getObject(n)), - // [#9002] This double used to omit `getAllObjects`, and the suite passed - // anyway: `delete()`'s by-id branch reads it twice (`planCascadeAtomicity`, - // then `cascadeDeleteRelations`) and BOTH reads sat behind a `catch` that - // answered "no relations". The swallow absorbed the `TypeError` this - // omission raises just as silently as it would absorb a real read failure, - // so an incomplete double read as a registry with nothing in it. With the - // swallows gone the omission is a hard failure, which is the point — the - // double now has to model the method the engine actually calls. Empty is - // the right body here: this suite pins the middleware operation VOCABULARY - // and registers no relations, so "no object references the deleted one" is - // the truthful answer rather than an invented one. - getAllObjects: vi.fn(() => []), - registerObject: vi.fn(), - getObjectOwner: vi.fn(), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn(), - reset: vi.fn(), - metadata: { get: vi.fn(() => new Map()) }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, - computeFQN: (_ns: string | undefined, name: string) => name, - parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; +vi.mock('./registry', async () => { + // [#10551] The one shared factory — see `registry-module-mock.ts` for the + // member set, the #9002 / #9154 lessons it carries, and why the async factory + // form is what makes this import legal under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); }); // ── the pinned vocabulary ────────────────────────────────────────────────── diff --git a/packages/objectql/src/engine-multivalue-normalize.test.ts b/packages/objectql/src/engine-multivalue-normalize.test.ts index 520824eb0f..db58d4de90 100644 --- a/packages/objectql/src/engine-multivalue-normalize.test.ts +++ b/packages/objectql/src/engine-multivalue-normalize.test.ts @@ -18,41 +18,12 @@ import { SchemaRegistry } from './registry'; * These tests assert on what the DRIVER receives — the actual corruption * point — not just on validator return values. */ -vi.mock('./registry', () => { - const instance: any = { - getObject: vi.fn(), - resolveObject: vi.fn((n: string) => instance.getObject(n)), - // [#9154] This double used to OMIT `getAllObjects`, and every test here - // passed anyway: the engine's roll-up summary index read it as - // `getAllObjects?.() ?? []`, so a double that does not model the method - // was indistinguishable from a registry with nothing in it — the write - // path silently skipped the insert-time roll-up seed (#5749) and the - // post-write recompute. With the optional call gone the omission is a - // hard `TypeError`, which is the point: the double now has to model the - // method the engine actually calls. Empty is the truthful body for THIS - // suite — it declares no `summary` field, so the roll-up index over it is - // empty either way, and now it says so instead of the engine inventing it. - getAllObjects: vi.fn(() => []), - registerObject: vi.fn(), - getObjectOwner: vi.fn(), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn(), - reset: vi.fn(), - metadata: { get: vi.fn(() => new Map()) }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, - computeFQN: (_ns: string | undefined, name: string) => name, - parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; +vi.mock('./registry', async () => { + // [#10551] The one shared factory — see `registry-module-mock.ts` for the + // member set, the #9002 / #9154 lessons it carries, and why the async factory + // form is what makes this import legal under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); }); const PROJECT_SCHEMA = { diff --git a/packages/objectql/src/engine-validation-locale.test.ts b/packages/objectql/src/engine-validation-locale.test.ts index e051ad19ce..1d66fb6975 100644 --- a/packages/objectql/src/engine-validation-locale.test.ts +++ b/packages/objectql/src/engine-validation-locale.test.ts @@ -15,41 +15,12 @@ import { SchemaRegistry } from './registry'; * `ql.update`, for every write shape the engine validates: single insert, batch * insert, single-id update, and multi-row update. */ -vi.mock('./registry', () => { - const instance: any = { - getObject: vi.fn(), - resolveObject: vi.fn((n: string) => instance.getObject(n)), - // [#9154] This double used to OMIT `getAllObjects`, and every test here - // passed anyway: the engine's roll-up summary index read it as - // `getAllObjects?.() ?? []`, so a double that does not model the method - // was indistinguishable from a registry with nothing in it — the write - // path silently skipped the insert-time roll-up seed (#5749) and the - // post-write recompute. With the optional call gone the omission is a - // hard `TypeError`, which is the point: the double now has to model the - // method the engine actually calls. Empty is the truthful body for THIS - // suite — it declares no `summary` field, so the roll-up index over it is - // empty either way, and now it says so instead of the engine inventing it. - getAllObjects: vi.fn(() => []), - registerObject: vi.fn(), - getObjectOwner: vi.fn(), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn(), - reset: vi.fn(), - metadata: { get: vi.fn(() => new Map()) }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, - computeFQN: (_ns: string | undefined, name: string) => name, - parseFQN: (fqn: string) => ({ namespace: undefined, shortName: fqn }), - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; +vi.mock('./registry', async () => { + // [#10551] The one shared factory — see `registry-module-mock.ts` for the + // member set, the #9002 / #9154 lessons it carries, and why the async factory + // form is what makes this import legal under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); }); // The reporting object from the issue: Chinese labels, range-guarded currency. diff --git a/packages/objectql/src/engine.test.ts b/packages/objectql/src/engine.test.ts index 0ee730ca3a..a949690203 100644 --- a/packages/objectql/src/engine.test.ts +++ b/packages/objectql/src/engine.test.ts @@ -14,51 +14,48 @@ import type { IDataDriver } from '@objectstack/spec/contracts'; // 2. Existing tests reach for `SchemaRegistry.getObject` as if it were a // static — we preserve that by also attaching the same mocks to the // class itself. -vi.mock('./registry', () => { +vi.mock('./registry', async () => { + // [#10551] The one shared factory (`registry-module-mock.ts`) owns the member + // set, the #9002 / #9154 lessons and the module envelope; this suite overrides + // the members whose truthful answer here is a REAL in-memory registry rather + // than an empty one. The async factory form is what makes the import legal + // under `vi.mock` hoisting. + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); const mockObjects = new Map(); const mockContributors = new Map(); - const instance: any = { - getObject: vi.fn((name) => mockObjects.get(name)), - resolveObject: vi.fn((name) => mockObjects.get(name)), - getAllObjects: vi.fn(() => [...mockObjects.values()]), - registerObject: vi.fn((obj, packageId, namespace, ownership, priority) => { - const fqn = namespace ? `${namespace}__${obj.name}` : obj.name; - mockObjects.set(fqn, { ...obj, name: fqn }); - if (!mockContributors.has(fqn)) { - mockContributors.set(fqn, []); - } - const contributors = mockContributors.get(fqn); - contributors.push({ packageId, namespace, ownership, priority, definition: obj }); - return fqn; - }), - getObjectOwner: vi.fn((fqn) => { - const contributors = mockContributors.get(fqn); - return contributors?.find((c: any) => c.ownership === 'own'); - }), - registerNamespace: vi.fn(), - registerKind: vi.fn(), - registerItem: vi.fn(), - registerApp: vi.fn(), - installPackage: vi.fn((manifest) => ({ - manifest, - status: 'installed', - enabled: true, - installedAt: new Date().toISOString(), - })), - reset: vi.fn(() => { - mockObjects.clear(); - mockContributors.clear(); - }), - metadata: { - get: vi.fn(() => mockObjects), + return createRegistryModuleMock({ + instance: { + getObject: vi.fn((name) => mockObjects.get(name)), + resolveObject: vi.fn((name) => mockObjects.get(name)), + getAllObjects: vi.fn(() => [...mockObjects.values()]), + registerObject: vi.fn((obj, packageId, namespace, ownership, priority) => { + const fqn = namespace ? `${namespace}__${obj.name}` : obj.name; + mockObjects.set(fqn, { ...obj, name: fqn }); + if (!mockContributors.has(fqn)) { + mockContributors.set(fqn, []); + } + const contributors = mockContributors.get(fqn); + contributors.push({ packageId, namespace, ownership, priority, definition: obj }); + return fqn; + }), + getObjectOwner: vi.fn((fqn) => { + const contributors = mockContributors.get(fqn); + return contributors?.find((c: any) => c.ownership === 'own'); + }), + installPackage: vi.fn((manifest) => ({ + manifest, + status: 'installed', + enabled: true, + installedAt: new Date().toISOString(), + })), + reset: vi.fn(() => { + mockObjects.clear(); + mockContributors.clear(); + }), + metadata: { + get: vi.fn(() => mockObjects), + }, }, - }; - function SchemaRegistry() { - return instance; - } - Object.assign(SchemaRegistry, instance); - return { - SchemaRegistry, computeFQN: (ns: string | undefined, name: string) => (ns && ns !== 'base' && ns !== 'system') ? `${ns}__${name}` : name, parseFQN: (fqn: string) => { @@ -66,8 +63,7 @@ vi.mock('./registry', () => { if (idx < 0) return { namespace: undefined, shortName: fqn }; return { namespace: fqn.slice(0, idx), shortName: fqn.slice(idx + 2) }; }, - RESERVED_NAMESPACES: new Set(['base', 'system']), - }; + }); }); describe('ObjectQL Engine', () => { diff --git a/packages/objectql/src/registry-module-mock.ts b/packages/objectql/src/registry-module-mock.ts new file mode 100644 index 0000000000..a969fa8905 --- /dev/null +++ b/packages/objectql/src/registry-module-mock.ts @@ -0,0 +1,182 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The ONE `vi.mock('./registry', …)` factory for this package's engine suites + * (#10551). + * + * ## What this replaces + * + * `vi.mock('./registry', …)` appeared in **twelve** test files in this + * directory, and all twelve were hand-copies of one shape: the same + * `const instance: any = {…}` member list, the same + * `function SchemaRegistry() { return instance; }`, the same + * `Object.assign(SchemaRegistry, instance)`, the same trailing `computeFQN` / + * `parseFQN` / `RESERVED_NAMESPACES` envelope. + * + * They had drifted, and the drift was invisible by construction — nothing + * compared the copies. Eleven declared twelve members; + * `engine-count-read-filter.test.ts` declared **eleven**, omitting + * `getAllObjects`. That omission was measured to be inert (a recording `Proxy` + * on all twelve doubles over the whole 224-file suite recorded **zero** + * accesses to an undeclared member), but inert is a property of which paths a + * suite happens to drive today, not a property of the double: the same omission + * in a suite that *did* drive one of the 13 `getAllObjects` call sites in this + * package is exactly #9002, and #9154, and #8896. + * + * ## Why a shared factory rather than adding the one missing line + * + * Adding `getAllObjects` to the outlier fixes today's instance and leaves the + * mechanism that produced it. The deciding evidence is the two lesson comments + * carried below: each one was written once, into whichever copy happened to be + * under repair, and **could not reach the other eleven, because there was no + * shared factory to write it in**. Here, every call site inherits them. + * + * ## Why this module is `.ts` and not `*.test.ts` + * + * Two reasons, and they point the same way as `register-object-authored-shape.pin.ts`: + * - vitest's default `include` collects `*.test.ts`, so a shared helper named + * that way is collected as a suite with no tests in it; + * - `packages/objectql/tsconfig.json` excludes `**\/*.test.ts`, so a helper + * named that way would be type-checked by no program the `typecheck` script + * runs. This file IS in that program. + * + * It is not reachable from `src/index.ts` or `src/core.ts`, so `tsup` never + * bundles it and it is never published. + * + * ## Usage — the async factory is the hoisting-safe form + * + * `vi.mock` calls are hoisted above the file's imports, so this module cannot be + * imported at the top of a test and referenced from the factory. `vi.mock` + * accepts an **async** factory, which makes a dynamic `import()` inside it legal + * and ordered correctly: + * + * ```ts + * vi.mock('./registry', async () => { + * const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + * return createRegistryModuleMock(); + * }); + * ``` + * + * Pass `instance` to override individual members with suite-specific behaviour + * (`engine.test.ts` drives a stateful in-memory registry that way), and + * `computeFQN` / `parseFQN` / `reservedNamespaces` to override the module + * envelope. Overrides are merged over the defaults, so a suite that overrides + * one member still inherits the other eleven — which is the property that makes + * the drift unrepeatable. + */ + +import { vi } from 'vitest'; + +/** + * The registry double's member set — the twelve members every hand-copy + * declared (before the merge, eleven of the twelve copies declared all of + * them; the twelfth omitted `getAllObjects`). + * + * The index signature is deliberate: the hand-copies were typed `any`, and a + * suite that needs to model a member the real `SchemaRegistry` grows tomorrow + * must be able to add it here without a type edit standing in the way. + */ +export interface RegistryDoubleInstance { + getObject: any; + resolveObject: any; + getAllObjects: any; + registerObject: any; + getObjectOwner: any; + registerNamespace: any; + registerKind: any; + registerItem: any; + registerApp: any; + installPackage: any; + reset: any; + metadata: any; + [member: string]: any; +} + +/** The module shape `vi.mock('./registry', …)` must return. */ +export interface RegistryModuleMock { + SchemaRegistry: any; + computeFQN: (namespace: string | undefined, name: string) => string; + parseFQN: (fqn: string) => { namespace: string | undefined; shortName: string }; + RESERVED_NAMESPACES: Set; +} + +export interface RegistryModuleMockOptions { + /** Per-suite member overrides, merged over the default member set. */ + instance?: Partial; + /** Defaults to the identity mapping (no namespace prefixing). */ + computeFQN?: (namespace: string | undefined, name: string) => string; + /** Defaults to "the whole string is the short name". */ + parseFQN?: (fqn: string) => { namespace: string | undefined; shortName: string }; + /** Defaults to `new Set(['base', 'system'])`. */ + reservedNamespaces?: Set; +} + +/** + * Build the module object for `vi.mock('./registry', …)`. + * + * @param options per-suite overrides; every unset member takes the default. + */ +export function createRegistryModuleMock( + options: RegistryModuleMockOptions = {}, +): RegistryModuleMock { + const { + instance: instanceOverrides, + computeFQN = (_namespace: string | undefined, name: string) => name, + parseFQN = (fqn: string) => ({ namespace: undefined, shortName: fqn }), + reservedNamespaces = new Set(['base', 'system']), + } = options; + + const instance: RegistryDoubleInstance = { + getObject: vi.fn(), + resolveObject: vi.fn((name: string) => instance.getObject(name)), + // [#9002] This double used to omit `getAllObjects`, and the suite passed + // anyway: `delete()`'s by-id branch reads it twice (`planCascadeAtomicity`, + // then `cascadeDeleteRelations`) and BOTH reads sat behind a `catch` that + // answered "no relations". The swallow absorbed the `TypeError` this + // omission raises just as silently as it would absorb a real read failure, + // so an incomplete double read as a registry with nothing in it. With the + // swallows gone the omission is a hard failure, which is the point — the + // double now has to model the method the engine actually calls. Empty is + // the right body here: a suite that registers no relations is telling the + // truth when it says "no object references the deleted one", where a double + // that cannot answer at all leaves the engine to invent it. + // + // [#9154] The same member, the same lesson, learned a second time from the + // other side: the engine's roll-up summary index read it as + // `getAllObjects?.() ?? []`, so a double that does not model the method was + // indistinguishable from a registry with nothing in it — the write path + // silently skipped the insert-time roll-up seed (#5749) and the post-write + // recompute. With the optional call gone the omission is a hard `TypeError`. + // Empty is the truthful body for a suite that declares no `summary` field: + // the roll-up index over it is empty either way, and now it says so instead + // of the engine inventing it. + // + // ⚠️ Both lessons were written into ONE of the twelve hand-copies each, and + // neither could reach the other eleven. That is why they live here (#10551). + // A suite whose truthful answer is NOT "no objects" overrides this member — + // it does not delete it. + getAllObjects: vi.fn(() => []), + registerObject: vi.fn(), + getObjectOwner: vi.fn(), + registerNamespace: vi.fn(), + registerKind: vi.fn(), + registerItem: vi.fn(), + registerApp: vi.fn(), + installPackage: vi.fn(), + reset: vi.fn(), + metadata: { get: vi.fn(() => new Map()) }, + ...instanceOverrides, + }; + + function SchemaRegistry(): RegistryDoubleInstance { + return instance; + } + Object.assign(SchemaRegistry, instance); + + return { + SchemaRegistry, + computeFQN, + parseFQN, + RESERVED_NAMESPACES: reservedNamespaces, + }; +}