mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-21 14:36:31 +00:00
fix(client): lint:skips counts skips with any title and Playwright's fixme
A skip only counted when its first argument was a string or template literal; anything else was taken for a Playwright run-time condition. So it.skip(name, fn) in a table loop, describe.skip(TITLE, fn) or test.skip(true, 'why') switched tests off without the baseline noticing, and test.fixme was not matched at all. The condition form is now recognised in Playwright specs under e2e/ only, and even there a call counts when it declares a test (a body as second argument), is called bare inside a test or group, or has the literal true as its condition. Under vitest every .skip counts. fixme is a skip. Nothing in the tree uses these shapes today, so the baseline stays at 35.
This commit is contained in:
+1
-1
@@ -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 <file>, 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)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user