diff --git a/client/CLAUDE.md b/client/CLAUDE.md index ecda33da0..6937dbc53 100644 --- a/client/CLAUDE.md +++ b/client/CLAUDE.md @@ -16,7 +16,7 @@ npm run lint:size # no source file or stylesheet past 1000 lines, no tes npm run lint:format # Prettier: every file outside scripts/format-baseline.json must be formatted, the list only shrinks (CI gate; npx prettier --write , then --update) npm run lint:layers # imports go downwards: components never import pages/ or mobile/, nothing under the views imports a view; per-file counts may only shrink (CI gate; --list, --update) npm run lint:offline # no view file (components/mobile/pages/hooks) imports src/api/ beyond scripts/offline-baseline.json, which only shrinks (CI gate; --list, --update) -npm run lint:skips # no .only anywhere; skipped/todo tests per file may only shrink against scripts/skip-baseline.json (CI gate; skipIf/runIf are fine) +npm run lint:skips # no .only anywhere; skipped/todo/fixme tests per file may only shrink against scripts/skip-baseline.json (CI gate; skipIf/runIf and Playwright's test.skip(condition, 'why') are fine) npm run test # vitest run (tests/** + co-located src/**/*.test.{ts,tsx}); also test:unit / test:integration / test:coverage npm run e2e # Playwright (CI runs --project=public --project=app; e2e:report opens the last report) npm run shots # Playwright screenshot project (shots:promote to accept) diff --git a/client/scripts/lib/skips.mjs b/client/scripts/lib/skips.mjs index 56864dc2b..118c1227f 100644 --- a/client/scripts/lib/skips.mjs +++ b/client/scripts/lib/skips.mjs @@ -21,17 +21,38 @@ function rootOf(node) { return ts.isIdentifier(node) ? node.text : null } +/** Playwright specs live under e2e/; everything else runs under vitest. */ +const isPlaywright = (file) => file.startsWith('e2e/') + const isTitle = (node) => Boolean(node) && (ts.isStringLiteralLike(node) || ts.isTemplateExpression(node)) +const isFunction = (node) => Boolean(node) && (ts.isArrowFunction(node) || ts.isFunctionExpression(node)) + +/** + * Whether a skip or fixme call switches tests off for good. Under vitest it + * always does: the first argument is the title, a literal or not. Playwright + * also knows a run-time form, test.skip(condition, 'why'), which only skips + * when the condition holds. A call counts there unless it is that form: it + * declares a test (a title first or a test body second), it has no argument + * (called bare, it skips the test or group it sits in), or its condition is + * the literal true. + */ +function switchesOff(call, file) { + const [first, second] = call.arguments + if (!isPlaywright(file) || !first || isTitle(first) || isFunction(second)) return true + return first.kind === ts.SyntaxKind.TrueKeyword +} /** * The tests a file switches off or narrows down, as `line: code`. * - * skipped: it.skip / describe.skip / test.todo and the x-shorthands, each - * declaring a test that never runs. A skip with a condition first - * (Playwright's test.skip(!seed.id, 'why'), or called bare inside a test) - * decides at run time and is no declaration; neither are skipIf and runIf. + * skipped: it.skip / describe.skip / test.todo / test.fixme and the + * x-shorthands, each switching off a test that never runs. Playwright's + * test.skip(!seed.id, 'why') decides at run time and does not count (see + * switchesOff); neither do skipIf and runIf. * * only: .only anywhere, which silently drops every other test in the file. + * + * file is the path from the client root: it decides vitest or Playwright. */ export function modifiers(source, file) { const kind = file.endsWith('.tsx') ? ts.ScriptKind.TSX : ts.ScriptKind.TS @@ -44,10 +65,10 @@ export function modifiers(source, file) { const name = node.name.text if (name === 'only') only.push(at(node)) else if (name === 'todo') skipped.push(at(node)) - else if (name === 'skip') { + else if (name === 'skip' || name === 'fixme') { const call = ts.isCallExpression(node.parent) && node.parent.expression === node ? node.parent : null - // test.skip.each(table)(...) declares skipped tests; test.skip('title', fn) declares one. - if (!call || isTitle(call.arguments[0])) skipped.push(at(node)) + // test.skip.each(table)(...) declares skipped tests without being called directly. + if (!call || switchesOff(call, file)) skipped.push(at(node)) } } else if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && SHORTHAND.has(node.expression.text)) { skipped.push(at(node.expression)) @@ -63,8 +84,8 @@ export function scan(root) { const skipped = {} const only = {} for (const path of listFiles(root, DIRS, accepts)) { - const found = modifiers(readText(path), path) const key = toKey(root, path) + const found = modifiers(readText(path), key) if (found.skipped.length) skipped[key] = found.skipped if (found.only.length) only[key] = found.only } diff --git a/client/scripts/skip-lint.mjs b/client/scripts/skip-lint.mjs index 9396fd5f8..5a8341967 100644 --- a/client/scripts/skip-lint.mjs +++ b/client/scripts/skip-lint.mjs @@ -7,9 +7,11 @@ * switched off and then forgotten (JourneyDetailPage.test.tsx has carried 35 * skipped blocks since April); the files that hold some are listed in * scripts/skip-baseline.json with their count, and a file may hold no more. - * A skip that decides at run time (skipIf, runIf, Playwright's - * test.skip(condition, 'why')) is not counted. Covers the vitest tests under - * src/ and tests/ and the Playwright specs under e2e/. + * Playwright's test.fixme counts as a skip. Only a skip that decides at run + * time is left out: skipIf, runIf, and in a Playwright spec + * test.skip(condition, 'why') unless the condition is the literal true. + * Covers the vitest tests under src/ and tests/ and the Playwright specs + * under e2e/. * * npm run lint:skips check against the baseline (CI) * npm run lint:skips -- --update lower the baseline to what the files hold now; diff --git a/client/tests/unit/scripts/skip-lint.test.ts b/client/tests/unit/scripts/skip-lint.test.ts index 115580bb1..70c1509da 100644 --- a/client/tests/unit/scripts/skip-lint.test.ts +++ b/client/tests/unit/scripts/skip-lint.test.ts @@ -45,8 +45,6 @@ describe('lint:skips', () => { it('SKIPS-002: leaves run-time skips, skipIf/runIf, comments and strings alone', () => { const source = [ - "test.skip(!seed.collectionId, 'collections addon unavailable')", - "test('x', async () => { test.skip() })", "it.skipIf(process.platform === 'win32')('y', () => {})", "describe.runIf(hasS3)('z', () => {})", "// it.skip('commented out', () => {})", @@ -54,6 +52,13 @@ describe('lint:skips', () => { 'ctx.skip()', ].join('\n'); expect(modifiers(source, 'x.test.ts')).toEqual({ skipped: [], only: [] }); + const playwright = [ + "test.skip(!seed.collectionId, 'collections addon unavailable')", + "test('x', async ({ page }) => { test.fixme(await page.isClosed(), 'flaky') })", + "test.skip(({ browserName }) => browserName === 'webkit', 'no webkit')", + "test.skip(false, 'never')", + ].join('\n'); + expect(modifiers(playwright, 'e2e/x.spec.ts')).toEqual({ skipped: [], only: [] }); }); it('SKIPS-003: finds .only on any runner', () => { @@ -88,6 +93,55 @@ describe('lint:skips', () => { expect(JSON.parse(readFileSync(tree.path(BASELINE), 'utf8'))).toEqual({ 'src/a.test.ts': 2 }); }); + it('SKIPS-009: counts every vitest skip, whatever its title is', () => { + const source = [ + "const T = 'a'", + 'describe.skip(T, () => {})', + 'it.skip(name, () => {})', + "it.skip(t('x'), () => {})", + 'test.skip(!ready, () => {})', + ].join('\n'); + expect(modifiers(source, 'tests/x.test.ts').skipped.map((s: string) => s.split(':')[0])).toEqual([ + '2', + '3', + '4', + '5', + ]); + }); + + it('SKIPS-010: counts the Playwright skips that do not depend on a condition', () => { + const source = [ + "test.skip(true, 'switched off')", + "test('x', async () => { test.skip() })", + "test.fixme('t', async () => {})", + 'test.fixme(title, async () => {})', + 'test.skip(title, async ({ page }) => {})', + "test.describe.fixme('group', () => {})", + "test.describe('group', () => { test.fixme() })", + ].join('\n'); + expect(modifiers(source, 'e2e/a.spec.ts').skipped.map((s: string) => s.split(':')[0])).toEqual([ + '1', + '2', + '3', + '4', + '5', + '6', + '7', + ]); + }); + + it('SKIPS-011: fails a variable-title skip and an unconditional Playwright skip', () => { + expect(run({}, { 'tests/b.test.ts': "const name = 'x';\nit.skip(name, () => {});\n" })).toBe(1); + expect(tree.error.join('\n')).toMatch(/tests\/b\.test\.ts: 1 skipped or todo test\(s\), baseline 0/); + tree.remove(); + const e2e = "test('a', async () => { test.skip() });\ntest.fixme('b', async () => {});\n"; + expect(run({}, { 'e2e/c.spec.ts': e2e })).toBe(1); + expect(tree.error.join('\n')).toMatch(/e2e\/c\.spec\.ts: 2 skipped or todo test\(s\), baseline 0/); + tree.remove(); + const conditional = "test('a', async () => { test.skip(!process.env.S3, 'needs S3') });\n"; + expect(run({}, { 'e2e/c.spec.ts': conditional })).toBe(0); + }); + it('SKIPS-008: a missing baseline or test directory stops the check', () => { tree = ratchetTree({ 'src/a.test.ts': PASSING, 'tests/b.test.ts': PASSING, 'e2e/c.spec.ts': PASSING }); expect(() => check({ root: tree.root, baselinePath: tree.path(BASELINE), ...tree.out })).toThrow(RatchetError);