From e4af7dd9a8dd6c2c3a41411c5657302e21e0e869 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 11 Aug 2026 18:00:50 -0500 Subject: [PATCH] test(client): check what the chunk registers, and fix two defects in the checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things, all about checks that had never met a real chunk. `checkExternals.js` rejected slice 3's build outright, naming a fragment of minified JSX as an imported specifier: a button reading "Approve and import" puts the token immediately before a quote, and no regexp can tell that from a statement. Same wall the server's `checkImports.js` hit, answered the same way — a character walk. A mask rather than a rewrite, because a real import has its keyword outside a string and its specifier inside one. Writing the test for that false positive found the false NEGATIVE underneath it: the pattern required whitespace after `import`, so it could not see `import{useState}from"react"` — the one shape a minified build actually emits, and the most likely way for a missed alias to reach production. It has never been able to see it. `registration.test.js` is new: stand up a fake `window.__rg` with a recording registry and the real React, import the BUILT chunk, and read back what it asked for. No DOM, because nothing renders. It holds the agreement that rots quietly — every nav row points at a route this module actually registered — rather than restating both lists. CI now builds before it tests, because both of those read `dist/entry.js` and skip without it. Run the other way round they are green and asking nothing. Co-Authored-By: Claude --- .gitea/workflows/pr-checks.yml | 11 +- client/scripts/checkExternals.js | 151 +++++++++++++++++++----- client/test/build.test.js | 61 +++++++++- client/test/regionBuckets.test.js | 60 ++++++++++ client/test/registration.test.js | 188 ++++++++++++++++++++++++++++++ client/test/shardEvents.test.js | 74 ++++++++++++ module.json | 4 +- 7 files changed, 509 insertions(+), 40 deletions(-) create mode 100644 client/test/regionBuckets.test.js create mode 100644 client/test/registration.test.js create mode 100644 client/test/shardEvents.test.js diff --git a/.gitea/workflows/pr-checks.yml b/.gitea/workflows/pr-checks.yml index 67a9983..1422d8c 100644 --- a/.gitea/workflows/pr-checks.yml +++ b/.gitea/workflows/pr-checks.yml @@ -94,11 +94,16 @@ jobs: - name: Install client deps run: npm ci --prefix client - - name: Run client tests - run: npm test --prefix client - + # The build comes FIRST, and that ordering is load-bearing as of slice 3. + # Two of the client tests read `dist/entry.js` — the chunk's externals, and + # what it registers when imported against a fake `window.__rg` — and both + # skip when there is no build. Run the other way round they skip silently + # in CI, which is the worst of both: green, and not asking the question. - name: Build the client chunk run: npm run build --prefix client + - name: Run client tests + run: npm test --prefix client + - name: Check the built chunk's externals (MODULE_API.md §3.6) run: npm run check:externals --prefix client diff --git a/client/scripts/checkExternals.js b/client/scripts/checkExternals.js index 385aa07..031b226 100644 --- a/client/scripts/checkExternals.js +++ b/client/scripts/checkExternals.js @@ -27,53 +27,146 @@ import { fileURLToPath } from 'node:url' const CHUNK = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', 'dist', 'entry.js') -if (!fs.existsSync(CHUNK)) { - console.error(`No chunk at ${CHUNK} — run \`npm run build\` first.`) - process.exit(1) +/** + * Which characters of the chunk are inside a string, template or comment. + * + * **A check that reads code with a regexp fails on code that talks about + * itself.** The first real chunk this script ever saw — slice 3's, the first + * with any content in it — was rejected for importing `" }),\n !l && …`, + * because a button reading "Approve and import" put the token `import` + * immediately before a quote and the pattern could not tell that from a + * statement. Slice 0's chunk was 0.2 kB and this branch had never run against + * anything. + * + * The server half hit the same wall from the other side and answered it the same + * way (`server/scripts/checkImports.js`): a character walk, not a cleverer + * regexp. There is no regexp that distinguishes a keyword from the same letters + * inside a string, because that distinction is a property of the parse. + * + * A mask rather than a rewrite, because the two halves of a real import — the + * keyword and the specifier — sit on opposite sides of the boundary: the keyword + * must be OUTSIDE a string and the specifier must be a string. Blanking strings + * would take the answer with the noise. + */ +export function stringMask(src) { + const inString = new Uint8Array(src.length) + let i = 0 + while (i < src.length) { + const c = src[i] + const two = src.slice(i, i + 2) + if (two === '//') { + const nl = src.indexOf('\n', i) + const end = nl === -1 ? src.length : nl + inString.fill(1, i, end) + i = end + } else if (two === '/*') { + const close = src.indexOf('*/', i + 2) + const end = close === -1 ? src.length : close + 2 + inString.fill(1, i, end) + i = end + } else if (c === '"' || c === "'" || c === '`') { + // The opening quote itself stays unmasked: a specifier is read starting + // at its quote, and the regexp below anchors on that. + i += 1 + while (i < src.length && src[i] !== c) { + // A backslash escapes the next character, including the closing quote. + const step = src[i] === '\\' ? 2 : 1 + inString.fill(1, i, Math.min(i + step, src.length)) + i += step + } + i += 1 + } else { + i += 1 + } + } + return inString } -const chunk = fs.readFileSync(CHUNK, 'utf8') -const problems = [] - // Static and dynamic imports that survived into the output. A relative or // absolute specifier is a chunk that was split, which this build does not do — // `lib` mode with one entry emits one file — so anything here is a bare name. -const IMPORTS = /(?:^|[\s;}])(?:import\s+[^'"]*?from\s*|import\s*|import\()\s*['"]([^'"]+)['"]/g -const bare = new Set() -for (const [, specifier] of chunk.matchAll(IMPORTS)) { - if (!specifier.startsWith('.') && !specifier.startsWith('/')) bare.add(specifier) -} -if (bare.size) { - problems.push( - `the chunk still imports ${[...bare].map((s) => `"${s}"`).join(', ')} — ` + - 'nothing can resolve a bare specifier in the browser without an import map, ' + - 'and CSP forbids one. Alias it to a shim in vite.config.js (MODULE_API.md §3.6).', - ) +// +// **This pattern used to require whitespace after `import`, and so could not see +// the one shape the build actually emits.** Minified Rollup output is +// `import{useState}from"react"`, with no space anywhere in it; the old +// `import\s+[^'"]*?from` needed at least one, fell through to the bare-specifier +// alternative, met `{` instead of a quote and matched nothing. A bare named +// import — the most likely way for an alias to miss — would have passed this +// check silently. It was found by writing the test for the false POSITIVE above +// it, which is the argument for testing a check against both answers. +// +// `(?:^|[^\w$.])` rather than a whitespace class, so `a.import(x)` and +// `myimport"x"` are excluded for the right reason: `import` must not be preceded +// by an identifier character or a dot. `[^'"()]*?` cannot swallow a dynamic +// import's parenthesis. +const IMPORTS = /(?:^|[^\w$.])import\s*(?:\(\s*|[^'"()]*?from\s*)?['"]([^'"]+)['"]/g + +/** Every bare specifier the chunk still imports at runtime. */ +export function bareImports(chunk) { + const masked = stringMask(chunk) + const bare = new Set() + for (const match of chunk.matchAll(IMPORTS)) { + // Where the `import` keyword itself starts — one past the leading delimiter, + // unless the match began at position 0. + const keywordAt = match.index + (match[0].startsWith('import') ? 0 : 1) + if (masked[keywordAt]) continue // the letters, inside a string. Not a statement. + const specifier = match[1] + if (!specifier.startsWith('.') && !specifier.startsWith('/')) bare.add(specifier) + } + return [...bare] } // Fingerprints from the shared libraries' own source. Each is a string those // packages ship and this module has no other reason to contain. +// +// These are matched against the RAW chunk, deliberately unmasked: a bundled +// library's source arrives as code AND as its own error-message strings, and +// masking would discard half the evidence. The direction of the risk is opposite +// to the import check's — here a false positive is a fingerprint too generic, +// which is a fixable choice of probe, not a property of the parse. const BUNDLED = [ { what: 'react', probe: 'react.development.js' }, { what: 'react', probe: 'Invalid hook call' }, { what: 'react-dom', probe: 'react-dom.development.js' }, { what: 'react-router-dom', probe: 'useRoutes() may be used only in the context of a component' }, ] -for (const { what, probe } of BUNDLED) { - if (chunk.includes(probe)) { + +/** Every problem with this chunk, as sentences. Empty means it ships. */ +export function problemsWith(chunk) { + const problems = [] + const bare = bareImports(chunk) + if (bare.length) { problems.push( - `the chunk appears to BUNDLE ${what} (found ${JSON.stringify(probe)}). ` + - 'There is exactly one React in the page and core owns it — a second copy ' + - 'loads fine and then fails at the first hook (MODULE_API.md §3.2).', + `the chunk still imports ${bare.map((s) => `"${s}"`).join(', ')} — ` + + 'nothing can resolve a bare specifier in the browser without an import map, ' + + 'and CSP forbids one. Alias it to a shim in vite.config.js (MODULE_API.md §3.6).', ) } + for (const { what, probe } of BUNDLED) { + if (chunk.includes(probe)) { + problems.push( + `the chunk appears to BUNDLE ${what} (found ${JSON.stringify(probe)}). ` + + 'There is exactly one React in the page and core owns it — a second copy ' + + 'loads fine and then fails at the first hook (MODULE_API.md §3.2).', + ) + } + } + return problems } -if (problems.length) { - console.error('\nThe built chunk breaks the shared-dependency rule:\n') - for (const p of problems) console.error(` - ${p}\n`) - process.exit(1) +// Only when run as a script. Importing this from a test must not read a chunk +// that may not have been built, and must not call process.exit. +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + if (!fs.existsSync(CHUNK)) { + console.error(`No chunk at ${CHUNK} — run \`npm run build\` first.`) + process.exit(1) + } + const problems = problemsWith(fs.readFileSync(CHUNK, 'utf8')) + if (problems.length) { + console.error('\nThe built chunk breaks the shared-dependency rule:\n') + for (const p of problems) console.error(` - ${p}\n`) + process.exit(1) + } + const kb = (fs.statSync(CHUNK).size / 1024).toFixed(1) + console.log(`OK — dist/entry.js (${kb} kB) has no bare imports and bundles no shared dependency.`) } - -const kb = (fs.statSync(CHUNK).size / 1024).toFixed(1) -console.log(`OK — dist/entry.js (${kb} kB) has no bare imports and bundles no shared dependency.`) diff --git a/client/test/build.test.js b/client/test/build.test.js index 8cd27e8..4375b80 100644 --- a/client/test/build.test.js +++ b/client/test/build.test.js @@ -20,6 +20,7 @@ import { fileURLToPath } from 'node:url' const HERE = path.dirname(fileURLToPath(import.meta.url)) const CLIENT = path.resolve(HERE, '..') +const { bareImports, problemsWith } = await import('../scripts/checkExternals.js') const configModule = await import('../vite.config.js') const config = configModule.default const { SHARED, SHARED_PACKAGES: guardedPackages } = configModule @@ -92,15 +93,63 @@ test('modulePreload polyfilling stays off — an inline bootstrap is refused und assert.strictEqual(config.build.modulePreload.polyfill, false) }) -test('every shim reads from window.__rg and imports nothing', () => { +test('exactly one file reads window.__rg, and every shim goes through it', () => { + // `shim/rg.js` is the single reader, and that is not tidiness: it is what + // makes the "core did not publish its dependencies" message reachable. The + // shims touch the global before anything else in the chunk does, so a check + // placed in the first-imported file is a guarantee that lasts until someone + // sorts the imports. const dir = path.join(CLIENT, 'src', 'shim') const shims = fs.readdirSync(dir) - assert.ok(shims.length >= 4) + assert.ok(shims.length >= 5) for (const file of shims) { const source = fs.readFileSync(path.join(dir, file), 'utf8') - assert.match(source, /window\.__rg/, `${file} does not read the global`) - // A shim that imported anything would be a shim with a dependency to - // resolve, which is the problem it exists to remove. - assert.doesNotMatch(source, /^\s*import\s/m, `${file} imports something`) + const code = source.replace(/^\s*\/\/.*$/gm, '') // the comments discuss the global + if (file === 'rg.js') { + assert.match(code, /window\.__rg/, 'rg.js must be the one that reads the global') + assert.doesNotMatch(code, /^\s*import\s/m, 'rg.js imports something') + continue + } + assert.doesNotMatch(code, /window\.__rg/, `${file} reads the global directly instead of via rg()`) + assert.match(code, /rg\(\)/, `${file} does not resolve through rg()`) + // A shim may import its sibling helper and nothing else — anything further + // would be a shim with a dependency to resolve, the problem it exists to remove. + for (const [, spec] of code.matchAll(/^\s*import\s[^'"]*['"]([^'"]+)['"]/gm)) { + assert.strictEqual(spec, './rg.js', `${file} imports ${spec}`) + } } }) + +test('the built chunk has no bare imports and bundles no shared dependency', () => { + // The artifact check itself, over the artifact that ships. Skipped rather than + // failed when there is no build: `npm test` must be runnable before `npm run + // build`, and CI runs them in order. + const chunk = path.join(CLIENT, 'dist', 'entry.js') + if (!fs.existsSync(chunk)) return + assert.deepStrictEqual(problemsWith(fs.readFileSync(chunk, 'utf8')), []) +}) + +test('an import inside a string is not an import — the check reads code, not text', () => { + // The regression that made this necessary: slice 3's chunk was the first with + // any content in it, and a button labelled "Approve and import" put the token + // immediately before a quote. The check rejected the whole build, naming a + // fragment of minified JSX as the offending specifier. + const uiCopy = 'const a=n("button",{children:"Approve and import"}),b=1;' + assert.deepStrictEqual(bareImports(uiCopy), []) + + // Neither is one in a comment, or in a template literal. + assert.deepStrictEqual(bareImports('// import "react" would be wrong here\nconst a=1'), []) + assert.deepStrictEqual(bareImports('/* import "react" */ const a=1'), []) + assert.deepStrictEqual(bareImports('const s=`import "react"`'), []) + + // And a real one still is, in each form the build could emit. + assert.deepStrictEqual(bareImports('import"react";'), ['react']) + assert.deepStrictEqual(bareImports('import{useState}from"react";'), ['react']) + assert.deepStrictEqual(bareImports('const m=await import("react-dom/client")'), ['react-dom/client']) + // A relative specifier is a split chunk, not a shared dependency: not our concern. + assert.deepStrictEqual(bareImports('import"./other.js";'), []) + + // The case that proves the mask tracks escapes: a quote escaped INSIDE a + // string must not end it early and leave the tail looking like code. + assert.deepStrictEqual(bareImports('const s="he said \\"import\\" loudly";'), []) +}) diff --git a/client/test/regionBuckets.test.js b/client/test/regionBuckets.test.js new file mode 100644 index 0000000..7649a2a --- /dev/null +++ b/client/test/regionBuckets.test.js @@ -0,0 +1,60 @@ +import { test } from 'node:test' +import assert from 'node:assert/strict' +import { bucketize, BUCKETS } from '../src/data/regionBuckets.js' + +// Unit-test the presence.online region roll-up for the "Players Online" widget. +// The load-bearing invariant: the bucket counts ALWAYS reconcile to the true +// total — anything unmatched lands in Wilderness — so the widget can never show +// a sum that disagrees with the headline online count. + +test('bucketize groups named regions into their buckets', () => { + const { rows, total } = bucketize({ + 'Britain': 4, + 'Moonglow': 2, + 'Despise': 3, + 'Green Acres House 12': 1, // not a town/dungeon name → Housing + }) + const byId = Object.fromEntries(rows.map((r) => [r.id, r.count])) + assert.equal(byId.britain, 4) + assert.equal(byId.towns, 2) + assert.equal(byId.dungeons, 3) + assert.equal(byId.housing, 1) + assert.equal(total, 10) +}) + +test('first match wins by BUCKETS order: a town-named house region counts as Towns, not Housing', () => { + // The towns regex is ^-anchored and towns is checked BEFORE housing, so a house + // region whose name starts with a town name is bucketed as Towns. Pinning this + // documents the ordering dependency for anyone retuning BUCKETS. + const { rows } = bucketize({ 'Trinsic House 12': 1 }) + const byId = Object.fromEntries(rows.map((r) => [r.id, r.count])) + assert.equal(byId.towns, 1) + assert.equal(byId.housing, undefined) // empty bucket dropped +}) + +test('an unmatched region falls through to Wilderness so counts always reconcile', () => { + const { rows, total } = bucketize({ 'Some Unnamed Field': 5, 'Wilderness': 2 }) + const wilderness = rows.find((r) => r.id === 'wilderness') + assert.equal(wilderness.count, 7) + assert.equal(total, 7) + // The reconciliation guarantee: the buckets sum to the total, exactly. + assert.equal(rows.reduce((s, r) => s + r.count, 0), total) +}) + +test('bucketize returns rows in BUCKETS order and drops empty buckets', () => { + const { rows } = bucketize({ 'Despise': 1, 'Britain': 1 }) + assert.deepEqual(rows.map((r) => r.id), ['britain', 'dungeons']) // BUCKETS order, no empty towns/housing/wilderness +}) + +test('bucketize coerces non-numeric counts and tolerates empty/nullish input', () => { + assert.deepEqual(bucketize({}), { rows: [], total: 0 }) + assert.deepEqual(bucketize(), { rows: [], total: 0 }) + const { total } = bucketize({ 'Britain': '3', 'Minoc': 'oops' }) + assert.equal(total, 3) // '3' → 3, 'oops' → 0 +}) + +test('the last bucket is the catch-all (its match accepts anything)', () => { + const last = BUCKETS[BUCKETS.length - 1] + assert.equal(last.id, 'wilderness') + assert.equal(last.match('literally anything'), true) +}) diff --git a/client/test/registration.test.js b/client/test/registration.test.js new file mode 100644 index 0000000..0a640ad --- /dev/null +++ b/client/test/registration.test.js @@ -0,0 +1,188 @@ +// ── What the chunk registers, checked without a browser ──────────────────── +// +// `build.test.js` says the honest thing about this half: its real failures are +// timing and resolution, and a DOM-less runner cannot see either. That is still +// true, and MODULE_API.md §7.7's browser smoke is still what proves the module +// works. But it left a gap worth closing, and slice 3 is when it started to +// matter: nothing checked *what* the chunk registers. +// +// It can be checked, because registration is the one thing this chunk does at +// evaluation time and it does it through an object core hands it. So: stand up a +// fake `window.__rg` with a recording registry and the real React behind it, +// import the BUILT artifact, and read back what it asked for. No DOM is needed +// because nothing renders — `` is `jsx(Shard)`, an object, and the +// route table is full of them by design. +// +// What this catches that review does not: a page that silently stops being +// routed, a nav row whose `to` drifts from its route's path, a slot fill that +// was renamed on one side, and the whole registration surface disappearing +// because an exception was thrown halfway down entry.jsx. +// +// What it deliberately does NOT do is re-assert the paths as a literal list. +// The interesting property is that the nav and the routes AGREE, and a test that +// restates both is a second copy of the thing it is checking. + +import test from 'node:test' +import assert from 'node:assert/strict' +import fs from 'node:fs' +import path from 'node:path' +import { fileURLToPath } from 'node:url' + +import * as react from 'react' +import * as jsxRuntime from 'react/jsx-runtime' +import * as router from 'react-router-dom' + +const HERE = path.dirname(fileURLToPath(import.meta.url)) +const CHUNK = path.resolve(HERE, '..', 'dist', 'entry.js') + +// A component, as far as the registry cares. The kit's real members are core's; +// nothing here renders, so a named stub is enough to be imported and passed on. +const stub = (name) => Object.assign(() => null, { displayName: name }) + +function fakeRg() { + const routes = { public: [], admin: [], player: [] } + const nav = { public: [], admin: [], player: [] } + const providers = new Map() + const extensions = new Map() + return { + version: '1.3.0', + react, + jsxRuntime, + router, + // `react-dom/client` is imported for the identity check in core.js and never + // called — createRoot in a DOM-less process would throw. The shim reads this + // object, so the check compares against whatever is here. + reactDom: { createRoot: () => { throw new Error('not in a browser') } }, + ui: Object.fromEntries( + ['PublicLayout', 'PageHeader', 'Loading', 'ErrorState', 'EmptyState', 'useAsync', 'useAuth', 'useSite'] + .map((n) => [n, stub(n)]), + ), + api: { request: async () => ({}), ApiError: Error, BASE: '/api/v1' }, + registry: { + registerRoutes(id, byArea) { + for (const [area, list] of Object.entries(byArea || {})) { + for (const r of list || []) routes[area].push({ ...r, path: `${id}/${r.path}`, moduleId: id }) + } + }, + registerNav(id, { area, items }) { + for (const item of items || []) nav[area].push({ ...item, moduleId: id }) + }, + registerFeatureProvider(id, namespace, hook) { providers.set(namespace, { id, hook }) }, + registerExtension(id, slot, Component) { + if (extensions.has(slot)) throw new Error(`slot "${slot}" already filled`) + extensions.set(slot, { id, Component }) + }, + routesFor: (area) => routes[area], + navFor: (area) => nav[area], + }, + _read: () => ({ routes, nav, providers, extensions }), + } +} + +// Loaded once: an ES module is evaluated a single time per process however many +// times it is imported, so every test below reads the same registration pass — +// which is also how it behaves in a browser. +let registered = null +let skip = false + +if (!fs.existsSync(CHUNK)) { + skip = true +} else { + const rg = fakeRg() + globalThis.window = { __rg: rg } + await import(`${new URL(`file://${CHUNK.split(path.sep).join('/')}`)}`) + registered = rg._read() +} + +const it = (name, fn) => test(name, { skip: skip && 'no dist/entry.js — run npm run build' }, fn) + +it('registers routes in all three areas, namespaced under the module id', () => { + const { routes } = registered + assert.equal(routes.public.length, 12) + assert.equal(routes.admin.length, 7) + assert.equal(routes.player.length, 2) + for (const area of ['public', 'admin', 'player']) { + for (const r of routes[area]) { + assert.match(r.path, /^uo\//, `${area} route "${r.path}" is not under the module namespace`) + assert.ok(r.element, `${area} route "${r.path}" has no element`) + } + } +}) + +it('every route path is distinct within its area', () => { + // Two routes on one path is a page that can never be reached, and React + // renders the first without complaint. + for (const [area, list] of Object.entries(registered.routes)) { + const paths = list.map((r) => r.path) + assert.equal(new Set(paths).size, paths.length, `duplicate path in ${area}`) + } +}) + +it('every nav row points at a route this module actually registered', () => { + // The agreement that matters, and the one that rots quietly: a row survives a + // route rename and becomes a link to core's catch-all redirect. Nav rows carry + // the FULL rendered path (`/uo/shard`), routes carry the namespaced one + // (`uo/shard`), and reconciling them is the whole test. + const rendered = { + public: (p) => `/${p}`, + admin: (p) => `/admin/${p}`, + player: (p) => `/player/${p}`, + } + for (const [area, rows] of Object.entries(registered.nav)) { + const reachable = new Set(registered.routes[area].map((r) => rendered[area](r.path))) + for (const row of rows) { + assert.ok( + reachable.has(row.to), + `${area} nav row "${row.label}" links to ${row.to}, which no route serves`, + ) + } + } +}) + +it('admin nav rows carry an icon; core rows all have one and a text-only row reads as breakage', () => { + for (const row of registered.nav.admin) { + assert.equal(typeof row.icon, 'function', `admin nav row "${row.label}" has no icon`) + } +}) + +it('a nav row that gates on a feature is gated by a namespace this module provides', () => { + // Resolution is by the REGISTERING module (§3.3), so a `feature` on a row from + // a module that registered no provider resolves against nothing — and + // everything fails open, which would re-advertise surfaces an operator hid. + const gated = Object.values(registered.nav).flat().filter((r) => r.feature) + assert.ok(gated.length > 0) + assert.ok(registered.providers.has('uo'), 'rows carry feature gates but no provider was registered') +}) + +it('fills the three extension slots, each with a component', () => { + const { extensions } = registered + assert.deepEqual( + [...extensions.keys()].sort(), + ['admin.users.detail', 'player.invite.accepted', 'site.footer.status'], + ) + for (const [slot, { id, Component }] of extensions) { + assert.equal(id, 'uo', `${slot} was filled under the wrong owner id`) + assert.equal(typeof Component, 'function', `${slot} was not filled with a component`) + } +}) + +it('the manifest\'s declared server slot is one this module fills', () => { + // module.json declares SERVER slots and the loader validates them before the + // chunk is ever served. Client slots cannot be declared there — the server has + // no knowledge of them — so this is the one place the two halves are compared. + const manifest = JSON.parse(fs.readFileSync(path.resolve(HERE, '..', '..', 'module.json'), 'utf8')) + for (const slot of manifest.extensions || []) { + assert.ok(registered.extensions.has(slot), `module.json declares "${slot}" and the chunk does not fill it`) + } +}) + +it('registers under exactly one module id, matching the manifest', () => { + const manifest = JSON.parse(fs.readFileSync(path.resolve(HERE, '..', '..', 'module.json'), 'utf8')) + const owners = new Set([ + ...Object.values(registered.routes).flat().map((r) => r.moduleId), + ...Object.values(registered.nav).flat().map((r) => r.moduleId), + ...[...registered.extensions.values()].map((e) => e.id), + ...[...registered.providers.values()].map((p) => p.id), + ]) + assert.deepEqual([...owners], [manifest.id]) +}) diff --git a/client/test/shardEvents.test.js b/client/test/shardEvents.test.js new file mode 100644 index 0000000..c4e5327 --- /dev/null +++ b/client/test/shardEvents.test.js @@ -0,0 +1,74 @@ +import { test } from 'node:test' +import assert from 'node:assert/strict' +import { describe, categoryOf, kindLabel, CATEGORIES } from '../src/lib/shardEvents.js' + +// Unit-test the shared shard-event formatter — the single place that decides how +// each event kind reads and which filter category it belongs to. These strings +// are user-facing on the public Shard page, the Activity feed, and the admin +// live feed, so a regression here is visible everywhere at once. + +// ── describe(): works on both stored (.payload) and live (top-level) frames ── +test('describe reads fields from .payload when present, else the top level', () => { + const stored = { kind: 'quest.complete', payload: { who: { name: 'Ada' }, quest: 'The Cavern' } } + const live = { kind: 'quest.complete', who: { name: 'Ada' }, quest: 'The Cavern' } + assert.equal(describe(stored), 'Ada completed “The Cavern”') + assert.equal(describe(live), 'Ada completed “The Cavern”') +}) + +test('describe resolves an actor from name → acct → "Someone"', () => { + assert.equal(describe({ kind: 'mob.login', who: { name: 'Bob' } }), 'Bob entered the world') + assert.equal(describe({ kind: 'mob.login', who: { acct: 'acct7' } }), 'acct7 entered the world') + assert.equal(describe({ kind: 'mob.login', who: null }), 'Someone entered the world') + assert.equal(describe({ kind: 'mob.login', who: 'RawString' }), 'RawString entered the world') +}) + +test('describe pluralizes a vendor sale only when amount > 1 and formats the price', () => { + assert.equal(describe({ kind: 'vendor.sale', itemType: 'Katana', amount: 1, price: 1200 }), 'Katana sold for 1,200gp') + assert.equal(describe({ kind: 'vendor.sale', itemType: 'Arrow', amount: 40, price: 80 }), 'Arrow ×40 sold for 80gp') +}) + +test('describe includes the killer only when present (optional clause)', () => { + assert.equal(describe({ kind: 'player.death', who: { name: 'Ada' } }), 'Ada was slain') + assert.equal( + describe({ kind: 'player.death', who: { name: 'Ada' }, killer: { name: 'Orc' } }), + 'Ada was slain by Orc', + ) +}) + +test('describe champ.update branches on status and boss state', () => { + assert.equal(describe({ kind: 'champ.update', name: 'Rikktor', status: 'active', bossUp: true }), 'Rikktor: boss is up') + assert.equal( + describe({ kind: 'champ.update', name: 'Rikktor', status: 'active', level: 3 }), + 'Rikktor is active — level 3', + ) + assert.equal(describe({ kind: 'champ.update', name: 'Rikktor', status: 'cooldown' }), 'Rikktor is on cooldown') +}) + +test('describe falls back to the raw kind for an unknown event', () => { + assert.equal(describe({ kind: 'some.future.kind' }), 'some.future.kind') +}) + +// ── categoryOf(): membership + catch-all ──────────────────────────────── +test('categoryOf groups kinds per the CATEGORIES table, and unknowns are "other"', () => { + assert.equal(categoryOf('player.death'), 'pvp') + assert.equal(categoryOf('skill.gain'), 'progress') + assert.equal(categoryOf('house.decay'), 'world') + assert.equal(categoryOf('vendor.sale'), 'other') // deliberately not a public category + assert.equal(categoryOf('totally.unknown'), 'other') +}) + +test('every kind listed in CATEGORIES maps back to that category (table stays consistent)', () => { + for (const cat of CATEGORIES) { + if (!cat.kinds) continue + for (const kind of cat.kinds) { + assert.equal(categoryOf(kind), cat.id, `${kind} should be in ${cat.id}`) + } + } +}) + +// ── kindLabel(): badge text ───────────────────────────────────────────── +test('kindLabel turns dots/underscores into spaces and tolerates empty input', () => { + assert.equal(kindLabel('player.death'), 'player death') + assert.equal(kindLabel('account.login.attempt'), 'account login attempt') + assert.equal(kindLabel(null), '') +}) diff --git a/module.json b/module.json index f7b76ca..26610c2 100644 --- a/module.json +++ b/module.json @@ -1,8 +1,8 @@ { "id": "uo", "name": "Ultima Online", - "version": "0.2.0", - "coreApi": "^1.1.0", + "version": "0.3.0", + "coreApi": "^1.3.0", "server": "server/index.js", "client": { "entry": "client/dist/entry.js" }, "schema": "server/db/schema.sql",