From 3afc074b12d4daea287c0d101960eccfc736166b Mon Sep 17 00:00:00 2001 From: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz> Date: Tue, 18 Aug 2026 20:04:34 -0400 Subject: [PATCH] test(e2e): cover structural bootstrap control flow Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz> --- desktop/scripts/check-e2e-bootstrap.mjs | 405 +++++++++++++------ desktop/scripts/check-e2e-bootstrap.test.mjs | 158 ++++++-- web/scripts/check-e2e-bootstrap.mjs | 405 +++++++++++++------ web/scripts/check-e2e-bootstrap.test.mjs | 158 ++++++-- 4 files changed, 830 insertions(+), 296 deletions(-) diff --git a/desktop/scripts/check-e2e-bootstrap.mjs b/desktop/scripts/check-e2e-bootstrap.mjs index 3793b2c85c..665cf412b9 100644 --- a/desktop/scripts/check-e2e-bootstrap.mjs +++ b/desktop/scripts/check-e2e-bootstrap.mjs @@ -3,25 +3,19 @@ import path from "node:path"; import ts from "typescript"; const SPEC_PATTERN = /\.(spec|perf)\.ts$/; -const CANONICAL_TEST_HELPER = "../helpers/test"; const exempt = new Set(["agents-everywhere.live.spec.ts"]); - -function namedImport(declaration, name) { - return declaration.importClause?.namedBindings?.elements.some( - (element) => - element.name.text === name && element.propertyName === undefined, - ); -} +const TEST_MEMBERS = new Set(["only", "skip", "fixme", "fail"]); +const CONTAINERS = new Set(["forEach", "map"]); function callbackOf(call) { - const candidate = call.arguments.at(-1); - return candidate && - (ts.isArrowFunction(candidate) || ts.isFunctionExpression(candidate)) - ? candidate + const callback = call.arguments.at(-1); + return callback && + (ts.isArrowFunction(callback) || ts.isFunctionExpression(callback)) + ? callback : undefined; } -function testMember(call) { +function memberOf(call) { return ts.isPropertyAccessExpression(call.expression) && ts.isIdentifier(call.expression.expression) && call.expression.expression.text === "test" @@ -29,137 +23,312 @@ function testMember(call) { : undefined; } -function isTestRegistration(call) { - if (ts.isIdentifier(call.expression)) return call.expression.text === "test"; - const member = testMember(call); +function helperImport(file, filename, canonicalHelperPath) { + for (const statement of file.statements) { + if ( + !ts.isImportDeclaration(statement) || + !ts.isStringLiteral(statement.moduleSpecifier) + ) + continue; + const resolved = path.resolve( + path.dirname(filename), + `${statement.moduleSpecifier.text}.ts`, + ); + const canonical = canonicalHelperPath + ? resolved === canonicalHelperPath + : statement.moduleSpecifier.text === "../helpers/test"; + if (!canonical) continue; + const elements = statement.importClause?.namedBindings; + if (!elements || !ts.isNamedImports(elements)) return undefined; + const names = new Map( + elements.elements.map((element) => [ + element.name.text, + element.propertyName?.text ?? element.name.text, + ]), + ); + if ( + names.get("test") === "test" && + names.get("bootstrapE2ePage") === "bootstrapE2ePage" + ) + return { test: "test", bootstrap: "bootstrapE2ePage" }; + } +} + +function hasShadowingDeclaration(file, canonical) { + let shadowed = false; + const visit = (node) => { + if (shadowed) return; + if ( + (ts.isVariableDeclaration(node) || + ts.isFunctionDeclaration(node) || + ts.isClassDeclaration(node)) && + node.name && + ts.isIdentifier(node.name) && + (node.name.text === canonical.test || + node.name.text === canonical.bootstrap) + ) { + shadowed = true; + return; + } + if ( + (ts.isArrowFunction(node) || + ts.isFunctionExpression(node) || + ts.isFunctionDeclaration(node)) && + node.parameters.some( + (parameter) => + ts.isIdentifier(parameter.name) && + (parameter.name.text === canonical.test || + parameter.name.text === canonical.bootstrap), + ) + ) { + shadowed = true; + return; + } + ts.forEachChild(node, visit); + }; + for (const statement of file.statements) { + if (ts.isImportDeclaration(statement)) continue; + visit(statement); + } + return shadowed; +} + +function collectModuleHelpers(file) { + const helpers = new Map(); + for (const statement of file.statements) { + if (ts.isFunctionDeclaration(statement) && statement.name && statement.body) + helpers.set(statement.name.text, statement); + if (ts.isVariableStatement(statement)) { + for (const declaration of statement.declarationList.declarations) { + if ( + ts.isIdentifier(declaration.name) && + declaration.initializer && + (ts.isArrowFunction(declaration.initializer) || + ts.isFunctionExpression(declaration.initializer)) + ) + helpers.set(declaration.name.text, declaration.initializer); + } + } + } + return helpers; +} + +function awaitedCall(statement) { + const expression = + ts.isExpressionStatement(statement) || ts.isReturnStatement(statement) + ? statement.expression + : ts.isVariableStatement(statement) && + statement.declarationList.declarations.length === 1 + ? statement.declarationList.declarations[0].initializer + : undefined; + return expression && + ts.isAwaitExpression(expression) && + ts.isCallExpression(expression.expression) + ? expression.expression + : undefined; +} + +function isAwaitedCall(statement, name) { + const call = awaitedCall(statement); return ( - member !== undefined && - ![ - "beforeEach", - "afterEach", - "beforeAll", - "afterAll", - "describe", - "use", - "setTimeout", - "slow", - "step", - "skip", - "fixme", - "fail", - ].includes(member) + call && ts.isIdentifier(call.expression) && call.expression.text === name ); } -function collectFunctionDeclarations(file) { - const functions = new Map(); - const visit = (node) => { - if (ts.isFunctionDeclaration(node) && node.name) - functions.set(node.name.text, node); - if ( - ts.isVariableDeclaration(node) && - ts.isIdentifier(node.name) && - node.initializer && - (ts.isArrowFunction(node.initializer) || - ts.isFunctionExpression(node.initializer)) - ) { - functions.set(node.name.text, node.initializer); - } - ts.forEachChild(node, visit); - }; - visit(file); - return functions; +function isStaticallyEmptyLoop(statement) { + if (ts.isForStatement(statement)) + return statement.condition?.kind === ts.SyntaxKind.FalseKeyword; + if (ts.isWhileStatement(statement)) + return statement.expression.kind === ts.SyntaxKind.FalseKeyword; + if (ts.isForOfStatement(statement)) { + const expression = statement.expression; + return ( + (ts.isArrayLiteralExpression(expression) && + !expression.elements.length) || + (ts.isStringLiteral(expression) && !expression.text.length) + ); + } + if (ts.isForInStatement(statement)) + return ( + ts.isObjectLiteralExpression(statement.expression) && + !statement.expression.properties.length + ); + return false; } -function callbackBootstraps(callback, functions) { - const visitedFunctions = new Set(); - const inspect = (node) => { - let found = false; - const visit = (child) => { +function statementBootstraps(statement, canonical, helpers, seen) { + if (isAwaitedCall(statement, canonical.bootstrap)) return true; + const call = awaitedCall(statement); + if (call && ts.isIdentifier(call.expression)) { + const helperName = call.expression.text; + const helper = helpers.get(helperName); + if (helper && !seen.has(helperName)) { + seen.add(helperName); + if (callbackBootstraps(helper, canonical, helpers, seen)) return true; + } + } + if (ts.isBlock(statement)) + return statement.statements.some((child) => + statementBootstraps(child, canonical, helpers, seen), + ); + if (ts.isTryStatement(statement)) + return ( + statementBootstraps(statement.tryBlock, canonical, helpers, seen) || + Boolean( + statement.finallyBlock && + statementBootstraps(statement.finallyBlock, canonical, helpers, seen), + ) + ); + if ( + (ts.isForStatement(statement) || + ts.isForInStatement(statement) || + ts.isForOfStatement(statement) || + ts.isWhileStatement(statement) || + ts.isDoStatement(statement)) && + !isStaticallyEmptyLoop(statement) + ) + return statementBootstraps(statement.statement, canonical, helpers, seen); + return false; +} + +function callbackBootstraps(callback, canonical, helpers, seen = new Set()) { + const statements = ts.isBlock(callback.body) + ? callback.body.statements + : [ts.factory.createExpressionStatement(callback.body)]; + return statements.some((statement) => + statementBootstraps(statement, canonical, helpers, seen), + ); +} + +function makeScope(parent) { + return { parent, hooks: [], tests: [], children: [] }; +} + +function scanFile(file, canonical) { + const helpers = collectModuleHelpers(file); + const root = makeScope(undefined); + const isTestCall = (call) => + ts.isIdentifier(call.expression) && call.expression.text === canonical.test; + const isMember = (call, name) => memberOf(call) === name; + + const scanStatements = (statements, scope) => { + for (const statement of statements) { + if (ts.isBlock(statement)) { + scanStatements(statement.statements, scope); + continue; + } if ( - ts.isAwaitExpression(child) && - ts.isCallExpression(child.expression) && - ts.isIdentifier(child.expression.expression) && - child.expression.expression.text === "bootstrapE2ePage" + ts.isForStatement(statement) || + ts.isForInStatement(statement) || + ts.isForOfStatement(statement) ) { - found = true; - return; + scanStatements( + ts.isBlock(statement.statement) + ? statement.statement.statements + : [statement.statement], + scope, + ); + continue; } - if (ts.isCallExpression(child) && ts.isIdentifier(child.expression)) { - const declaration = functions.get(child.expression.text); - if (declaration && !visitedFunctions.has(child.expression.text)) { - visitedFunctions.add(child.expression.text); - if (inspect(declaration.body)) found = true; - } + if ( + !ts.isExpressionStatement(statement) || + !ts.isCallExpression(statement.expression) + ) + continue; + const call = statement.expression; + const callback = callbackOf(call); + if (isMember(call, "describe") && callback) { + const child = makeScope(scope); + scope.children.push(child); + if (ts.isBlock(callback.body)) + scanStatements(callback.body.statements, child); + continue; + } + if (isMember(call, "beforeEach") && callback) { + scope.hooks.push(callbackBootstraps(callback, canonical, helpers)); + continue; + } + if ( + isTestCall(call) || + (memberOf(call) && TEST_MEMBERS.has(memberOf(call))) + ) { + if (callback) + scope.tests.push(callbackBootstraps(callback, canonical, helpers)); + continue; + } + if ( + ts.isPropertyAccessExpression(call.expression) && + CONTAINERS.has(call.expression.name.text) && + callback && + ts.isArrayLiteralExpression(call.expression.expression) + ) { + if (ts.isBlock(callback.body)) + scanStatements(callback.body.statements, scope); } - if (!found) ts.forEachChild(child, visit); - }; - ts.forEachChild(node, visit); - return found; - }; - return inspect(callback.body); -} - -function scanFile(file, functions) { - const hooks = []; - const tests = []; - const visit = (node) => { - if (ts.isCallExpression(node)) { - const callback = callbackOf(node); - const member = testMember(node); - if (member === "beforeEach" && callback) - hooks.push(callbackBootstraps(callback, functions)); - else if (isTestRegistration(node) && callback) - tests.push(callbackBootstraps(callback, functions)); } - ts.forEachChild(node, visit); }; - visit(file); - return { hooks, tests }; + scanStatements(file.statements, root); + return root; } -export function checkSource(source, filename = "fixture.spec.ts") { +function everyTestCovered(scope, inheritedHook = false) { + const coveredByHook = inheritedHook || scope.hooks.some(Boolean); + return ( + scope.tests.every((covered) => coveredByHook || covered) && + scope.children.every((child) => everyTestCovered(child, coveredByHook)) + ); +} + +function testCount(scope) { + return ( + scope.tests.length + + scope.children.reduce((count, child) => count + testCount(child), 0) + ); +} + +export function checkSource( + source, + filename = "fixture.spec.ts", + canonicalHelperPath, +) { const file = ts.createSourceFile( filename, source, ts.ScriptTarget.Latest, true, ); - const helperImports = file.statements.filter( - (statement) => - ts.isImportDeclaration(statement) && - ts.isStringLiteral(statement.moduleSpecifier) && - statement.moduleSpecifier.text === CANONICAL_TEST_HELPER, - ); - if ( - !helperImports.some((declaration) => namedImport(declaration, "test")) || - !helperImports.some((declaration) => - namedImport(declaration, "bootstrapE2ePage"), - ) - ) { - return "must directly import unaliased test and bootstrapE2ePage from ../helpers/test"; - } + const canonical = helperImport(file, filename, canonicalHelperPath); + if (!canonical) + return "must directly import unaliased test and bootstrapE2ePage from the canonical tests/helpers/test module"; + if (hasShadowingDeclaration(file, canonical)) + return "must not shadow canonical test or bootstrapE2ePage imports"; + const scope = scanFile(file, canonical); + if (!testCount(scope)) return "does not register a browser test"; + if (!everyTestCovered(scope)) + return "must structurally await bootstrapE2ePage for every test, directly or through an applicable ancestor test.beforeEach"; +} - const { hooks, tests } = scanFile(file, collectFunctionDeclarations(file)); - if (!tests.length) return "does not register a browser test"; - if (!hooks.some(Boolean) && !tests.some(Boolean)) { - return "must await bootstrapE2ePage on every test path, directly or through a registered test.beforeEach"; - } +function specFiles(directory) { + return fs.readdirSync(directory, { withFileTypes: true }).flatMap((entry) => { + const child = path.join(directory, entry.name); + if (entry.isDirectory()) return specFiles(child); + return entry.isFile() && SPEC_PATTERN.test(entry.name) ? [child] : []; + }); } export function runCheck(projectRoot) { - const violations = []; - for (const name of fs - .readdirSync(path.join(projectRoot, "tests/e2e")) - .filter((name) => SPEC_PATTERN.test(name))) { - if (exempt.has(name)) continue; + const e2eRoot = path.join(projectRoot, "tests/e2e"); + const canonicalHelperPath = path.join(projectRoot, "tests/helpers/test.ts"); + return specFiles(e2eRoot).flatMap((filename) => { + const relative = path.relative(e2eRoot, filename); + if (exempt.has(relative)) return []; const violation = checkSource( - fs.readFileSync(path.join(projectRoot, "tests/e2e", name), "utf8"), - name, + fs.readFileSync(filename, "utf8"), + filename, + canonicalHelperPath, ); - if (violation) violations.push(`${name}: ${violation}`); - } - return violations; + return violation ? [`${relative}: ${violation}`] : []; + }); } if (process.argv[1] === import.meta.filename) { diff --git a/desktop/scripts/check-e2e-bootstrap.test.mjs b/desktop/scripts/check-e2e-bootstrap.test.mjs index e7b84afa0a..6d0fc560e4 100644 --- a/desktop/scripts/check-e2e-bootstrap.test.mjs +++ b/desktop/scripts/check-e2e-bootstrap.test.mjs @@ -1,67 +1,165 @@ import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import test from "node:test"; const projects = [ ["desktop", "../scripts/check-e2e-bootstrap.mjs"], ["web", "../scripts/check-e2e-bootstrap.mjs"], ]; - -const validDirect = ` - import { test, bootstrapE2ePage } from "../helpers/test"; - test("uses bootstrap", async ({ page }) => { await bootstrapE2ePage(page); }); -`; -const validBeforeEach = ` - import { test, bootstrapE2ePage } from "../helpers/test"; - test.beforeEach(async ({ page }) => { await bootstrapE2ePage(page); }); - test("uses registered hook", async ({ page }) => { await page.title(); }); -`; -const validBeforeNavigate = ` - import { test, bootstrapE2ePage } from "../helpers/test"; - test("draft seed", async ({ page }) => { - await bootstrapE2ePage(page, { beforeNavigate: async () => page.addInitScript(() => {}) }); - }); -`; +const imports = 'import { test, bootstrapE2ePage } from "../helpers/test";'; +const spec = (body) => + `${imports}\ntest("x", async ({ page }) => { ${body} });`; for (const [project, checkerPath] of projects) { - const { checkSource } = await import(checkerPath); + const { checkSource, runCheck } = await import(checkerPath); + const accepts = (source) => assert.equal(checkSource(source), undefined); + const rejects = (source) => assert.notEqual(checkSource(source), undefined); + test(`${project}: accepts direct, hook, and beforeNavigate bootstrap patterns`, () => { - for (const source of [validDirect, validBeforeEach, validBeforeNavigate]) { - assert.equal(checkSource(source), undefined); - } + accepts(spec("await bootstrapE2ePage(page);")); + accepts( + `${imports}\ntest.beforeEach(async ({ page }) => { await bootstrapE2ePage(page); });\ntest("x", async ({ page }) => { await page.title(); });`, + ); + accepts( + spec( + "await bootstrapE2ePage(page, { beforeNavigate: async () => page.addInitScript(() => {}) });", + ), + ); + }); + + test(`${project}: accepts module helpers and ratified containers`, () => { + accepts( + `${imports}\nasync function open(page) { return await bootstrapE2ePage(page); }\ntest("x", async ({ page }) => { const response = await open(page); void response; });`, + ); + accepts( + spec( + "try { await bootstrapE2ePage(page); } finally { await page.title(); }", + ), + ); + accepts( + spec( + "try { await page.title(); } finally { await bootstrapE2ePage(page); }", + ), + ); + accepts( + spec("for (const attempt of [0, 1]) { await bootstrapE2ePage(page); }"), + ); + accepts( + spec( + "for (let attempt = 0; attempt < 2; attempt += 1) { await bootstrapE2ePage(page); }", + ), + ); }); for (const [name, source] of [ [ ".ts helper suffix", - `import { test, bootstrapE2ePage } from "../helpers/test.ts"; test("x", async () => {});`, + 'import { test, bootstrapE2ePage } from "../helpers/test.ts"; test("x", async () => {});', ], [ "import alias", - `import { test as e2eTest, bootstrapE2ePage } from "../helpers/test"; e2eTest("x", async () => { await bootstrapE2ePage(); });`, + 'import { test as e2eTest, bootstrapE2ePage } from "../helpers/test"; e2eTest("x", async () => { await bootstrapE2ePage(); });', ], [ "re-export", - `import { test, bootstrapE2ePage } from "./reexport"; test("x", async () => { await bootstrapE2ePage(); });`, + 'import { test, bootstrapE2ePage } from "./reexport"; test("x", async () => { await bootstrapE2ePage(); });', ], [ "dynamic import", - `const { test, bootstrapE2ePage } = await import("../helpers/test"); test("x", async () => { await bootstrapE2ePage(); });`, + 'const { test, bootstrapE2ePage } = await import("../helpers/test"); test("x", async () => { await bootstrapE2ePage(); });', ], [ "dead-code call", - `import { test, bootstrapE2ePage } from "../helpers/test"; async function never(page) { await bootstrapE2ePage(page); } test("x", async () => {});`, + `${imports}\nasync function never(page) { await bootstrapE2ePage(page); } test("x", async () => {});`, ], [ "comment", - `import { test, bootstrapE2ePage } from "../helpers/test"; // await bootstrapE2ePage(page)\ntest("x", async () => {});`, + `${imports}\n// await bootstrapE2ePage(page)\ntest("x", async () => {});`, ], [ "string", - `import { test, bootstrapE2ePage } from "../helpers/test"; const hint = "await bootstrapE2ePage(page)"; test("x", async () => {});`, + `${imports}\nconst hint = "await bootstrapE2ePage(page)"; test("x", async () => {});`, + ], + [ + "mixed safe and unsafe tests", + `${imports}\ntest("safe", async ({ page }) => { await bootstrapE2ePage(page); }); test("unsafe", async () => {});`, + ], + ["if-wrapped call", spec("if (true) { await bootstrapE2ePage(page); }")], + [ + "catch-only call", + spec( + "try { await page.title(); } catch { await bootstrapE2ePage(page); }", + ), + ], + [ + "zero-iteration false loop", + spec("for (; false;) { await bootstrapE2ePage(page); }"), + ], + [ + "zero-iteration empty iterable", + spec("for (const value of []) { await bootstrapE2ePage(page); }"), + ], + [ + "uncalled nested function", + spec("async function setup() { await bootstrapE2ePage(page); }"), + ], + [ + "unawaited helper", + `${imports}\nasync function open(page) { await bootstrapE2ePage(page); }\ntest("x", async ({ page }) => { open(page); });`, + ], + [ + "fake test.extend", + `${imports}\ntest.extend("x", async ({ page }) => { await bootstrapE2ePage(page); });`, + ], + [ + "misplaced describe hook", + `${imports}\ntest.describe("inside", () => { test.beforeEach(async ({ page }) => { await bootstrapE2ePage(page); }); test("covered", async () => {}); }); test("outside", async () => {});`, + ], + [ + "shadowed bootstrap", + `${imports}\ntest("x", async ({ page }) => { const bootstrapE2ePage = async () => {}; await bootstrapE2ePage(page); });`, ], ]) { - test(`${project}: rejects ${name}`, () => { - assert.notEqual(checkSource(source), undefined); - }); + test(`${project}: rejects ${name}`, () => rejects(source)); } + + test(`${project}: discovers nested specs with a canonical relative import`, () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "e2e-bootstrap-")); + try { + fs.mkdirSync(path.join(root, "tests/e2e/nested"), { recursive: true }); + fs.mkdirSync(path.join(root, "tests/helpers"), { recursive: true }); + fs.writeFileSync( + path.join(root, "tests/helpers/test.ts"), + "export {};\n", + ); + fs.writeFileSync( + path.join(root, "tests/e2e/nested/example.spec.ts"), + 'import { test, bootstrapE2ePage } from "../../helpers/test"; test("x", async ({ page }) => { await bootstrapE2ePage(page); });', + ); + assert.deepEqual(runCheck(root), []); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); +} + +for (const [project, checkerPath] of projects) { + const { checkSource } = await import(checkerPath); + test(`${project}: traverses canonical parameterized registration containers only`, () => { + assert.equal( + checkSource(`${imports} + for (const name of ["one", "two"]) test(name, async ({ page }) => { await bootstrapE2ePage(page); }); + ["three"].forEach((name) => test(name, async ({ page }) => { await bootstrapE2ePage(page); })); + ["four"].map((name) => test(name, async ({ page }) => { await bootstrapE2ePage(page); }));`), + undefined, + ); + assert.notEqual( + checkSource(`${imports} + function registerLater() { test("hidden", async ({ page }) => { await bootstrapE2ePage(page); }); } + test("unsafe", async () => {});`), + undefined, + ); + }); } diff --git a/web/scripts/check-e2e-bootstrap.mjs b/web/scripts/check-e2e-bootstrap.mjs index 3793b2c85c..665cf412b9 100644 --- a/web/scripts/check-e2e-bootstrap.mjs +++ b/web/scripts/check-e2e-bootstrap.mjs @@ -3,25 +3,19 @@ import path from "node:path"; import ts from "typescript"; const SPEC_PATTERN = /\.(spec|perf)\.ts$/; -const CANONICAL_TEST_HELPER = "../helpers/test"; const exempt = new Set(["agents-everywhere.live.spec.ts"]); - -function namedImport(declaration, name) { - return declaration.importClause?.namedBindings?.elements.some( - (element) => - element.name.text === name && element.propertyName === undefined, - ); -} +const TEST_MEMBERS = new Set(["only", "skip", "fixme", "fail"]); +const CONTAINERS = new Set(["forEach", "map"]); function callbackOf(call) { - const candidate = call.arguments.at(-1); - return candidate && - (ts.isArrowFunction(candidate) || ts.isFunctionExpression(candidate)) - ? candidate + const callback = call.arguments.at(-1); + return callback && + (ts.isArrowFunction(callback) || ts.isFunctionExpression(callback)) + ? callback : undefined; } -function testMember(call) { +function memberOf(call) { return ts.isPropertyAccessExpression(call.expression) && ts.isIdentifier(call.expression.expression) && call.expression.expression.text === "test" @@ -29,137 +23,312 @@ function testMember(call) { : undefined; } -function isTestRegistration(call) { - if (ts.isIdentifier(call.expression)) return call.expression.text === "test"; - const member = testMember(call); +function helperImport(file, filename, canonicalHelperPath) { + for (const statement of file.statements) { + if ( + !ts.isImportDeclaration(statement) || + !ts.isStringLiteral(statement.moduleSpecifier) + ) + continue; + const resolved = path.resolve( + path.dirname(filename), + `${statement.moduleSpecifier.text}.ts`, + ); + const canonical = canonicalHelperPath + ? resolved === canonicalHelperPath + : statement.moduleSpecifier.text === "../helpers/test"; + if (!canonical) continue; + const elements = statement.importClause?.namedBindings; + if (!elements || !ts.isNamedImports(elements)) return undefined; + const names = new Map( + elements.elements.map((element) => [ + element.name.text, + element.propertyName?.text ?? element.name.text, + ]), + ); + if ( + names.get("test") === "test" && + names.get("bootstrapE2ePage") === "bootstrapE2ePage" + ) + return { test: "test", bootstrap: "bootstrapE2ePage" }; + } +} + +function hasShadowingDeclaration(file, canonical) { + let shadowed = false; + const visit = (node) => { + if (shadowed) return; + if ( + (ts.isVariableDeclaration(node) || + ts.isFunctionDeclaration(node) || + ts.isClassDeclaration(node)) && + node.name && + ts.isIdentifier(node.name) && + (node.name.text === canonical.test || + node.name.text === canonical.bootstrap) + ) { + shadowed = true; + return; + } + if ( + (ts.isArrowFunction(node) || + ts.isFunctionExpression(node) || + ts.isFunctionDeclaration(node)) && + node.parameters.some( + (parameter) => + ts.isIdentifier(parameter.name) && + (parameter.name.text === canonical.test || + parameter.name.text === canonical.bootstrap), + ) + ) { + shadowed = true; + return; + } + ts.forEachChild(node, visit); + }; + for (const statement of file.statements) { + if (ts.isImportDeclaration(statement)) continue; + visit(statement); + } + return shadowed; +} + +function collectModuleHelpers(file) { + const helpers = new Map(); + for (const statement of file.statements) { + if (ts.isFunctionDeclaration(statement) && statement.name && statement.body) + helpers.set(statement.name.text, statement); + if (ts.isVariableStatement(statement)) { + for (const declaration of statement.declarationList.declarations) { + if ( + ts.isIdentifier(declaration.name) && + declaration.initializer && + (ts.isArrowFunction(declaration.initializer) || + ts.isFunctionExpression(declaration.initializer)) + ) + helpers.set(declaration.name.text, declaration.initializer); + } + } + } + return helpers; +} + +function awaitedCall(statement) { + const expression = + ts.isExpressionStatement(statement) || ts.isReturnStatement(statement) + ? statement.expression + : ts.isVariableStatement(statement) && + statement.declarationList.declarations.length === 1 + ? statement.declarationList.declarations[0].initializer + : undefined; + return expression && + ts.isAwaitExpression(expression) && + ts.isCallExpression(expression.expression) + ? expression.expression + : undefined; +} + +function isAwaitedCall(statement, name) { + const call = awaitedCall(statement); return ( - member !== undefined && - ![ - "beforeEach", - "afterEach", - "beforeAll", - "afterAll", - "describe", - "use", - "setTimeout", - "slow", - "step", - "skip", - "fixme", - "fail", - ].includes(member) + call && ts.isIdentifier(call.expression) && call.expression.text === name ); } -function collectFunctionDeclarations(file) { - const functions = new Map(); - const visit = (node) => { - if (ts.isFunctionDeclaration(node) && node.name) - functions.set(node.name.text, node); - if ( - ts.isVariableDeclaration(node) && - ts.isIdentifier(node.name) && - node.initializer && - (ts.isArrowFunction(node.initializer) || - ts.isFunctionExpression(node.initializer)) - ) { - functions.set(node.name.text, node.initializer); - } - ts.forEachChild(node, visit); - }; - visit(file); - return functions; +function isStaticallyEmptyLoop(statement) { + if (ts.isForStatement(statement)) + return statement.condition?.kind === ts.SyntaxKind.FalseKeyword; + if (ts.isWhileStatement(statement)) + return statement.expression.kind === ts.SyntaxKind.FalseKeyword; + if (ts.isForOfStatement(statement)) { + const expression = statement.expression; + return ( + (ts.isArrayLiteralExpression(expression) && + !expression.elements.length) || + (ts.isStringLiteral(expression) && !expression.text.length) + ); + } + if (ts.isForInStatement(statement)) + return ( + ts.isObjectLiteralExpression(statement.expression) && + !statement.expression.properties.length + ); + return false; } -function callbackBootstraps(callback, functions) { - const visitedFunctions = new Set(); - const inspect = (node) => { - let found = false; - const visit = (child) => { +function statementBootstraps(statement, canonical, helpers, seen) { + if (isAwaitedCall(statement, canonical.bootstrap)) return true; + const call = awaitedCall(statement); + if (call && ts.isIdentifier(call.expression)) { + const helperName = call.expression.text; + const helper = helpers.get(helperName); + if (helper && !seen.has(helperName)) { + seen.add(helperName); + if (callbackBootstraps(helper, canonical, helpers, seen)) return true; + } + } + if (ts.isBlock(statement)) + return statement.statements.some((child) => + statementBootstraps(child, canonical, helpers, seen), + ); + if (ts.isTryStatement(statement)) + return ( + statementBootstraps(statement.tryBlock, canonical, helpers, seen) || + Boolean( + statement.finallyBlock && + statementBootstraps(statement.finallyBlock, canonical, helpers, seen), + ) + ); + if ( + (ts.isForStatement(statement) || + ts.isForInStatement(statement) || + ts.isForOfStatement(statement) || + ts.isWhileStatement(statement) || + ts.isDoStatement(statement)) && + !isStaticallyEmptyLoop(statement) + ) + return statementBootstraps(statement.statement, canonical, helpers, seen); + return false; +} + +function callbackBootstraps(callback, canonical, helpers, seen = new Set()) { + const statements = ts.isBlock(callback.body) + ? callback.body.statements + : [ts.factory.createExpressionStatement(callback.body)]; + return statements.some((statement) => + statementBootstraps(statement, canonical, helpers, seen), + ); +} + +function makeScope(parent) { + return { parent, hooks: [], tests: [], children: [] }; +} + +function scanFile(file, canonical) { + const helpers = collectModuleHelpers(file); + const root = makeScope(undefined); + const isTestCall = (call) => + ts.isIdentifier(call.expression) && call.expression.text === canonical.test; + const isMember = (call, name) => memberOf(call) === name; + + const scanStatements = (statements, scope) => { + for (const statement of statements) { + if (ts.isBlock(statement)) { + scanStatements(statement.statements, scope); + continue; + } if ( - ts.isAwaitExpression(child) && - ts.isCallExpression(child.expression) && - ts.isIdentifier(child.expression.expression) && - child.expression.expression.text === "bootstrapE2ePage" + ts.isForStatement(statement) || + ts.isForInStatement(statement) || + ts.isForOfStatement(statement) ) { - found = true; - return; + scanStatements( + ts.isBlock(statement.statement) + ? statement.statement.statements + : [statement.statement], + scope, + ); + continue; } - if (ts.isCallExpression(child) && ts.isIdentifier(child.expression)) { - const declaration = functions.get(child.expression.text); - if (declaration && !visitedFunctions.has(child.expression.text)) { - visitedFunctions.add(child.expression.text); - if (inspect(declaration.body)) found = true; - } + if ( + !ts.isExpressionStatement(statement) || + !ts.isCallExpression(statement.expression) + ) + continue; + const call = statement.expression; + const callback = callbackOf(call); + if (isMember(call, "describe") && callback) { + const child = makeScope(scope); + scope.children.push(child); + if (ts.isBlock(callback.body)) + scanStatements(callback.body.statements, child); + continue; + } + if (isMember(call, "beforeEach") && callback) { + scope.hooks.push(callbackBootstraps(callback, canonical, helpers)); + continue; + } + if ( + isTestCall(call) || + (memberOf(call) && TEST_MEMBERS.has(memberOf(call))) + ) { + if (callback) + scope.tests.push(callbackBootstraps(callback, canonical, helpers)); + continue; + } + if ( + ts.isPropertyAccessExpression(call.expression) && + CONTAINERS.has(call.expression.name.text) && + callback && + ts.isArrayLiteralExpression(call.expression.expression) + ) { + if (ts.isBlock(callback.body)) + scanStatements(callback.body.statements, scope); } - if (!found) ts.forEachChild(child, visit); - }; - ts.forEachChild(node, visit); - return found; - }; - return inspect(callback.body); -} - -function scanFile(file, functions) { - const hooks = []; - const tests = []; - const visit = (node) => { - if (ts.isCallExpression(node)) { - const callback = callbackOf(node); - const member = testMember(node); - if (member === "beforeEach" && callback) - hooks.push(callbackBootstraps(callback, functions)); - else if (isTestRegistration(node) && callback) - tests.push(callbackBootstraps(callback, functions)); } - ts.forEachChild(node, visit); }; - visit(file); - return { hooks, tests }; + scanStatements(file.statements, root); + return root; } -export function checkSource(source, filename = "fixture.spec.ts") { +function everyTestCovered(scope, inheritedHook = false) { + const coveredByHook = inheritedHook || scope.hooks.some(Boolean); + return ( + scope.tests.every((covered) => coveredByHook || covered) && + scope.children.every((child) => everyTestCovered(child, coveredByHook)) + ); +} + +function testCount(scope) { + return ( + scope.tests.length + + scope.children.reduce((count, child) => count + testCount(child), 0) + ); +} + +export function checkSource( + source, + filename = "fixture.spec.ts", + canonicalHelperPath, +) { const file = ts.createSourceFile( filename, source, ts.ScriptTarget.Latest, true, ); - const helperImports = file.statements.filter( - (statement) => - ts.isImportDeclaration(statement) && - ts.isStringLiteral(statement.moduleSpecifier) && - statement.moduleSpecifier.text === CANONICAL_TEST_HELPER, - ); - if ( - !helperImports.some((declaration) => namedImport(declaration, "test")) || - !helperImports.some((declaration) => - namedImport(declaration, "bootstrapE2ePage"), - ) - ) { - return "must directly import unaliased test and bootstrapE2ePage from ../helpers/test"; - } + const canonical = helperImport(file, filename, canonicalHelperPath); + if (!canonical) + return "must directly import unaliased test and bootstrapE2ePage from the canonical tests/helpers/test module"; + if (hasShadowingDeclaration(file, canonical)) + return "must not shadow canonical test or bootstrapE2ePage imports"; + const scope = scanFile(file, canonical); + if (!testCount(scope)) return "does not register a browser test"; + if (!everyTestCovered(scope)) + return "must structurally await bootstrapE2ePage for every test, directly or through an applicable ancestor test.beforeEach"; +} - const { hooks, tests } = scanFile(file, collectFunctionDeclarations(file)); - if (!tests.length) return "does not register a browser test"; - if (!hooks.some(Boolean) && !tests.some(Boolean)) { - return "must await bootstrapE2ePage on every test path, directly or through a registered test.beforeEach"; - } +function specFiles(directory) { + return fs.readdirSync(directory, { withFileTypes: true }).flatMap((entry) => { + const child = path.join(directory, entry.name); + if (entry.isDirectory()) return specFiles(child); + return entry.isFile() && SPEC_PATTERN.test(entry.name) ? [child] : []; + }); } export function runCheck(projectRoot) { - const violations = []; - for (const name of fs - .readdirSync(path.join(projectRoot, "tests/e2e")) - .filter((name) => SPEC_PATTERN.test(name))) { - if (exempt.has(name)) continue; + const e2eRoot = path.join(projectRoot, "tests/e2e"); + const canonicalHelperPath = path.join(projectRoot, "tests/helpers/test.ts"); + return specFiles(e2eRoot).flatMap((filename) => { + const relative = path.relative(e2eRoot, filename); + if (exempt.has(relative)) return []; const violation = checkSource( - fs.readFileSync(path.join(projectRoot, "tests/e2e", name), "utf8"), - name, + fs.readFileSync(filename, "utf8"), + filename, + canonicalHelperPath, ); - if (violation) violations.push(`${name}: ${violation}`); - } - return violations; + return violation ? [`${relative}: ${violation}`] : []; + }); } if (process.argv[1] === import.meta.filename) { diff --git a/web/scripts/check-e2e-bootstrap.test.mjs b/web/scripts/check-e2e-bootstrap.test.mjs index e7b84afa0a..6d0fc560e4 100644 --- a/web/scripts/check-e2e-bootstrap.test.mjs +++ b/web/scripts/check-e2e-bootstrap.test.mjs @@ -1,67 +1,165 @@ import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import test from "node:test"; const projects = [ ["desktop", "../scripts/check-e2e-bootstrap.mjs"], ["web", "../scripts/check-e2e-bootstrap.mjs"], ]; - -const validDirect = ` - import { test, bootstrapE2ePage } from "../helpers/test"; - test("uses bootstrap", async ({ page }) => { await bootstrapE2ePage(page); }); -`; -const validBeforeEach = ` - import { test, bootstrapE2ePage } from "../helpers/test"; - test.beforeEach(async ({ page }) => { await bootstrapE2ePage(page); }); - test("uses registered hook", async ({ page }) => { await page.title(); }); -`; -const validBeforeNavigate = ` - import { test, bootstrapE2ePage } from "../helpers/test"; - test("draft seed", async ({ page }) => { - await bootstrapE2ePage(page, { beforeNavigate: async () => page.addInitScript(() => {}) }); - }); -`; +const imports = 'import { test, bootstrapE2ePage } from "../helpers/test";'; +const spec = (body) => + `${imports}\ntest("x", async ({ page }) => { ${body} });`; for (const [project, checkerPath] of projects) { - const { checkSource } = await import(checkerPath); + const { checkSource, runCheck } = await import(checkerPath); + const accepts = (source) => assert.equal(checkSource(source), undefined); + const rejects = (source) => assert.notEqual(checkSource(source), undefined); + test(`${project}: accepts direct, hook, and beforeNavigate bootstrap patterns`, () => { - for (const source of [validDirect, validBeforeEach, validBeforeNavigate]) { - assert.equal(checkSource(source), undefined); - } + accepts(spec("await bootstrapE2ePage(page);")); + accepts( + `${imports}\ntest.beforeEach(async ({ page }) => { await bootstrapE2ePage(page); });\ntest("x", async ({ page }) => { await page.title(); });`, + ); + accepts( + spec( + "await bootstrapE2ePage(page, { beforeNavigate: async () => page.addInitScript(() => {}) });", + ), + ); + }); + + test(`${project}: accepts module helpers and ratified containers`, () => { + accepts( + `${imports}\nasync function open(page) { return await bootstrapE2ePage(page); }\ntest("x", async ({ page }) => { const response = await open(page); void response; });`, + ); + accepts( + spec( + "try { await bootstrapE2ePage(page); } finally { await page.title(); }", + ), + ); + accepts( + spec( + "try { await page.title(); } finally { await bootstrapE2ePage(page); }", + ), + ); + accepts( + spec("for (const attempt of [0, 1]) { await bootstrapE2ePage(page); }"), + ); + accepts( + spec( + "for (let attempt = 0; attempt < 2; attempt += 1) { await bootstrapE2ePage(page); }", + ), + ); }); for (const [name, source] of [ [ ".ts helper suffix", - `import { test, bootstrapE2ePage } from "../helpers/test.ts"; test("x", async () => {});`, + 'import { test, bootstrapE2ePage } from "../helpers/test.ts"; test("x", async () => {});', ], [ "import alias", - `import { test as e2eTest, bootstrapE2ePage } from "../helpers/test"; e2eTest("x", async () => { await bootstrapE2ePage(); });`, + 'import { test as e2eTest, bootstrapE2ePage } from "../helpers/test"; e2eTest("x", async () => { await bootstrapE2ePage(); });', ], [ "re-export", - `import { test, bootstrapE2ePage } from "./reexport"; test("x", async () => { await bootstrapE2ePage(); });`, + 'import { test, bootstrapE2ePage } from "./reexport"; test("x", async () => { await bootstrapE2ePage(); });', ], [ "dynamic import", - `const { test, bootstrapE2ePage } = await import("../helpers/test"); test("x", async () => { await bootstrapE2ePage(); });`, + 'const { test, bootstrapE2ePage } = await import("../helpers/test"); test("x", async () => { await bootstrapE2ePage(); });', ], [ "dead-code call", - `import { test, bootstrapE2ePage } from "../helpers/test"; async function never(page) { await bootstrapE2ePage(page); } test("x", async () => {});`, + `${imports}\nasync function never(page) { await bootstrapE2ePage(page); } test("x", async () => {});`, ], [ "comment", - `import { test, bootstrapE2ePage } from "../helpers/test"; // await bootstrapE2ePage(page)\ntest("x", async () => {});`, + `${imports}\n// await bootstrapE2ePage(page)\ntest("x", async () => {});`, ], [ "string", - `import { test, bootstrapE2ePage } from "../helpers/test"; const hint = "await bootstrapE2ePage(page)"; test("x", async () => {});`, + `${imports}\nconst hint = "await bootstrapE2ePage(page)"; test("x", async () => {});`, + ], + [ + "mixed safe and unsafe tests", + `${imports}\ntest("safe", async ({ page }) => { await bootstrapE2ePage(page); }); test("unsafe", async () => {});`, + ], + ["if-wrapped call", spec("if (true) { await bootstrapE2ePage(page); }")], + [ + "catch-only call", + spec( + "try { await page.title(); } catch { await bootstrapE2ePage(page); }", + ), + ], + [ + "zero-iteration false loop", + spec("for (; false;) { await bootstrapE2ePage(page); }"), + ], + [ + "zero-iteration empty iterable", + spec("for (const value of []) { await bootstrapE2ePage(page); }"), + ], + [ + "uncalled nested function", + spec("async function setup() { await bootstrapE2ePage(page); }"), + ], + [ + "unawaited helper", + `${imports}\nasync function open(page) { await bootstrapE2ePage(page); }\ntest("x", async ({ page }) => { open(page); });`, + ], + [ + "fake test.extend", + `${imports}\ntest.extend("x", async ({ page }) => { await bootstrapE2ePage(page); });`, + ], + [ + "misplaced describe hook", + `${imports}\ntest.describe("inside", () => { test.beforeEach(async ({ page }) => { await bootstrapE2ePage(page); }); test("covered", async () => {}); }); test("outside", async () => {});`, + ], + [ + "shadowed bootstrap", + `${imports}\ntest("x", async ({ page }) => { const bootstrapE2ePage = async () => {}; await bootstrapE2ePage(page); });`, ], ]) { - test(`${project}: rejects ${name}`, () => { - assert.notEqual(checkSource(source), undefined); - }); + test(`${project}: rejects ${name}`, () => rejects(source)); } + + test(`${project}: discovers nested specs with a canonical relative import`, () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "e2e-bootstrap-")); + try { + fs.mkdirSync(path.join(root, "tests/e2e/nested"), { recursive: true }); + fs.mkdirSync(path.join(root, "tests/helpers"), { recursive: true }); + fs.writeFileSync( + path.join(root, "tests/helpers/test.ts"), + "export {};\n", + ); + fs.writeFileSync( + path.join(root, "tests/e2e/nested/example.spec.ts"), + 'import { test, bootstrapE2ePage } from "../../helpers/test"; test("x", async ({ page }) => { await bootstrapE2ePage(page); });', + ); + assert.deepEqual(runCheck(root), []); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); +} + +for (const [project, checkerPath] of projects) { + const { checkSource } = await import(checkerPath); + test(`${project}: traverses canonical parameterized registration containers only`, () => { + assert.equal( + checkSource(`${imports} + for (const name of ["one", "two"]) test(name, async ({ page }) => { await bootstrapE2ePage(page); }); + ["three"].forEach((name) => test(name, async ({ page }) => { await bootstrapE2ePage(page); })); + ["four"].map((name) => test(name, async ({ page }) => { await bootstrapE2ePage(page); }));`), + undefined, + ); + assert.notEqual( + checkSource(`${imports} + function registerLater() { test("hidden", async ({ page }) => { await bootstrapE2ePage(page); }); } + test("unsafe", async () => {});`), + undefined, + ); + }); }