diff --git a/README.md b/README.md index a51a171..8ec7603 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,7 @@ Tests that are confidently irrelevant get skipped. Everything else runs through - **Fail open**: uncertainty means RUN. A missing API key, an API timeout, or a malformed response always falls back to running the full suite, loudly (`⚠ Judge unavailable (...), running the full suite.`). Finding no tests at all is an error (exit 1), not a silent pass. - **Deterministic overrides**: no threshold decides these, the judge isn't even asked. A test whose own file changed always runs, as does one that statically imports a changed file, or that navigates a route a changed file's own path names (e.g. `page.goto("/admin/users")` against a changed `routes/admin/users.tsx`) -- a heuristic that catches e2e route coupling no import graph can see, since a browser test never imports the page it drives. +- **Whole-suite rules**: before any per-test decision, a change to the runner's own setup (`playwright.config.*`, `vitest.config.*`, `vite.config.*`, `package.json`, a lockfile, or anything in `.github/workflows/`) runs every test, and a change that only touches Markdown files skips every test. Neither asks the judge, so every shard of a matrix gets the same answer. - **Leanest doesn't run tests itself**: it selects file paths and hands them to your actual runner (`playwright test `, `vitest run `). It leaves reporters, retries, sharding, and CI-required-check behavior alone. Anything after `--` goes straight to the runner: `leanest playwright -- --shard=1/3`. - **Static checks are out of scope on purpose**: lint/format/typecheck are already fast at full scope, and semantic per-rule selection would add latency for no real payoff. Leanest spends its Jev budget only on suites that are expensive to run in full: e2e today, more later. diff --git a/src/cli.test.ts b/src/cli.test.ts index dc83a79..35819d4 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { parseFlags, renderReport, reportMarker } from "./cli.js"; +import { explainRuns, parseFlags, renderReport, reportMarker } from "./cli.js"; import type { SelectionResult, TestCase } from "./types.js"; describe("parseFlags", () => { @@ -103,6 +103,34 @@ describe("renderReport", () => { expect(md).toContain("| `a\\|b.spec.ts` | RUN | judge unavailable retry |"); }); + test("says why the selected tests run, in plain words, in the comment too", () => { + const why = (rule: number, judgeUnsure: number, judgeLikely: number) => + explainRuns(result({ runBreakdown: { rule, judgeUnsure, judgeLikely } })); + expect(why(2, 9, 1)).toBe( + "2 touch the change directly, the judge wasn't sure enough to skip 9 and it thinks 1 is affected.", + ); + expect(why(0, 37, 0)).toBe("The judge wasn't sure enough to skip 37."); + expect(why(1, 0, 3)).toBe("1 touches the change directly and the judge thinks 3 are affected."); + expect(explainRuns(result({ suiteReason: "runner setup changed (package.json)" }))).toBe( + "The only test runs because the runner setup changed (package.json).", + ); + expect(explainRuns(result({ selectedTests: [] }))).toBeNull(); + expect(explainRuns(result({ selectedTests: [], suiteReason: "only Markdown changed" }))).toBe( + "Nothing runs because only Markdown changed.", + ); + const suite = renderReport( + "playwright", + ".", + result({ suiteReason: "runner setup changed (package.json)" }), + false, + ); + expect(suite.indexOf("
1 test file")).toBeLessThan( + suite.indexOf("| `a.spec.ts` | RUN |"), + ); + const r = result({ runBreakdown: { rule: 1, judgeUnsure: 0, judgeLikely: 0 } }); + expect(renderReport("playwright", ".", r, false)).toContain("1 touches the change directly."); + }); + test("marker differs per framework and dir, so each run keeps its own comment", () => { expect(reportMarker("playwright", ".")).not.toBe(reportMarker("vitest", ".")); expect(reportMarker("playwright", "apps/web")).not.toBe(reportMarker("playwright", ".")); diff --git a/src/cli.ts b/src/cli.ts index b406d01..41cdd8f 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -165,6 +165,15 @@ function printInspect(result: any): void { } console.log(`Discovering ${discovered.framework} tests...`); console.log(` ${discovered.count} tests found`); + if (result.error) { + console.log( + `\n⚠ Judge unavailable (${result.error}), all ${discovered.count} tests would run.`, + ); + } + if (result.suiteReason) { + console.log(`\nNo judge needed: ${result.suiteReason}.`); + console.log(`Selected ${result.selected.length} / ${discovered.count} tests`); + } if (result.evaluated.length > 0) { console.log(`\nEvaluating semantic impact...`); console.log(` ${result.evaluated.length} tests evaluated`); @@ -182,25 +191,58 @@ function printInspect(result: any): void { console.log(`\nSkipping ${result.skipped} tests.`); } -function printSelect(result: any): void { +/** One sentence on why the selected tests run, or null when there's nothing to explain. */ +export function explainRuns(result: SelectionResult): string | null { + const n = result.selectedTests.length; + // Whole-suite rules: "only Markdown changed" skips all, "runner setup changed (…)" runs all. + if (n === 0) return result.suiteReason ? `Nothing runs because ${result.suiteReason}.` : null; + if (result.suiteReason) { + return `${n === 1 ? "The only test runs" : `All ${n} run`} because the ${result.suiteReason}.`; + } + const b = result.runBreakdown; + if (!b) return null; + const parts = [ + b.rule > 0 ? `${b.rule} ${b.rule === 1 ? "touches" : "touch"} the change directly` : "", + b.judgeUnsure > 0 ? `the judge wasn't sure enough to skip ${b.judgeUnsure}` : "", + b.judgeLikely > 0 + ? `${b.judgeUnsure > 0 ? "it" : "the judge"} thinks ${b.judgeLikely === 1 ? "1 is" : `${b.judgeLikely} are`} affected` + : "", + ].filter(Boolean); + const sentence = + parts.length > 1 ? `${parts.slice(0, -1).join(", ")} and ${parts.at(-1)}` : parts[0]!; + return `${sentence.charAt(0).toUpperCase()}${sentence.slice(1)}.`; +} + +function printList(lines: string[]): void { + for (const line of lines.slice(0, 20)) console.log(` ${line}`); + if (lines.length > 20) console.log(` ... and ${lines.length - 20} more`); +} + +function printSelect(result: SelectionResult): void { const noChanges = result.changedFiles.length === 0; if (noChanges && result.totalTests > 0) { console.log(`No changes detected.`); console.log(`Evaluating all ${result.totalTests} tests conservatively...\n`); } else { console.log(`Changed:`); - for (const f of result.changedFiles.slice(0, 20)) { - console.log(` ${f}`); - } + printList(result.changedFiles); console.log(``); } console.log(`${result.totalTests} tests found`); console.log(`\nSelected ${result.selectedTests.length} / ${result.totalTests} tests`); - for (const test of result.selectedTests.slice(0, 20)) { - console.log(` RUN ${test.identity.path}`); - } + const why = explainRuns(result); + if (why) console.log(why); + // A whole-suite rule already said why in one line; don't repeat it per test. + printList( + result.selectedTests.map((t) => + result.suiteReason + ? `RUN ${t.identity.path}` + : `RUN ${t.identity.path} (${result.reasons[t.identity.path]})`, + ), + ); if (result.skippedTests > 0) { - console.log(`\nSkipping ${result.skippedTests} tests.`); + const n = result.skippedTests; + console.log(`\nSkipping ${n} tests.`); } } @@ -239,6 +281,7 @@ export function renderReport( : [`
${summary}`, "", ...table(rows), "", "
", ""]; const runRows = result.selectedTests.map((t) => row(t, "RUN")); const skipRows = result.skipped.map((t) => row(t, "SKIP")); + const why = explainRuns(result); if (result.status === "error") { return [ @@ -259,8 +302,15 @@ export function renderReport( reportMarker(command, dir), `### leanest: ${result.selectedTests.length} of ${result.totalTests} ${command} test files selected`, "", + // Under --shadow/--full everything runs, so "Nothing runs because…" would contradict it. + ...(why && !runningAll ? [why, ""] : []), ...(runningAll ? ["The full suite runs anyway (`--shadow` or `--full`).", ""] : []), - ...(runRows.length > 0 ? [...table(runRows), ""] : []), + // A whole-suite rule gives every row the same reason, already said above: fold them. + ...(result.suiteReason + ? details(`${runRows.length} test file${runRows.length === 1 ? "" : "s"}`, runRows) + : runRows.length > 0 + ? [...table(runRows), ""] + : []), ...details(`${skipRows.length} skipped`, skipRows), ].join("\n"); } diff --git a/src/leanest.test.ts b/src/leanest.test.ts new file mode 100644 index 0000000..3aa3e2d --- /dev/null +++ b/src/leanest.test.ts @@ -0,0 +1,58 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { execFileSync } from "child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; +import { Leanest } from "./leanest.js"; + +// A real git repo, so the whole-suite rules run end to end. Neither case calls the judge. +// Runs from the repo root with dir ".", like the Action's default. +describe("Leanest.select with a Markdown-only change", () => { + const root = mkdtempSync(join(tmpdir(), "leanest-suite-")); + const git = (...args: string[]) => + execFileSync("git", ["-c", "user.name=t", "-c", "user.email=t@t", ...args], { cwd: root }); + + const startDir = process.cwd(); + + beforeAll(() => { + process.chdir(root); + git("init", "-q"); + writeFileSync(join(root, "terms.md"), "v1"); + writeFileSync(join(root, "README.md"), "v1"); + writeFileSync( + join(root, "terms.test.ts"), + `import terms from "./terms.md";\ntest("x", () => {});`, + ); + writeFileSync(join(root, "math.test.ts"), `test("y", () => {});`); + git("add", "."); + git("commit", "-qm", "base"); + }); + + afterAll(() => { + process.chdir(startDir); + rmSync(root, { recursive: true, force: true }); + }); + + const change = (file: string) => { + writeFileSync(join(root, file), `${Date.now()}`); + git("commit", "-qam", file); + }; + const names = (tests: { identity: { path: string } }[]) => + tests.map((t) => t.identity.path.split("/").pop()); + + test("skips everything when no test depends on the Markdown", async () => { + change("README.md"); + const result = await new Leanest(".", "HEAD~1").select("vitest"); + expect(result.selectedTests).toEqual([]); + expect(result.suiteReason).toBe("only Markdown changed"); + }); + + test("still runs a test that imports the changed Markdown", async () => { + change("terms.md"); + const result = await new Leanest(".", "HEAD~1").select("vitest"); + expect(names(result.selectedTests)).toEqual(["terms.test.ts"]); + expect(names(result.skipped)).toEqual(["math.test.ts"]); + // The rule no longer explains every test, so no shared reason. + expect(result.suiteReason).toBeUndefined(); + }); +}); diff --git a/src/leanest.ts b/src/leanest.ts index fcaf641..e325d83 100644 --- a/src/leanest.ts +++ b/src/leanest.ts @@ -6,7 +6,7 @@ import { import { ChangeResolver, type GitChange } from "./git-diff.js"; import { TestDiscovery } from "./test-discovery.js"; import { ContextBuilder } from "./context-builder.js"; -import { SelectionPolicy } from "./selection-policy.js"; +import { MIN_CONFIDENCE, SelectionPolicy, suiteRule } from "./selection-policy.js"; import { importsChangedFile } from "./import-graph.js"; import { touchesSameRoute } from "./route-heuristic.js"; import type { TestCase, SelectionResult, PipelineResult } from "./types.js"; @@ -53,19 +53,33 @@ export class Leanest { }; } + const suite = this.suiteDecision(discovery.tests, change); + if (suite) { + return { + change, + discovered: { framework, count: discovery.tests.length, tests: discovery.tests }, + evaluated: [], + selected: suite.run, + skipped: suite.skip.length, + decision: "RUN", + suiteReason: suite.rule.reason, + }; + } + const state = this.context.buildState(change, discovery.tests); const questions = this.buildQuestions(discovery.tests, change.changedFiles.length === 0); let answers: Record; try { answers = await this.judge.evaluate(state, questions); - } catch { + } catch (error) { return { change, discovered: { framework, count: discovery.tests.length, tests: discovery.tests }, evaluated: [], - selected: [], + selected: discovery.tests, skipped: 0, decision: "RUN", + error: error instanceof Error ? error.message : String(error), }; } @@ -120,6 +134,25 @@ export class Leanest { }; } + const suite = this.suiteDecision(tests, change); + if (suite) { + return { + command: "select", + args: [framework], + status: "complete", + totalTests: tests.length, + selectedTests: suite.run, + skippedTests: suite.skip.length, + runTests: suite.run, + skipped: suite.skip, + reasons: suite.reasons, + suiteReason: suite.sharedReason, + runBreakdown: { rule: suite.run.length, judgeUnsure: 0, judgeLikely: 0 }, + changedFiles: change.changedFiles, + diff: change.diff, + }; + } + const state = this.context.buildState(change, tests); const questions = this.buildQuestions(tests, change.changedFiles.length === 0); let answers: Record; @@ -155,6 +188,7 @@ export class Leanest { const runTests: TestCase[] = []; const skipTests: TestCase[] = []; const reasons: Record = {}; + const runBreakdown = { rule: 0, judgeUnsure: 0, judgeLikely: 0 }; for (const entry of ranked) { const deterministic = this.deterministicReason(entry.test, change); @@ -164,6 +198,9 @@ export class Leanest { this.policy.decide(entry.probability, entry.confidence, deterministic !== null) === "RUN" ) { runTests.push(entry.test); + if (deterministic) runBreakdown.rule++; + else if (entry.confidence < MIN_CONFIDENCE) runBreakdown.judgeUnsure++; + else runBreakdown.judgeLikely++; } else { skipTests.push(entry.test); } @@ -179,11 +216,32 @@ export class Leanest { runTests, skipped: skipTests, reasons, + runBreakdown, changedFiles: change.changedFiles, diff: change.diff, }; } + /** + * A whole-suite rule's outcome, or null to ask the judge. A Markdown-only change still runs + * the tests a deterministic rule forces, e.g. one that imports the changed .md file. + */ + private suiteDecision(tests: TestCase[], change: GitChange) { + const rule = suiteRule(change.changedFiles); + if (!rule) return null; + const run: TestCase[] = []; + const skip: TestCase[] = []; + const reasons: Record = {}; + for (const test of tests) { + const forced = rule.decision === "SKIP" ? this.deterministicReason(test, change) : null; + reasons[test.identity.path] = forced ?? rule.reason; + (rule.decision === "RUN" || forced ? run : skip).push(test); + } + // Only when the rule decided every test does its reason explain the whole selection. + const sharedReason = rule.decision === "RUN" || run.length === 0 ? rule.reason : undefined; + return { rule, run, skip, reasons, sharedReason }; + } + private deterministicReason(test: TestCase, change: GitChange): string | null { if (change.changedFiles.includes(test.identity.path)) return "test file changed"; if (importsChangedFile(test, change.changedFiles, this.cwd)) return "imports a changed file"; diff --git a/src/selection-policy.test.ts b/src/selection-policy.test.ts index 58c191b..0cf60ce 100644 --- a/src/selection-policy.test.ts +++ b/src/selection-policy.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { SelectionPolicy } from "./selection-policy.js"; +import { SelectionPolicy, suiteRule } from "./selection-policy.js"; describe("SelectionPolicy.decide", () => { const policy = new SelectionPolicy(); @@ -25,3 +25,33 @@ describe("SelectionPolicy.decide", () => { expect(policy.decide(0.1, undefined, false)).toBe("RUN"); }); }); + +describe("suiteRule", () => { + test("runs everything when the runner's setup changed", () => { + for (const f of [ + "playwright.config.ts", + "apps/web/vitest.config.mts", + "package.json", + "bun.lock", + ".github/workflows/test.yml", + ]) { + expect(suiteRule(["src/a.ts", f])?.decision).toBe("RUN"); + } + }); + + test("skips everything when only Markdown changed", () => { + expect(suiteRule(["AGENTS.md", "docs/guide.md"])).toEqual({ + decision: "SKIP", + reason: "only Markdown changed", + }); + }); + + test("asks per test otherwise, and on no changes", () => { + expect(suiteRule(["README.md", "src/a.ts"])).toBeNull(); + expect(suiteRule([])).toBeNull(); + }); + + test("runner config wins over Markdown", () => { + expect(suiteRule(["README.md", ".github/workflows/test.yml"])?.decision).toBe("RUN"); + }); +}); diff --git a/src/selection-policy.ts b/src/selection-policy.ts index e10119b..7ba6cd5 100644 --- a/src/selection-policy.ts +++ b/src/selection-policy.ts @@ -1,5 +1,31 @@ import type { TestCase } from "./types.js"; +/** Below this judge confidence the probability isn't trusted, and the test runs. */ +export const MIN_CONFIDENCE = 0.5; + +// Files every test depends on: the runner's config, dependencies, and CI workflows. +const RUNNER_CONFIG = [ + /(^|\/)(playwright|vitest|vite)\.config\.[cm]?[jt]s$/, + /(^|\/)package\.json$/, + /(^|\/)(package-lock\.json|bun\.lockb?|pnpm-lock\.yaml|yarn\.lock)$/, + /^\.github\/workflows\//, +]; + +/** + * A decision for the whole suite that needs no judge: run everything when the runner's + * own setup changed, skip everything when only Markdown changed. Null means ask per test. + */ +export function suiteRule( + changedFiles: string[], +): { decision: "RUN" | "SKIP"; reason: string } | null { + const config = changedFiles.find((f) => RUNNER_CONFIG.some((re) => re.test(f))); + if (config) return { decision: "RUN", reason: `runner setup changed (${config})` }; + if (changedFiles.length > 0 && changedFiles.every((f) => f.endsWith(".md"))) { + return { decision: "SKIP", reason: "only Markdown changed" }; + } + return null; +} + export class SelectionPolicy { decide( probability: number | undefined | null, @@ -9,7 +35,7 @@ export class SelectionPolicy { if (testChanged) return "RUN"; if (probability === undefined || probability === null) return "RUN"; if (confidence === undefined || confidence === null) return "RUN"; - if (confidence < 0.5) return "RUN"; + if (confidence < MIN_CONFIDENCE) return "RUN"; if (probability < 0.3) return "SKIP"; return "RUN"; } diff --git a/src/types.ts b/src/types.ts index 9a96558..0b5337d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -43,6 +43,10 @@ export interface SelectionResult { skipped: TestCase[]; /** Why each test path was run or skipped. */ reasons: Record; + /** Why the selected tests run: forced by a rule, the judge too unsure to skip, or judged affected. */ + runBreakdown?: { rule: number; judgeUnsure: number; judgeLikely: number }; + /** Set when a whole-suite rule decided every test, e.g. "only Markdown changed". */ + suiteReason?: string; decision?: string; changedFiles: string[]; diff: string; @@ -59,4 +63,7 @@ export interface PipelineResult { selected: TestCase[]; skipped: number; decision: "RUN" | "SKIP"; + /** Set when the judge failed; every test is then selected. */ + error?: string /** Set when a whole-suite rule decided instead of the judge. */; + suiteReason?: string; }