From 96d6e0afdbb32130a0a9b9d0b7a66b1a44890220 Mon Sep 17 00:00:00 2001 From: Wassim SAMAD Date: Tue, 2 Jun 2026 10:44:15 -0400 Subject: [PATCH] fix(core): update stale registry + stair-opening tests to match behavior (#358) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two unit-test groups had drifted from intentional implementation changes and have been failing CI on main since before they were noticed: - registry: re-registering a node kind is now HMR-aware — it warns and replaces in dev/test and only throws in production (registry._register, introduced in the in-world-selection work). `bun test` runs with NODE_ENV=test, so the duplicate-kind tests took the dev (warn) path and no longer threw. Pin NODE_ENV to 'production' for the throw-path assertions via an `inProduction` helper, and add explicit dev/HMR coverage for the warn-and-replace path. - stair-opening-sync: the straight-flight opening geometry was deliberately enlarged ("increase stair opening buffer constraints", treadDepth*10 / length*0.8 / 3.0, openingOffset default 0.15). The enlarged opening now spans nearly the full slab depth, so the manual hole fixtures (which must *contain* the auto opening to suppress it) no longer covered it. Enlarge the manual-opening fixtures to genuinely cover the current opening, preserving each test's intent. No production code changed — only test fixtures/assertions. Co-authored-by: Claude Opus 4.8 --- packages/core/src/registry/registry.test.ts | 47 +++++++++++++++---- .../systems/stair/stair-opening-sync.test.ts | 16 +++---- 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/packages/core/src/registry/registry.test.ts b/packages/core/src/registry/registry.test.ts index 1052e698..9d1eb535 100644 --- a/packages/core/src/registry/registry.test.ts +++ b/packages/core/src/registry/registry.test.ts @@ -12,6 +12,20 @@ import { } from './registry' import type { AnyNodeDefinition, Plugin } from './types' +// Re-registering a kind warns + replaces in dev (HMR) but throws in +// production — see `registry._register`. `bun test` runs with +// NODE_ENV=test (dev path), so the throw-path tests pin NODE_ENV to +// 'production' for the duration of the call. +async function inProduction(fn: () => T | Promise): Promise { + const prev = process.env.NODE_ENV + process.env.NODE_ENV = 'production' + try { + return await fn() + } finally { + process.env.NODE_ENV = prev + } +} + function makeDefinition( kind: string, overrides: Partial = {}, @@ -47,9 +61,20 @@ describe('nodeRegistry', () => { expect(nodeRegistry.get('column')).toBe(def) }) - test('registerNode throws on duplicate kind', () => { - registerNode(makeDefinition('column')) - expect(() => registerNode(makeDefinition('column'))).toThrow(/duplicate node kind/) + test('registerNode throws on duplicate kind in production', async () => { + await inProduction(() => { + registerNode(makeDefinition('column')) + expect(() => registerNode(makeDefinition('column'))).toThrow(/duplicate node kind/) + }) + }) + + test('registerNode replaces on duplicate kind in dev (HMR)', () => { + const first = makeDefinition('column') + const second = makeDefinition('column') + registerNode(first) + registerNode(second) + expect(nodeRegistry.size).toBe(1) + expect(nodeRegistry.get('column')).toBe(second) }) test('registerNode rejects empty kind', () => { @@ -184,19 +209,21 @@ describe('loadPlugin', () => { await expect(loadPlugin(plugin)).rejects.toThrow(/apiVersion/) }) - test('propagates duplicate-kind error from a single plugin', async () => { + test('propagates duplicate-kind error from a single plugin in production', async () => { const plugin: Plugin = { id: 'broken', apiVersion: 1, nodes: [makeDefinition('dup'), makeDefinition('dup')], } - await expect(loadPlugin(plugin)).rejects.toThrow(/duplicate node kind/) + await inProduction(() => expect(loadPlugin(plugin)).rejects.toThrow(/duplicate node kind/)) }) - test('propagates duplicate-kind error across plugins', async () => { - await loadPlugin({ id: 'a', apiVersion: 1, nodes: [makeDefinition('shared')] }) - await expect( - loadPlugin({ id: 'b', apiVersion: 1, nodes: [makeDefinition('shared')] }), - ).rejects.toThrow(/duplicate node kind/) + test('propagates duplicate-kind error across plugins in production', async () => { + await inProduction(async () => { + await loadPlugin({ id: 'a', apiVersion: 1, nodes: [makeDefinition('shared')] }) + await expect( + loadPlugin({ id: 'b', apiVersion: 1, nodes: [makeDefinition('shared')] }), + ).rejects.toThrow(/duplicate node kind/) + }) }) }) diff --git a/packages/core/src/systems/stair/stair-opening-sync.test.ts b/packages/core/src/systems/stair/stair-opening-sync.test.ts index f3a194f6..006a99a7 100644 --- a/packages/core/src/systems/stair/stair-opening-sync.test.ts +++ b/packages/core/src/systems/stair/stair-opening-sync.test.ts @@ -79,10 +79,10 @@ describe('syncAutoStairOpenings', () => { const ground = LevelNode.parse({ name: 'Ground', level: 0, parentId: building.id }) const upper = LevelNode.parse({ name: 'Upper', level: 1, parentId: building.id }) const manualOpening: Array<[number, number]> = [ - [1.2, 0.8], - [2.8, 0.8], - [2.8, 2.9], - [1.2, 2.9], + [1.0, 0.0], + [3.0, 0.0], + [3.0, 3.0], + [1.0, 3.0], ] const sourceCeiling = CeilingNode.parse({ name: 'Source Ceiling', @@ -206,10 +206,10 @@ describe('syncAutoStairOpenings', () => { const ground = LevelNode.parse({ name: 'Ground', level: 0, parentId: building.id }) const upper = LevelNode.parse({ name: 'Upper', level: 1, parentId: building.id }) const manualOpening: Array<[number, number]> = [ - [1.2, 0.8], - [2.8, 0.8], - [2.8, 2.9], - [1.2, 2.9], + [1.0, 0.0], + [3.0, 0.0], + [3.0, 3.0], + [1.0, 3.0], ] const staleAutoOpening: Array<[number, number]> = [ [1.5, 1],