From 4062b314cfa2a225626e0e404588fd96fcf18698 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 12:58:09 +0200 Subject: [PATCH] fix(client): lint:skips counts skipIf(true) and runIf(false) skipIf and runIf were left out as run-time skips, which they are with a real condition. With the literal true (skipIf) or false (runIf) they switch a test off for good and gave a free way past the ratchet, while the same file already counted Playwright's test.skip(true, 'why') for that reason. Both now count as skips; nothing in the client uses them today, so the counts stay as they are. --- client/CLAUDE.md | 2 +- client/scripts/lib/skips.mjs | 8 +++++++- client/scripts/skip-lint.mjs | 5 +++-- client/tests/unit/scripts/skip-lint.test.ts | 16 ++++++++++++++++ 4 files changed, 27 insertions(+), 4 deletions(-) diff --git a/client/CLAUDE.md b/client/CLAUDE.md index 29cf71766..32aeb884a 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 .ts/.tsx/.mjs/.css file under src/, tests/, e2e/ and scripts/ 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/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 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, but skipIf(true) and runIf(false) count as skips) 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 3998a5334..ade56a3c1 100644 --- a/client/scripts/lib/skips.mjs +++ b/client/scripts/lib/skips.mjs @@ -56,7 +56,8 @@ function switchesOff(call, file) { * 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. + * switchesOff); neither do skipIf and runIf, unless their condition is the + * literal that always skips: skipIf(true), runIf(false). * * only: .only anywhere, which silently drops every other test in the file. * @@ -78,6 +79,11 @@ export function modifiers(source, file) { const call = ts.isCallExpression(node.parent) && node.parent.expression === node ? node.parent : null; // test.skip.each(table)(...) declares skipped tests without being called directly. if (!call || switchesOff(call, file)) skipped.push(at(node)); + } else if (name === 'skipIf' || name === 'runIf') { + // skipIf(true) and runIf(false) are a skip in disguise: the condition can never change. + const call = ts.isCallExpression(node.parent) && node.parent.expression === node ? node.parent : null; + const never = name === 'skipIf' ? ts.SyntaxKind.TrueKeyword : ts.SyntaxKind.FalseKeyword; + if (call?.arguments[0]?.kind === never) skipped.push(at(node)); } } else if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && SHORTHAND.has(node.expression.text)) { skipped.push(at(node.expression)); diff --git a/client/scripts/skip-lint.mjs b/client/scripts/skip-lint.mjs index 7b6ec13d9..ad53fe1ae 100644 --- a/client/scripts/skip-lint.mjs +++ b/client/scripts/skip-lint.mjs @@ -8,8 +8,9 @@ * 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. * 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. + * time is left out: skipIf and runIf, and in a Playwright spec + * test.skip(condition, 'why'), unless the condition is a literal that always + * skips (skipIf(true), runIf(false), test.skip(true, 'why')). * Covers the vitest tests under src/ and tests/ and the Playwright specs * under e2e/. * diff --git a/client/tests/unit/scripts/skip-lint.test.ts b/client/tests/unit/scripts/skip-lint.test.ts index 70c1509da..567d2e644 100644 --- a/client/tests/unit/scripts/skip-lint.test.ts +++ b/client/tests/unit/scripts/skip-lint.test.ts @@ -142,6 +142,22 @@ describe('lint:skips', () => { expect(run({}, { 'e2e/c.spec.ts': conditional })).toBe(0); }); + it('SKIPS-012: counts skipIf(true) and runIf(false), whose condition can never change', () => { + const source = [ + "it.skipIf(true)('a', () => {})", + "describe.runIf(false)('b', () => {})", + "test.skipIf(true).each([1])('c %s', () => {})", + "it.skipIf(false)('d', () => {})", + "describe.runIf(true)('e', () => {})", + "it.skipIf(isCI)('f', () => {})", + ].join('\n'); + expect(modifiers(source, 'tests/a.test.ts').skipped.map((s: string) => s.split(':')[0])).toEqual(['1', '2', '3']); + expect( + run({}, { 'tests/b.test.ts': "it.skipIf(true)('a', () => {});\ndescribe.runIf(false)('b', () => {});\n" }) + ).toBe(1); + expect(tree.error.join('\n')).toMatch(/tests\/b\.test\.ts: 2 skipped or todo test\(s\), baseline 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);