diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 07e01f7..a58ccd3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,6 +29,10 @@ jobs: # Runs the Action from this checkout against a stub Playwright, so no browsers are needed. action: runs-on: ubuntu-latest + # Lets the Action post its sticky report comment on this repo's own PRs. + permissions: + contents: read + pull-requests: write steps: - uses: actions/checkout@v4 with: @@ -60,3 +64,4 @@ jobs: cat "$RUNNER_TEMP/playwright-args" grep -q "fixtures/action/price.pw.ts" "$RUNNER_TEMP/playwright-args" grep -q -- "--shard=1/1" "$RUNNER_TEMP/playwright-args" + grep -q "leanest-report" "$RUNNER_TEMP/leanest-report.md" diff --git a/README.md b/README.md index 7de95ad..a51a171 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,11 @@ LEANEST_PROVIDER=jev npx leanest playwright ### GitHub Actions ```yaml +permissions: + contents: read + pull-requests: write # for the report comment + +steps: - uses: actions/checkout@v4 with: fetch-depth: 0 @@ -186,7 +191,7 @@ LEANEST_PROVIDER=jev npx leanest playwright This installs the `leanest` version matching the Action's ref with the runner's Node (it doesn't touch your Bun), and replaces your existing "run e2e tests" step: same reporter output, same exit code, just fewer tests executed. No secret required — the default `classifier-dev` provider needs no API key, which also means forked-repo PRs can use it without access to your repo's secrets. Pass `provider: jev` and `typesafe-api-key: ${{ secrets.TYPESAFE_API_KEY }}` to use Jev instead. -On pull requests it diffs against the PR's base branch; on push, against the previous commit. Override with `base:`. Pass runner flags with `args:`, for example `args: --shard=${{ matrix.shard }}/3`. Each run writes a job summary listing every test file, whether it ran, and why. +On pull requests it diffs against the PR's base branch; on push, against the previous commit. Override with `base:`. Pass runner flags with `args:`, for example `args: --shard=${{ matrix.shard }}/3`. Each run writes a job summary listing every test file, whether it ran, and why. On pull requests it also posts that report as a PR comment and edits the same comment on later pushes. Turn it off with `comment: false`. Without `pull-requests: write`, and on fork PRs (which get a read-only token), posting logs a warning and the tests' result stands. ### Any other CI diff --git a/action.yml b/action.yml index 4261c1d..9b86560 100644 --- a/action.yml +++ b/action.yml @@ -28,6 +28,10 @@ inputs: description: Extra arguments passed to the test runner, e.g. --shard=1/3 required: false default: "" + comment: + description: Post the selection report as a sticky PR comment (needs pull-requests - write) + required: false + default: "true" runs: using: composite @@ -53,7 +57,9 @@ runs: PR_BASE: ${{ github.event.pull_request.base.ref }} BEFORE: ${{ github.event.before }} DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + LEANEST_REPORT_FILE: ${{ runner.temp }}/leanest-report.md run: | + : > "$LEANEST_REPORT_FILE" if [ -z "$BASE" ]; then if [ -n "$PR_BASE" ]; then BASE="origin/$PR_BASE" # A push that creates a branch has an all-zero "before" SHA. @@ -63,3 +69,39 @@ runs: fi # ARGS is split on purpose so each runner flag becomes its own argument. leanest "$FRAMEWORK" --base "$BASE" --dir "$DIR" -- $ARGS + + # Edits the bot's comment starting with the report's first line (its marker), or posts a new one. + # Fails soft: fork PRs get a read-only token, and that must not fail the tests. + - name: Post the report as a PR comment + if: always() && inputs.comment == 'true' && github.event_name == 'pull_request' + shell: bash + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + PR: ${{ github.event.pull_request.number }} + REPORT: ${{ runner.temp }}/leanest-report.md + ARGS: ${{ inputs.args }} + run: | + [ -s "$REPORT" ] || exit 0 + # Sharded jobs would race to create the comment on a PR's first push: only shard 1 posts. + # ponytail: other matrix axes (browser, OS) still race; set comment: false on all but one. + case " $ARGS " in *" --shard="*|*" --shard "*) + case " $ARGS " in *" --shard=1/"*|*" --shard 1/"*) ;; *) exit 0 ;; esac ;; + esac + BODY="$REPORT" + # GitHub rejects comments over 65,536 characters: keep the summary, point to the job summary. + if [ "$(wc -c < "$REPORT")" -gt 65000 ]; then + BODY="$RUNNER_TEMP/leanest-comment.md" + { awk '/^(\| Test|
)/ { exit } { print }' "$REPORT" + echo "The full table is too long for a comment. It's in this run's job summary."; } > "$BODY" + fi + export MARKER="$(head -n1 "$REPORT")" + ID="$(gh api --paginate "repos/$REPO/issues/$PR/comments" \ + --jq '.[] | select(.user.login == "github-actions[bot]" and (.body | startswith(env.MARKER))) | .id' | + sed -n 1p)" && + if [ -n "$ID" ]; then + gh api -X PATCH "repos/$REPO/issues/comments/$ID" -F body=@"$BODY" >/dev/null + else + gh api "repos/$REPO/issues/$PR/comments" -F body=@"$BODY" >/dev/null + fi || + echo "::warning::leanest couldn't post its PR comment. The workflow needs 'pull-requests: write', and fork PRs only get a read-only token." diff --git a/src/cli.test.ts b/src/cli.test.ts index 726ddc1..dc83a79 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; -import { parseFlags } from "./cli.js"; +import { parseFlags, renderReport, reportMarker } from "./cli.js"; +import type { SelectionResult, TestCase } from "./types.js"; describe("parseFlags", () => { test("reads --base and --dir as string values, not booleans", () => { @@ -36,3 +37,74 @@ describe("parseFlags", () => { expect(flags.project).toBeUndefined(); }); }); + +describe("renderReport", () => { + const tc = (path: string): TestCase => ({ + identity: { framework: "playwright", path, suite: [], name: path, hash: path }, + source: "", + context: "", + }); + const result = (over: Partial): SelectionResult => ({ + command: "select", + args: [], + status: "complete", + totalTests: 2, + selectedTests: [tc("a.spec.ts")], + skippedTests: 1, + runTests: [tc("a.spec.ts")], + skipped: [tc("b.spec.ts")], + reasons: { "a.spec.ts": "test file changed", "b.spec.ts": "judge p=0.04 c=0.91" }, + changedFiles: [], + diff: "", + ...over, + }); + + test("starts with the marker, shows RUN rows, folds SKIP rows into details", () => { + const md = renderReport("playwright", ".", result({}), false); + expect(md.startsWith(reportMarker("playwright", "."))).toBe(true); + expect(md).toContain("1 of 2 playwright test files selected"); + expect(md).toContain("| `a.spec.ts` | RUN | test file changed |"); + expect(md).toContain("
1 skipped"); + expect(md.indexOf("
")).toBeLessThan(md.indexOf("| `b.spec.ts` | SKIP |")); + }); + + test("judge down: warning with the reason, every test still runs", () => { + const md = renderReport( + "playwright", + ".", + result({ + status: "error", + error: "classifier.dev error (503): upstream timeout", + selectedTests: [tc("a.spec.ts"), tc("b.spec.ts")], + skipped: [], + }), + false, + ); + expect(md).toContain("all 2 playwright test files run"); + expect(md).toContain("> [!WARNING]"); + expect(md).toContain("> Reason: `classifier.dev error (503): upstream timeout`"); + expect(md).toContain("
2 test files, all RUN"); + }); + + test("keeps pipes, newlines and backticks from breaking the table and the warning", () => { + const md = renderReport( + "playwright", + ".", + result({ + status: "error", + error: "classifier.dev error (502): \n`bad` gateway", + selectedTests: [tc("a|b.spec.ts")], + skipped: [], + reasons: { "a|b.spec.ts": "judge unavailable\nretry" }, + }), + false, + ); + expect(md).toContain("> Reason: `classifier.dev error (502): 'bad' gateway`"); + expect(md).toContain("| `a\\|b.spec.ts` | RUN | judge unavailable retry |"); + }); + + 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 3466f69..b406d01 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -76,7 +76,7 @@ async function main(): Promise { } else { printSelect(result); } - writeStepSummary(command, result, shadow || full); + appendReport(renderReport(command, cwd, result, shadow || full)); const paths = result.selectedTests.map((t) => t.identity.path); const skippedPaths = result.skipped.map((t) => t.identity.path); @@ -93,7 +93,7 @@ async function main(): Promise { ? "Shadow mode: skipped tests passed, selection missed nothing." : "Shadow mode: MISS, skipped tests failed. Selection alone would have let this through."; console.log(`\n${verdict}`); - appendStepSummary(`\n**${verdict}**\n`); + appendReport(`\n**${verdict}**\n`); return selectedCode || skippedCode; } @@ -204,25 +204,65 @@ function printSelect(result: any): void { } } -function appendStepSummary(markdown: string): void { - const file = process.env.GITHUB_STEP_SUMMARY; - if (file) appendFileSync(file, markdown); +// Written to the job summary, and to LEANEST_REPORT_FILE for the Action's PR comment. +function appendReport(markdown: string): void { + for (const file of [process.env.GITHUB_STEP_SUMMARY, process.env.LEANEST_REPORT_FILE]) { + if (file) appendFileSync(file, markdown); + } } -function writeStepSummary(command: string, result: SelectionResult, runningAll: boolean): void { +/** The Action finds its sticky PR comment by this first line: one comment per framework and dir. */ +export const reportMarker = (command: string, dir: string) => + ``; + +// Judge errors often carry HTTP response bodies: keep them on one line, inside one code span. +const inline = (text: string) => text.replace(/\s+/g, " ").replace(/`/g, "'"); +// A table cell also can't hold a bare pipe. +const cell = (text: string) => inline(text).replace(/\|/g, "\\|"); + +export function renderReport( + command: string, + dir: string, + result: SelectionResult, + runningAll: boolean, +): string { const row = (t: TestCase, decision: string) => - `| \`${t.identity.path}\` | ${decision} | ${result.reasons[t.identity.path] ?? ""} |`; - appendStepSummary( - [ - `### leanest: ${result.selectedTests.length} of ${result.totalTests} ${command} test files selected`, - runningAll ? "\nThe full suite runs anyway (`--shadow` or `--full`).\n" : "", - "| Test | Decision | Reason |", - "| --- | --- | --- |", - ...result.selectedTests.map((t) => row(t, "RUN")), - ...result.skipped.map((t) => row(t, "SKIP")), + `| \`${cell(t.identity.path)}\` | ${decision} | ${cell(result.reasons[t.identity.path] ?? "")} |`; + const table = (rows: string[]) => [ + "| Test | Decision | Reason |", + "| --- | --- | --- |", + ...rows, + ]; + const details = (summary: string, rows: string[]) => + rows.length === 0 + ? [] + : [`
${summary}`, "", ...table(rows), "", "
", ""]; + const runRows = result.selectedTests.map((t) => row(t, "RUN")); + const skipRows = result.skipped.map((t) => row(t, "SKIP")); + + if (result.status === "error") { + return [ + reportMarker(command, dir), + `### leanest: all ${result.totalTests} ${command} test files run`, "", - ].join("\n"), - ); + "> [!WARNING]", + "> **The judge was unavailable, so leanest couldn't select tests and ran the full suite instead.**", + `> Reason: \`${inline(result.error ?? "")}\``, + ">", + "> Nothing was skipped, so this run is as safe as not using leanest. The next run tries the judge again.", + "", + ...details(`${result.totalTests} test files, all RUN`, runRows), + ].join("\n"); + } + + return [ + reportMarker(command, dir), + `### leanest: ${result.selectedTests.length} of ${result.totalTests} ${command} test files selected`, + "", + ...(runningAll ? ["The full suite runs anyway (`--shadow` or `--full`).", ""] : []), + ...(runRows.length > 0 ? [...table(runRows), ""] : []), + ...details(`${skipRows.length} skipped`, skipRows), + ].join("\n"); } function printHelp(): void { diff --git a/src/leanest.ts b/src/leanest.ts index 9b31e82..fcaf641 100644 --- a/src/leanest.ts +++ b/src/leanest.ts @@ -22,7 +22,13 @@ export class Leanest { constructor(cwd?: string, baseRef?: string) { this.cwd = cwd ?? "."; - this.judge = getProvider(); + // A bad provider name fails at evaluate(), so it gets the same full-suite fallback + // as any other judge failure instead of crashing before a report is written. + try { + this.judge = getProvider(); + } catch (error) { + this.judge = { name: "unavailable", evaluate: () => Promise.reject(error) }; + } this.git = new ChangeResolver(baseRef, this.cwd); this.discovery = new TestDiscovery(); this.context = new ContextBuilder();