From ac918d362f884cf1593642149e2ec392773efa8d Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 10 Oct 2026 19:56:54 +0200 Subject: [PATCH] build(server): the test-mock and hand-built-service ratchets read code, not comments, and see the import() and alias forms lint:test-mocks missed vi.mock(import('../src/config'), ...), the typed form Vitest takes, and counted mocks quoted in doc comments, which left slack a real mock could take. lint:test-new-service missed a service built under an import alias or as new (Name)(...). Both now strip comments first and recognise those forms; the baselines lose the entries that were only comments. --- server/scripts/lib/count-ratchet.mjs | 37 +++++++++++++++++++ server/scripts/test-mocks-baseline.json | 3 -- server/scripts/test-mocks-ratchet.mjs | 13 ++++--- server/scripts/test-new-service-baseline.json | 13 +++---- server/scripts/test-new-service-ratchet.mjs | 37 ++++++++++++++++--- .../test-architecture-ratchets.test.ts | 26 +++++++++++++ 6 files changed, 106 insertions(+), 23 deletions(-) diff --git a/server/scripts/lib/count-ratchet.mjs b/server/scripts/lib/count-ratchet.mjs index ddcf68196..ab2113bbf 100644 --- a/server/scripts/lib/count-ratchet.mjs +++ b/server/scripts/lib/count-ratchet.mjs @@ -24,6 +24,43 @@ function walk(dir, files = []) { return files; } +/** + * The text with every // and /* comment blanked out (newlines kept, so line + * numbers hold), strings and template literals left as they are. A count + * that reads code, not prose, runs over this: a call quoted in a doc comment + * is not a call. + */ +export function stripComments(text) { + let out = ''; + let i = 0; + while (i < text.length) { + const c = text[i]; + const next = text[i + 1]; + if (c === '/' && next === '/') { + const end = text.indexOf('\n', i); + i = end < 0 ? text.length : end; + continue; + } + if (c === '/' && next === '*') { + const end = text.indexOf('*/', i + 2); + const stop = end < 0 ? text.length : end + 2; + out += text.slice(i, stop).replace(/[^\n]/g, ' '); + i = stop; + continue; + } + if (c === '"' || c === "'" || c === '`') { + let j = i + 1; + while (j < text.length && text[j] !== c) j += text[j] === '\\' ? 2 : 1; + out += text.slice(i, j + 1); + i = j + 1; + continue; + } + out += c; + i++; + } + return out; +} + /** Counts keyed by the server-relative POSIX path, files without any left out. */ export function scan(serverDir, check) { const dir = join(serverDir, check.root); diff --git a/server/scripts/test-mocks-baseline.json b/server/scripts/test-mocks-baseline.json index 387c434f4..14f790d16 100644 --- a/server/scripts/test-mocks-baseline.json +++ b/server/scripts/test-mocks-baseline.json @@ -19,7 +19,6 @@ "tests/e2e/feeds.e2e.test.ts": 1, "tests/e2e/files.e2e.test.ts": 1, "tests/e2e/google-quota.e2e.test.ts": 1, - "tests/e2e/harness.ts": 1, "tests/e2e/journey.e2e.test.ts": 1, "tests/e2e/mailer.e2e.test.ts": 1, "tests/e2e/maps.e2e.test.ts": 1, @@ -54,8 +53,6 @@ "tests/e2e/trips.e2e.test.ts": 1, "tests/e2e/vacay.e2e.test.ts": 1, "tests/e2e/weather.e2e.test.ts": 1, - "tests/helpers/db-mock.ts": 3, - "tests/helpers/test-db.ts": 1, "tests/integration/admin.test.ts": 1, "tests/integration/airtrail-sync-request-context.test.ts": 1, "tests/integration/api-docs.test.ts": 1, diff --git a/server/scripts/test-mocks-ratchet.mjs b/server/scripts/test-mocks-ratchet.mjs index 75348b46a..afbd4b5bd 100644 --- a/server/scripts/test-mocks-ratchet.mjs +++ b/server/scripts/test-mocks-ratchet.mjs @@ -16,24 +16,25 @@ * to the code under test; the remaining `src/db/database` * mocks are the legacy this counts down * - * Every `vi.mock(` / `vi.doMock(` whose path ends in one of those modules - * counts, per file, against scripts/test-mocks-baseline.json; a file without + * Every `vi.mock(` / `vi.doMock(` outside a comment whose path (a string, or + * `import('...')`) ends in one of those modules counts, per file, against scripts/test-mocks-baseline.json; a file without * an entry may hold none (scripts/lib/count-ratchet.mjs has the rules). * * npm run lint:test-mocks check against the baseline (CI) * npm run lint:test-mocks -- --update lower the counts; never raises or adds an entry */ -import { cliMain } from './lib/count-ratchet.mjs'; +import { cliMain, stripComments } from './lib/count-ratchet.mjs'; -const MOCK_CALL = /\bvi\.(?:mock|doMock)\(\s*(['"`])([^'"`]+)\1/g; +// The path as a string, or wrapped in import(), the typed form vi.mock also takes. +const MOCK_CALL = /\bvi\.(?:mock|doMock)\(\s*(?:import\(\s*)?(['"`])([^'"`]+)\1/g; /** A mocked path that names one of the held modules, with or without an extension. */ const HELD_TARGET = /(?:^|\/)src\/(?:config|websocket|db(?:\/[^'"`]*)?|nest\/realtime(?:\/[^'"`]*)?)$/; -/** The module mocks of held targets in one file's text. */ +/** The module mocks of held targets in one file's text; a mock quoted in a comment does not count. */ export function countHeldMocks(text) { let n = 0; - for (const match of text.matchAll(MOCK_CALL)) { + for (const match of stripComments(text).matchAll(MOCK_CALL)) { const target = match[2].replace(/\.(?:[cm]?[jt]s)$/, ''); if (HELD_TARGET.test(target)) n++; } diff --git a/server/scripts/test-new-service-baseline.json b/server/scripts/test-new-service-baseline.json index 3f75679d0..9f3b986ae 100644 --- a/server/scripts/test-new-service-baseline.json +++ b/server/scripts/test-new-service-baseline.json @@ -1,16 +1,13 @@ { "counts": { "tests/helpers/accommodations-service.ts": 8, - "tests/helpers/budget-repos.ts": 1, - "tests/helpers/fake-realtime.ts": 1, "tests/helpers/maps-service.ts": 1, "tests/helpers/notifications.ts": 10, "tests/helpers/place-import.ts": 1, "tests/helpers/plugin-host.ts": 45, "tests/helpers/sessions.ts": 1, - "tests/helpers/share-repos.ts": 1, "tests/helpers/storage-fixture.ts": 1, - "tests/helpers/test-addons.ts": 2, + "tests/helpers/test-addons.ts": 1, "tests/helpers/test-app.ts": 1, "tests/helpers/test-uow.ts": 3, "tests/integration/dayReorder.test.ts": 3, @@ -59,7 +56,7 @@ "tests/unit/nest/budget.service.db.test.ts": 6, "tests/unit/nest/budget.service.test.ts": 1, "tests/unit/nest/calendar.service.test.ts": 6, - "tests/unit/nest/categories.service.test.ts": 2, + "tests/unit/nest/categories.service.test.ts": 1, "tests/unit/nest/channel-override.test.ts": 2, "tests/unit/nest/charging.test.ts": 6, "tests/unit/nest/collab.service.test.ts": 6, @@ -122,7 +119,7 @@ "tests/unit/nest/place-enrichment.description.test.ts": 1, "tests/unit/nest/place-enrichment.service.test.ts": 1, "tests/unit/nest/place-photos/place-photo-cache.job.test.ts": 2, - "tests/unit/nest/place-photos/place-photo-cache.service.test.ts": 2, + "tests/unit/nest/place-photos/place-photo-cache.service.test.ts": 1, "tests/unit/nest/place-regions-repair.test.ts": 1, "tests/unit/nest/place-shadow.service.test.ts": 1, "tests/unit/nest/places.directions.test.ts": 6, @@ -144,7 +141,7 @@ "tests/unit/nest/roadtrip-search.test.ts": 3, "tests/unit/nest/roadtrip.service.test.ts": 1, "tests/unit/nest/route-usage.service.test.ts": 1, - "tests/unit/nest/school-holidays.service.test.ts": 2, + "tests/unit/nest/school-holidays.service.test.ts": 1, "tests/unit/nest/sessions/sessions.service.test.ts": 1, "tests/unit/nest/settings.service.test.ts": 1, "tests/unit/nest/share.service.test.ts": 3, @@ -157,7 +154,7 @@ "tests/unit/nest/synology.service.test.ts": 1, "tests/unit/nest/system-notices.branch-coverage.test.ts": 1, "tests/unit/nest/system-notices.service.test.ts": 1, - "tests/unit/nest/tags.service.test.ts": 2, + "tests/unit/nest/tags.service.test.ts": 1, "tests/unit/nest/thumbnail.service.test.ts": 1, "tests/unit/nest/todo.service.test.ts": 5, "tests/unit/nest/token.service.test.ts": 1, diff --git a/server/scripts/test-new-service-ratchet.mjs b/server/scripts/test-new-service-ratchet.mjs index 780a07141..c75c3cdaa 100644 --- a/server/scripts/test-new-service-ratchet.mjs +++ b/server/scripts/test-new-service-ratchet.mjs @@ -11,8 +11,9 @@ * test-module.ts: createTestModule with overrides), and a booted app hands * the rest out through `app.get(...)`. * - * Every `new Service(` and `new Repository(` under tests/ counts, - * per file, against scripts/test-new-service-baseline.json; a file without an + * Every `new Service(` and `new Repository(` under tests/ outside a + * comment counts (an import alias or `new (Name)(` counts as the class it + * names), per file, against scripts/test-new-service-baseline.json; a file without an * entry may hold none (scripts/lib/count-ratchet.mjs has the rules). The * allowlist is derived, not kept by hand: a class whose constructor takes no * parameters (no constructor of its own, or an empty one, down its chain of @@ -25,9 +26,8 @@ */ import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs'; import { join } from 'node:path'; -import { cliMain } from './lib/count-ratchet.mjs'; +import { cliMain, stripComments } from './lib/count-ratchet.mjs'; -const NEW_CALL = /\bnew\s+([A-Z][A-Za-z0-9_]*(?:Service|Repository))\s*[(<]/g; const CLASS_DECL = /\bclass\s+([A-Za-z_$][\w$]*)(?:\s*<[^{]*?>)?(?:\s+extends\s+([A-Za-z_$][\w$.]*))?[^{]*\{/g; /** The trees whose classes the allowlist is derived from, relative to the server root. */ @@ -119,10 +119,35 @@ export function zeroArgClasses(classes) { return new Set([...byName.keys()].filter((name) => resolve(name))); } -/** The hand-built services and repositories in one file's text, `allowed` left out. */ +// `new X(`, `new X(` and `new (X)(`: the name is checked after aliases resolve. +const ANY_NEW = /\bnew(?:\s+|\s*\(\s*)([A-Za-z_$][\w$]*)\s*\)?\s*[(<]/g; +// `import { DaysService as Days }` (and `type`-less re-imports of the same shape). +const IMPORT_BLOCK = /\bimport\s+(?:type\s+)?\{([^}]*)\}\s*from\b/g; +const ALIAS = /\b([A-Za-z_$][\w$]*)\s+as\s+([A-Za-z_$][\w$]*)/g; +const SERVICE_NAME = /^[A-Z][A-Za-z0-9_]*(?:Service|Repository)$/; + +/** Local name -> imported name, for every renamed import in the text. */ +function importAliases(text) { + const aliases = new Map(); + for (const block of text.matchAll(IMPORT_BLOCK)) { + for (const m of block[1].matchAll(ALIAS)) aliases.set(m[2], m[1]); + } + return aliases; +} + +/** + * The hand-built services and repositories in one file's text, `allowed` left + * out. Comments do not count; an import alias (`DaysService as Days`) and a + * parenthesised class (`new (DaysService)(`) count as the class they name. + */ export function countNewServices(text, allowed = new Set()) { + const code = stripComments(text); + const aliases = importAliases(code); let n = 0; - for (const match of text.matchAll(NEW_CALL)) if (!allowed.has(match[1])) n++; + for (const match of code.matchAll(ANY_NEW)) { + const name = aliases.get(match[1]) ?? match[1]; + if (SERVICE_NAME.test(name) && !allowed.has(name)) n++; + } return n; } diff --git a/server/tests/unit/scripts/test-architecture-ratchets.test.ts b/server/tests/unit/scripts/test-architecture-ratchets.test.ts index 1bf451551..b20378e60 100644 --- a/server/tests/unit/scripts/test-architecture-ratchets.test.ts +++ b/server/tests/unit/scripts/test-architecture-ratchets.test.ts @@ -77,6 +77,19 @@ describe('lint:test-mocks', () => { expect(countHeldMocks(text)).toBe(0); }); + it('TEST-RATCHET-005: counts the import() form of a mock, and not a mock quoted in a comment', () => { + const text = [ + `${MOCK}import('../../src/config'), () => ({}));`, + `${DO_MOCK}import("../../src/db/database"), async () => ({}));`, + '/**', + ` * ${MOCK}'../../src/db/database', async () =>`, + ' */', + `// ${MOCK}'../../src/config', () => ({}));`, + `const path = "// not a comment"; ${MOCK}'../../src/nest/realtime/realtime.service', () => ({}));`, + ].join('\n'); + expect(countHeldMocks(text)).toBe(3); + }); + it('TEST-RATCHET-003: fails a new mock, passes at the baseline and fails a stale entry until --update', () => { const mock = `${MOCK}'../../src/config', () => ({}));\n`; const grown = run( @@ -151,6 +164,19 @@ describe('lint:test-new-service', () => { expect(countNewServices(text, new Set(['RealtimeService']))).toBe(3); }); + it('TEST-RATCHET-016: an import alias and a parenthesised class count as the service they name, a comment does not', () => { + const text = [ + "import { DaysService as Days, TodoService } from '../../src/nest/days/days.service';", + "import { RealtimeService as Rt } from '../../src/nest/realtime/realtime.service';", + `const a = ${NEW}Days(repo, uow);`, + `const b = ${['ne', 'w'].join('')}(TodoService)(permissions);`, + `const c = ${NEW}Rt();`, + `// const d = ${NEW}DaysService(repo, uow);`, + `const e = ${NEW}Map();`, + ].join('\n'); + expect(countNewServices(text, new Set(['RealtimeService']))).toBe(2); + }); + it('TEST-RATCHET-013: derives the allowlist from src/ and tests/helpers/ when it runs', () => { const files = { 'src/plain.service.ts': 'export class PlainService { ping() {} }',