refactor(modules)!: de-UO core's copy, and enforce it (phase 3, slice 4)
Phase 3's acceptance criterion 1, made real. Three things, one review: **The dead bindings.** `client/src/api/client.js` still carried ~190 lines of UO namespaces — `shard`, `atlas`, the two SSE URLs, `admin.shard/shardOps/atlas/ userShard`, the uo-link and town-crier calls, `player.shard` — with zero core consumers since slice 3 deleted the views. module-uo vendors its own bindings. The five assertions core's `apiClient.test.js` made about those URLs moved with them (Module-uo#5); the encoding test that used `governorHistory` now uses a core route. **The copy.** Core is the platform, not one game's site, so its words are game-neutral now: `About`, `Screenshots`, `Website`'s cards, `Status` (which was never about a game server at all — it reports site mode), `Wiki`, `SiteFooter`, the default hero, `brand.js`'s tagline and description, the seeded wiki categories, and two user-visible NavEditor strings that named a module's admin screen by its proper name. Which game an instance is for is the operator's to say — BRAND_* vars, the hero editor, CMS pages — and every real instance already does: `.env.uomysticmoon.example` sets both brand strings explicitly, so nothing live changes wording. Wiki page SLUGS are untouched: `seedDefault*` only inserts what is absent, so renaming one adds a duplicate page to every install. Also gone: an orphan comment block in `schema.sql` describing the spawn-atlas tables slice 1 took away, and the two settings rows core seeded for a module (`game_account_signup`, `uo_link_protocol_3_migrated`). The second was a live defect — see Module-uo#5, which takes ownership of both and repairs the one-shot migration core's ordering had disabled. **The check.** `scripts/checkModuleIdentifiers.js` + `npm run check:modules`, first step of the server-tests job because it needs no dependencies. It reads CODE, not prose — file names, import specifiers, route path literals, declared identifiers and property names — per §5.2, so core's English may still say "shard" where saying it is worth more than the word costs. Two things it gets right only because getting them wrong was tried first: it matches WHOLE WORDS (a substring pass flags `defaultImage`, which contains "ultIma", four times in this repo), and it strips comments and string bodies in one character walk (a comment contains quotes, a string contains `//`) — the `checkImports.js` lesson. It has its own 17-test suite, because a boundary check that silently stops checking is worse than none. The three §6.5 grandfathering allowlists are exempt by name, and an exemption that stops matching fails the build rather than lingering. BREAKING CHANGE: core no longer seeds `game_account_signup` or `uo_link_protocol_3_migrated`; module-uo's schema fragment does. An install running core without module-uo keeps whatever rows it already has and gains no new ones — nothing in core reads either key. Deferred to slice 5, deliberately: README.md's 48 UO mentions, including a `## Shard integration (uo-link)` section and the architecture diagram. That is documentation, which §5.2 does not cover, and it belongs with the phase-closing docs pass rather than half-done here. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
151
server/test/checkModuleIdentifiers.test.js
Normal file
151
server/test/checkModuleIdentifiers.test.js
Normal file
@@ -0,0 +1,151 @@
|
||||
// ── The §5.2 check has to be provably still checking ───────────────────────
|
||||
//
|
||||
// `scripts/checkModuleIdentifiers.js` passes today. So does a check that reads
|
||||
// nothing, and the two are indistinguishable from CI's green tick — which is the
|
||||
// whole failure mode of a boundary test: it is written when the boundary is
|
||||
// clean, it never fires again, and nobody finds out it stopped working until the
|
||||
// thing it guards has been broken for months.
|
||||
//
|
||||
// So this file feeds it code it MUST reject and code it MUST accept. The
|
||||
// accept cases are not filler: every one of them is a false positive that a
|
||||
// simpler implementation actually produced against this repo, and each would
|
||||
// have made the check something people route around rather than obey.
|
||||
|
||||
const { test } = require('node:test')
|
||||
const assert = require('node:assert/strict')
|
||||
|
||||
const {
|
||||
run,
|
||||
checkFile,
|
||||
maskCode,
|
||||
tokenize,
|
||||
reservedWordIn,
|
||||
EXEMPT,
|
||||
} = require('../../scripts/checkModuleIdentifiers')
|
||||
|
||||
const words = (src) => checkFile('x.js', src).map((h) => `${h.kind}:${h.name}`)
|
||||
|
||||
// ── the repo itself ─────────────────────────────────────────────────────────
|
||||
|
||||
test('core, as it stands, names no module identifier', () => {
|
||||
const { hits, unused } = run()
|
||||
assert.deepEqual(
|
||||
hits.map((h) => `${h.file}:${h.line} ${h.kind} "${h.name}"`),
|
||||
[],
|
||||
)
|
||||
assert.deepEqual(unused.map((e) => `${e.file} "${e.name}"`), [], 'a grandfathering exemption matches nothing')
|
||||
})
|
||||
|
||||
test('every exemption is one of the §6.5 grandfathering allowlists', () => {
|
||||
// Not a count assertion — a reason assertion. The exemption list is the only
|
||||
// part of this check that can weaken it, so what it may contain is pinned.
|
||||
for (const e of EXEMPT) {
|
||||
assert.match(e.file, /^server\/src\/modules\//, `${e.file} is not core's module machinery`)
|
||||
assert.equal(e.name, 'uo')
|
||||
assert.match(e.why, /§6\.5/)
|
||||
}
|
||||
})
|
||||
|
||||
// ── what it must catch ──────────────────────────────────────────────────────
|
||||
|
||||
test('an import specifier reaching into a module is a hit', () => {
|
||||
assert.deepEqual(words("const s = require('../shardState/shardState.model')"), [
|
||||
'import specifier:../shardState/shardState.model',
|
||||
])
|
||||
assert.deepEqual(words("import { feed } from './lib/useShardFeed.js'"), [
|
||||
'import specifier:./lib/useShardFeed.js',
|
||||
])
|
||||
})
|
||||
|
||||
test('a route path literal for a module surface is a hit', () => {
|
||||
assert.deepEqual(words("router.get('/shard/status', handler)"), ['route path:/shard/status'])
|
||||
assert.deepEqual(words("app.use('/uo-link', r)"), ['route path:/uo-link'])
|
||||
assert.deepEqual(words("router.post('/atlas/import', h)"), ['route path:/atlas/import'])
|
||||
})
|
||||
|
||||
test('a declared identifier carrying a module word is a hit', () => {
|
||||
assert.deepEqual(words('function shardStatus() {}'), ['declared identifier:shardStatus'])
|
||||
assert.deepEqual(words('const uoLinkClient = 1'), ['declared identifier:uoLinkClient'])
|
||||
assert.deepEqual(words('class TownCrier {}'), ['declared identifier:TownCrier'])
|
||||
})
|
||||
|
||||
test('a property name carrying a module word is a hit', () => {
|
||||
assert.deepEqual(words('const api = {\n shard: {},\n}'), ['property name:shard'])
|
||||
})
|
||||
|
||||
test('the module id on its own is a hit', () => {
|
||||
assert.deepEqual(words("router.use('/uo', r)"), ['route path:/uo'])
|
||||
})
|
||||
|
||||
// ── what it must NOT catch ──────────────────────────────────────────────────
|
||||
|
||||
test('a substring that is not a word is not a hit — `defaultImage` contains "ultIma"', () => {
|
||||
// The real false positive: a case-insensitive substring pass flagged
|
||||
// heroLayout.js and Portal.jsx four times over on its first run, for a
|
||||
// parameter called `defaultImage`.
|
||||
assert.deepEqual(words('function heroBackground(layout, { defaultImage } = {}) {}'), [])
|
||||
assert.deepEqual(words('const fallback = defaultImage || DEFAULT_HERO_IMAGE'), [])
|
||||
})
|
||||
|
||||
test('core prose may say the words — comments are not read', () => {
|
||||
assert.deepEqual(words('// the shard one does, indirectly, by asking an endpoint'), [])
|
||||
assert.deepEqual(words('/* Wraps the public site. When the shard is in maintenance … */'), [])
|
||||
assert.deepEqual(words('/**\n * hidden by a module\'s shard visibility rules\n */'), [])
|
||||
})
|
||||
|
||||
test('core copy may say the words — string CONTENT is not read', () => {
|
||||
assert.deepEqual(words("const lead = 'daily life on the shard'"), [])
|
||||
assert.deepEqual(words('const t = `an independent Ultima Online shard`'), [])
|
||||
})
|
||||
|
||||
test('a comment containing quotes does not swallow the file', () => {
|
||||
// The `checkImports.js` lesson, restated: strip comments BEFORE quotes, in one
|
||||
// pass. A comment with an apostrophe used to open a string that ran to EOF,
|
||||
// and everything after it silently stopped being checked.
|
||||
const src = "// it's the operator's own file\nfunction shardStatus() {}"
|
||||
assert.deepEqual(words(src), ['declared identifier:shardStatus'])
|
||||
})
|
||||
|
||||
test('a string containing // does not hide the code after it', () => {
|
||||
const src = "const u = 'https://example.com/x'\nfunction shardFeed() {}"
|
||||
assert.deepEqual(words(src), ['declared identifier:shardFeed'])
|
||||
})
|
||||
|
||||
test('an escaped quote inside a string does not end it early', () => {
|
||||
const src = "const s = 'it\\'s fine, shard'\nconst clilocMap = {}"
|
||||
assert.deepEqual(words(src), ['declared identifier:clilocMap'])
|
||||
})
|
||||
|
||||
test('ordinary core words that merely resemble a reserved one pass', () => {
|
||||
assert.deepEqual(words('const link = 1'), []) // "link" alone is not "uo-link"
|
||||
assert.deepEqual(words('const unlinkIdentity = () => {}'), [])
|
||||
assert.deepEqual(words('const townName = 1'), []) // "town" alone is not "towncrier"
|
||||
assert.deepEqual(words('const uploadUrl = 1'), [])
|
||||
})
|
||||
|
||||
// ── the internals the above depends on ──────────────────────────────────────
|
||||
|
||||
test('tokenize splits camelCase, snake_case and kebab-case the same way', () => {
|
||||
assert.deepEqual(tokenize('shardStatus'), ['shard', 'status'])
|
||||
assert.deepEqual(tokenize('uo_link_config'), ['uo', 'link', 'config'])
|
||||
assert.deepEqual(tokenize('uo-link'), ['uo', 'link'])
|
||||
assert.deepEqual(tokenize('UOLinkClient'), ['uo', 'link', 'client'])
|
||||
assert.deepEqual(tokenize('defaultImage'), ['default', 'image'])
|
||||
})
|
||||
|
||||
test('reservedWordIn matches whole words and adjacent pairs only', () => {
|
||||
assert.equal(reservedWordIn('shardFeed'), 'shard')
|
||||
assert.equal(reservedWordIn('uoLinkConfig'), 'uo')
|
||||
assert.equal(reservedWordIn('townCrier'), 'town-crier')
|
||||
assert.equal(reservedWordIn('defaultImage'), null)
|
||||
assert.equal(reservedWordIn('townHall'), null)
|
||||
})
|
||||
|
||||
test('maskCode keeps every offset, so reported line numbers are the real ones', () => {
|
||||
const src = "// shard\nconst a = 'shard'\nfunction shardX() {}"
|
||||
const masked = maskCode(src)
|
||||
assert.equal(masked.length, src.length)
|
||||
assert.equal(masked.split('\n').length, src.split('\n').length)
|
||||
const [hit] = checkFile('x.js', src)
|
||||
assert.equal(hit.line, 3)
|
||||
})
|
||||
Reference in New Issue
Block a user