From cff8dfb4f188676d316d1cae902e64d0f913efa4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 04:15:32 +0000 Subject: [PATCH 1/5] feat(app): resolve a run's execution from its file, title and retry Add GET /test-runs/:id/locate so a link can be built from what the reporter knows before execution ids exist (run id, spec file, title, retry, project) and redirect to /test-run-cases/:id. An unknown test renders a readable 404 page; a signed-out visitor is sent through /login and back to the link. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0145ws9TJpSi3vjZfzdBmB4c --- apps/application/app/pages/login.vue | 9 +- .../routes/test-runs/[id]/locate.get.ts | 134 ++++++++++++++++++ apps/application/shared/test-project-names.ts | 1 + apps/application/tests/run-locate.spec.ts | 99 +++++++++++++ 4 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 apps/application/server/routes/test-runs/[id]/locate.get.ts create mode 100644 apps/application/tests/run-locate.spec.ts diff --git a/apps/application/app/pages/login.vue b/apps/application/app/pages/login.vue index cfb731b9..ad21ea48 100644 --- a/apps/application/app/pages/login.vue +++ b/apps/application/app/pages/login.vue @@ -13,6 +13,13 @@ const state = reactive({ const loading = ref(false); const error = ref(''); +// Where to land after signing in: a same-origin path handed over by a link +// that needed a session first (the reporter's per-failure links), else home. +function redirectTarget(): string { + const target = route.query.redirect; + return typeof target === 'string' && target.startsWith('/') && !target.startsWith('//') ? target : '/'; +} + // Fresh instance with auth enabled and zero users: the login form can never // succeed, so show a first-admin setup form instead (mirrors the documented // `POST /api/auth/setup` curl flow, but reachable from the UI). @@ -124,7 +131,7 @@ async function handleLogin() { title: 'Login successful', color: 'success', }); - router.push('/'); + router.push(redirectTarget()); } catch (err: unknown) { const errorMessage = err && typeof err === 'object' && 'data' in err ? (err.data as { message?: string })?.message : undefined; diff --git a/apps/application/server/routes/test-runs/[id]/locate.get.ts b/apps/application/server/routes/test-runs/[id]/locate.get.ts new file mode 100644 index 00000000..acdd95ae --- /dev/null +++ b/apps/application/server/routes/test-runs/[id]/locate.get.ts @@ -0,0 +1,134 @@ +import type { H3Event } from 'h3'; +import { and, eq } from 'drizzle-orm'; +import { getDatabase } from '../../../database'; +import { testCases, testRuns, testRunsCases } from '../../../database/schema'; +import { getCurrentUser, isAuthEnabled } from '../../../utils/auth'; +import { canAccessProject } from '../../../utils/project-access'; +import { FAILED_STATUS_KEYS } from '#shared/utils/test-counts'; + +/** + * Resolve an execution from what the reporter knows before the server assigns + * ids — the run id plus the test's spec file, title, retry and Playwright + * project — and redirect to its page. The reporter prints these links the + * moment a test fails, in streaming and batch mode alike, so the link has to + * be computable without waiting for an execution id. + * + * `retry` and `browser` are preferences, not filters: when the exact attempt + * is missing (a retry that was never persisted, a project name that changed) + * the closest execution of the same test still resolves rather than 404ing. + */ +export default eventHandler(async (event) => { + setResponseHeader(event, 'Cache-Control', 'no-store'); + setResponseHeader(event, 'X-Robots-Tag', 'noindex, nofollow'); + + const runId = Number.parseInt(getRouterParam(event, 'id') ?? '', 10); + const query = getQuery(event); + const file = queryString(query.file); + const title = queryString(query.title); + const retry = queryString(query.retry); + const browser = queryString(query.browser); + const wantedRetry = retry !== null && /^\d+$/.test(retry) ? Number(retry) : null; + + if (!Number.isInteger(runId) || runId <= 0 || !file || !title) { + return notFoundPage(event, 'This link is missing the run, spec file or test title it should point at.', null); + } + + if (isAuthEnabled(event) && !(await getCurrentUser(event))) { + const url = getRequestURL(event); + return sendRedirect(event, `/login?redirect=${encodeURIComponent(url.pathname + url.search)}`); + } + + const db = await getDatabase(); + const [run] = await db.select({ projectId: testRuns.projectId }).from(testRuns).where(eq(testRuns.id, runId)); + const user = isAuthEnabled(event) ? await getCurrentUser(event) : null; + if (!run || !(await canAccessProject(db, user, run.projectId))) { + return notFoundPage(event, `Run #${runId} does not exist, or it was deleted.`, null); + } + + const candidates = await db + .select({ + id: testRunsCases.id, + status: testRunsCases.status, + retries: testRunsCases.retries, + browserName: testRunsCases.browserName, + }) + .from(testRunsCases) + .innerJoin(testCases, eq(testRunsCases.testCaseId, testCases.id)) + .where(and(eq(testRunsCases.testRunId, runId), eq(testCases.filePath, file), eq(testCases.title, title))); + + const match = pickExecution(candidates, wantedRetry, browser); + if (!match) { + return notFoundPage(event, `Run #${runId} has no execution of "${title}" in ${file}.`, runId); + } + + return sendRedirect(event, `/test-run-cases/${match.id}`); +}); + +interface Candidate { + id: number; + status: string; + retries: number | null; + browserName: string | null; +} + +/** + * The best execution for the requested attempt: the exact project and retry + * when persisted, otherwise the failing one, otherwise the latest attempt. + */ +export function pickExecution(candidates: Candidate[], retry: number | null, browser: string | null): Candidate | null { + const failed = new Set(FAILED_STATUS_KEYS); + const score = (c: Candidate): number[] => [ + browser !== null && c.browserName === browser ? 1 : 0, + retry !== null && (c.retries ?? 0) === retry ? 1 : 0, + failed.has(c.status) ? 1 : 0, + c.retries ?? 0, + c.id, + ]; + let best: Candidate | null = null; + let bestScore: number[] = []; + for (const candidate of candidates) { + const s = score(candidate); + if (!best || compareScores(s, bestScore) > 0) { + best = candidate; + bestScore = s; + } + } + return best; +} + +function compareScores(a: number[], b: number[]): number { + for (let i = 0; i < a.length; i++) { + const diff = (a[i] ?? 0) - (b[i] ?? 0); + if (diff !== 0) return diff; + } + return 0; +} + +function queryString(value: unknown): string | null { + if (Array.isArray(value)) value = value[0]; + return typeof value === 'string' && value !== '' ? value : null; +} + +function escapeHtml(text: string): string { + return text + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + +/** A readable 404 for someone who followed a link from a terminal or a CI log. */ +function notFoundPage(event: H3Event, reason: string, runId: number | null): string { + setResponseStatus(event, 404); + setResponseHeader(event, 'Content-Type', 'text/html; charset=utf-8'); + const runLink = runId ? `

Open run #${runId} and find the test there.

` : ''; + return ( + 'Test not found' + + '' + + '

Piwi could not find that test

' + + `

${escapeHtml(reason)}

` + + `

The run may not have finished uploading yet, or its results may have been pruned by retention.

${runLink}` + + '' + ); +} diff --git a/apps/application/shared/test-project-names.ts b/apps/application/shared/test-project-names.ts index ae1a5554..9af3d0c4 100644 --- a/apps/application/shared/test-project-names.ts +++ b/apps/application/shared/test-project-names.ts @@ -111,6 +111,7 @@ export const PROJECT = { REVOKED_KEY: 'revoked-key-test', RUN_COMPARE: 'run-compare', RUN_LABEL: 'run-label-test', + RUN_LOCATE: 'run-locate-test', RUN_PAGE_FILTERS: 'run-page-filters-test', RUN_SUMMARY_TEST: 'run-summary-test', SHARDING_TEST: 'sharding-test', diff --git a/apps/application/tests/run-locate.spec.ts b/apps/application/tests/run-locate.spec.ts new file mode 100644 index 00000000..5db603f6 --- /dev/null +++ b/apps/application/tests/run-locate.spec.ts @@ -0,0 +1,99 @@ +import { test, expect } from './fixtures'; +import { PROJECT } from '#shared/test-project-names'; + +const FILE = 'tests/locate.spec.ts'; +const TITLE = 'locate me & friends'; + +/** + * The reporter prints `/test-runs/:id/locate?file=…&title=…&retry=…&browser=…` + * links the moment a test fails, before the server has assigned execution ids. + * The route resolves them to the execution page. + */ +test.describe.serial('Run execution locator', () => { + let runId: number; + let executions: Array<{ executionId: number; retries: number; status: string }>; + + test('a run with a retried failure provides executions to resolve', async ({ request }) => { + const submit = await request.post('/api/test-runs/submit', { + data: { + projectName: PROJECT.RUN_LOCATE, + status: 'failed', + startTime: new Date().toISOString(), + duration: 4000, + totalTests: 2, + passedTests: 1, + failedTests: 1, + skippedTests: 0, + testCases: [ + { + title: TITLE, + status: 'failed', + duration: 1200, + retries: 0, + location: `${FILE}:5:3`, + browser: { projectName: 'chromium', browserName: 'chromium' }, + error: 'Error: boom', + }, + { + title: TITLE, + status: 'failed', + duration: 1100, + retries: 1, + location: `${FILE}:5:3`, + browser: { projectName: 'chromium', browserName: 'chromium' }, + error: 'Error: boom', + }, + { + title: 'passes', + status: 'passed', + duration: 100, + retries: 0, + location: `${FILE}:12:3`, + browser: { projectName: 'chromium', browserName: 'chromium' }, + }, + ], + }, + }); + expect(submit.ok()).toBeTruthy(); + ({ runId } = await submit.json()); + + const run = (await (await request.get(`/api/test-runs/${runId}`)).json()) as { + testCases: Array<{ executionId: number; title: string; retries: number; status: string }>; + }; + executions = run.testCases.filter((c) => c.title === TITLE); + expect(executions).toHaveLength(2); + }); + + test('redirects to the execution of the requested attempt', async ({ request }) => { + const params = new URLSearchParams({ file: FILE, title: TITLE, retry: '1', browser: 'chromium' }); + const res = await request.get(`/test-runs/${runId}/locate?${params}`, { maxRedirects: 0 }); + expect(res.status()).toBe(302); + const wanted = executions.find((e) => e.retries === 1)!; + expect(res.headers()['location']).toBe(`/test-run-cases/${wanted.executionId}`); + }); + + test('falls back to the latest failing attempt when the retry is unknown', async ({ request }) => { + const params = new URLSearchParams({ file: FILE, title: TITLE, retry: '7' }); + const res = await request.get(`/test-runs/${runId}/locate?${params}`, { maxRedirects: 0 }); + expect(res.status()).toBe(302); + const latest = executions.reduce((a, b) => (b.retries > a.retries ? b : a)); + expect(res.headers()['location']).toBe(`/test-run-cases/${latest.executionId}`); + }); + + test('renders a readable 404 page when the test is not in the run', async ({ request }) => { + const params = new URLSearchParams({ file: FILE, title: 'never ran', retry: '0' }); + const res = await request.get(`/test-runs/${runId}/locate?${params}`, { maxRedirects: 0 }); + expect(res.status()).toBe(404); + expect(res.headers()['content-type']).toContain('text/html'); + const body = await res.text(); + expect(body).toContain('Piwi could not find that test'); + expect(body).toContain(`/test-runs/${runId}`); + }); + + test('renders a readable 404 page for an unknown run', async ({ request }) => { + const params = new URLSearchParams({ file: FILE, title: TITLE, retry: '0' }); + const res = await request.get(`/test-runs/999999999/locate?${params}`, { maxRedirects: 0 }); + expect(res.status()).toBe(404); + expect(await res.text()).toContain('does not exist'); + }); +}); From 83e22079ce9a1e3d087ae42ac327fb90160dbe44 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 2 Sep 2026 04:15:39 +0000 Subject: [PATCH 2/5] feat(reporter): print a dashboard link per failed test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Print one `✗ → <url>` line per test whose final attempt failed, as soon as the run id is known (immediately while streaming, after the submit in batch mode). The link targets the dashboard's locate route so it needs no execution id. GitHub Actions gets the same list in the job summary (capped at 20) and a piwi_failed_count step output; the GitLab dotenv and the JSON output file carry the count and the failures too. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145ws9TJpSi3vjZfzdBmB4c --- apps/docs/ci.md | 27 +++++-- apps/docs/reporter.md | 3 +- packages/reporter/README.md | 24 ++++-- .../src/internal/submit/run-submitter.ts | 11 ++- .../src/internal/support/ci-output.ts | 56 ++++++++++--- .../src/internal/support/failure-links.ts | 74 +++++++++++++++++ packages/reporter/src/public/reporter.ts | 34 +++++++- packages/reporter/tests/ci-output.spec.ts | 79 ++++++++++++++++++- packages/reporter/tests/failure-links.spec.ts | 52 ++++++++++++ 9 files changed, 331 insertions(+), 29 deletions(-) create mode 100644 packages/reporter/src/internal/support/failure-links.ts create mode 100644 packages/reporter/tests/failure-links.spec.ts diff --git a/apps/docs/ci.md b/apps/docs/ci.md index d5ad0b3b..d124a628 100644 --- a/apps/docs/ci.md +++ b/apps/docs/ci.md @@ -135,9 +135,22 @@ After results land, the reporter surfaces the dashboard run URL wherever a pipel a later step (a Slack post, a deploy gate, a PR comment) doesn't have to scrape stdout. All of it is best-effort — a failure in any channel is logged and never fails your run. -**Always** — a `View run: <url>` line in the log. +**Always** — one line per failed test, printed the moment its final attempt fails, then a +`View run: <url>` line once the run lands: -**GitHub Actions (automatic)** — step outputs, a job-summary link, and a `::notice::` annotation: +``` +[Piwi Dashboard] ✗ applies the discount code → https://piwi.example.com/test-runs/42/locate?file=tests%2Fcheckout.spec.ts&title=applies%20the%20discount%20code&retry=1&browser=chromium +[Piwi Dashboard] View run: https://piwi.example.com/test-runs/42 +``` + +The per-test link resolves to the failing execution's page (`/test-run-cases/:id`) and works in +streaming and batch mode alike — it is built from what the reporter knows at that moment (run id, +spec file, title, retry and Playwright project), so it prints while the run is still going. A link to a +test the dashboard cannot find (results pruned by retention, an upload that never landed) renders a +readable "not found" page instead of an error. + +**GitHub Actions (automatic)** — step outputs, a job summary listing the failed tests with their links +(20 at most, the rest counted as "+N more"), and a `::notice::` annotation: ```yaml - run: npx playwright test @@ -148,10 +161,11 @@ best-effort — a failure in any channel is logged and never fails your run. if: always() ``` -Available outputs: `piwi_run_url`, `piwi_run_id`, `piwi_run_status`, `piwi_project_id`. +Available outputs: `piwi_run_url`, `piwi_run_id`, `piwi_run_status`, `piwi_failed_count`, `piwi_project_id`. **GitLab CI (automatic)** — a dotenv report (`piwi.env` by default, override with `PIWI_DOTENV_FILE`) -carrying `PIWI_RUN_URL`, `PIWI_RUN_ID`, `PIWI_RUN_STATUS`, `PIWI_PROJECT_ID` and `PIWI_CI_BUILD_URL`. +carrying `PIWI_RUN_URL`, `PIWI_RUN_ID`, `PIWI_RUN_STATUS`, `PIWI_FAILED_COUNT`, `PIWI_PROJECT_ID` and +`PIWI_CI_BUILD_URL`. Declare it so later jobs inherit the variables: ```yaml @@ -169,9 +183,12 @@ e2e: - run: npx playwright test env: PIWI_OUTPUT_FILE: piwi-run.json -- run: cat piwi-run.json # { runUrl, runId, projectId, projectName, status, ciBuildUrl } +- run: cat piwi-run.json # { runUrl, runId, projectId, projectName, status, ciBuildUrl, failedCount, failures } ``` +`failures` lists every test whose final attempt failed as `{ title, file, retry, browser, url }`, with +`url` the same per-test link the log prints. + ## Pull-request feedback The run URL above is a link somebody has to click. Piwi can instead post the result onto the pull request itself, which diff --git a/apps/docs/reporter.md b/apps/docs/reporter.md index 09708f23..3ea6db7f 100644 --- a/apps/docs/reporter.md +++ b/apps/docs/reporter.md @@ -189,7 +189,8 @@ By default, the reporter streams test results to the dashboard in real-time as t 3. With `liveFileUploads` (the default), each test's trace and attachments are uploaded right after the test finishes, so they are viewable on the test case page while the run is still in progress 4. The dashboard UI shows a live progress bar and test results as they arrive 5. While a test runs, the steps it is executing (Playwright `pw:api` actions, `pw:expect` assertions, and hook/fixture steps) stream to the run page as they happen — each running test's row shows the step it is on right now. The polling attempts of `pw:assert` steps are deliberately not streamed; the persisted step events on a completed test still carry everything -6. When tests finish, the reporter finalizes the run with the overall status +6. When a test's final attempt fails, the reporter prints `[Piwi Dashboard] ✗ <title> → <url>` right away — the link opens that execution on the dashboard, so you can start reading the failure while the rest of the suite is still running. In batch mode the same lines print after the upload +7. When tests finish, the reporter finalizes the run with the overall status and prints `View run: <url>` (see [CI → Getting the run URL back out](./ci#getting-the-run-url-back-out-of-ci) for the step outputs and job summary that go with it) ### Disabling streaming diff --git a/packages/reporter/README.md b/packages/reporter/README.md index b60ed09e..0bd6aed5 100644 --- a/packages/reporter/README.md +++ b/packages/reporter/README.md @@ -246,25 +246,31 @@ When `collectCiInfo` is enabled (default), the reporter auto-detects: After a run is submitted, the reporter surfaces the dashboard run URL so a later CI step (a custom email, a Slack message, a deploy gate) can pick it up without -scraping the log. The URL is always printed as `View run: <url>`, and in -addition: +scraping the log. The URL is always printed as `View run: <url>`, preceded by +one `✗ <title> → <url>` line per failed test — each linking straight to that +execution on the dashboard, printed the moment the test's final attempt fails +(in streaming mode, before the run is over). In addition: - **Any CI — JSON output file.** Set `outputFile` (or `PIWI_OUTPUT_FILE`) and the reporter writes a small JSON file when the run lands: ```json - { "runUrl": "https://piwi.example.com/test-runs/1234", "runId": 1234, "projectId": 5, "projectName": "checkout", "status": "passed", "ciBuildUrl": "https://ci.example.com/build/9" } + { "runUrl": "https://piwi.example.com/test-runs/1234", "runId": 1234, "projectId": 5, "projectName": "checkout", "status": "passed", "ciBuildUrl": "https://ci.example.com/build/9", "failedCount": 0, "failures": [] } ``` + `failures` lists every test whose final attempt failed as + `{ title, file, retry, browser, url }`. + Read it from any pipeline, e.g. `node -e "console.log(require('./piwi-run.json').runUrl)"` (portable) or `cat piwi-run.json` and parse it in your email step. In Jenkins, `def run = readJSON file: 'piwi-run.json'` then use `run.runUrl`. - **GitHub Actions (automatic).** When `GITHUB_ACTIONS` is set, the reporter appends step outputs to `$GITHUB_OUTPUT` (`piwi_run_url`, `piwi_run_id`, - `piwi_project_id`, `piwi_run_status`), writes a markdown link to the job - summary, and prints a `::notice::` annotation. Give the test step an `id` and a - downstream step can read it: + `piwi_project_id`, `piwi_run_status`, `piwi_failed_count`), writes a markdown + link plus the failed tests with their links to the job summary (20 at most, + the rest counted), and prints a `::notice::` annotation. Give the test step an + `id` and a downstream step can read it: ```yaml - id: tests @@ -273,8 +279,10 @@ addition: ``` - **GitLab CI (automatic).** When `GITLAB_CI` is set, the reporter writes a - dotenv report (`piwi.env` by default, override with `PIWI_DOTENV_FILE`). - Declare it so later jobs inherit `$PIWI_RUN_URL`: + dotenv report (`piwi.env` by default, override with `PIWI_DOTENV_FILE`) + carrying `PIWI_RUN_URL`, `PIWI_RUN_ID`, `PIWI_RUN_STATUS`, `PIWI_FAILED_COUNT`, + `PIWI_PROJECT_ID` and `PIWI_CI_BUILD_URL`. Declare it so later jobs inherit + `$PIWI_RUN_URL`: ```yaml test: diff --git a/packages/reporter/src/internal/submit/run-submitter.ts b/packages/reporter/src/internal/submit/run-submitter.ts index 41e49c7d..86bcd396 100644 --- a/packages/reporter/src/internal/submit/run-submitter.ts +++ b/packages/reporter/src/internal/submit/run-submitter.ts @@ -11,6 +11,7 @@ import { computePerformanceSummary } from '../collect/step-analyzer.js'; import { resolveOverallStatus, serializeRun } from './serializer.js'; import { runUrl } from '../support/run-url.js'; import { emitRunOutputs, ciBuildUrlFromMetadata, type RunOutput } from '../support/ci-output.js'; +import type { FailureLinks } from '../support/failure-links.js'; import type { CollectedTestCase, SetupStep, FilterDetails } from '../../types.js'; /** @@ -68,6 +69,7 @@ export class RunSubmitter { * @param recovery Crash-recovery persistence. * @param streamManager Streaming session (may be `null` when streaming is disabled). * @param logger Prefixed logger. + * @param failureLinks Failed tests collected during the run, for the per-failure links. */ constructor( private readonly httpClient: HttpClient, @@ -75,6 +77,7 @@ export class RunSubmitter { private readonly recovery: CrashRecovery, private readonly streamManager: StreamManager | null, private readonly logger: Logger = new Logger(), + private readonly failureLinks: FailureLinks | null = null, ) {} /** Run the fallback ladder for a completed test run. */ @@ -129,7 +132,12 @@ export class RunSubmitter { outcome = await this.tryUploadJSON(run, overallStatus, duration, auth); } - if (outcome.output) emitRunOutputs(outcome.output, this.logger, run.options.outputFile); + if (outcome.output) { + // Failures that had no run id yet (batch mode, or a stream that never + // opened) get their link lines now, ahead of the run URL. + this.failureLinks?.printPending(outcome.output.runId); + emitRunOutputs(outcome.output, this.logger, run.options.outputFile); + } } /** Assemble a CI-facing run output, or `null` when the server returned no run id. */ @@ -147,6 +155,7 @@ export class RunSubmitter { projectName: run.options.projectName!, status, ciBuildUrl: ciBuildUrlFromMetadata(run.metadata), + failures: this.failureLinks?.resolve(runId) ?? [], }; } diff --git a/packages/reporter/src/internal/support/ci-output.ts b/packages/reporter/src/internal/support/ci-output.ts index 6dbbf66e..03d7ece4 100644 --- a/packages/reporter/src/internal/support/ci-output.ts +++ b/packages/reporter/src/internal/support/ci-output.ts @@ -1,13 +1,14 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import type { Logger } from './logger.js'; +import type { FailureLink } from './failure-links.js'; import { errorMessage } from './errors.js'; /** * The facts about a just-submitted run that CI pipelines want to consume: the - * clickable dashboard URL plus the identifiers and status behind it. Produced by - * the submit ladder once a run lands, and rendered into CI-native channels by - * `emitRunOutputs`. + * clickable dashboard URL plus the identifiers and status behind it, and the + * failed tests with their execution links. Produced by the submit ladder once + * a run lands, and rendered into CI-native channels by `emitRunOutputs`. */ export interface RunOutput { /** Clickable dashboard URL for the run (`<serverUrl>/test-runs/<id>`). */ @@ -22,8 +23,13 @@ export interface RunOutput { status: string; /** Link back to the CI build/job that produced the run, when detected. */ ciBuildUrl?: string | null; + /** Tests whose final attempt failed, each with its dashboard link. */ + failures: FailureLink[]; } +/** The GitHub job summary lists at most this many failed tests; the rest are counted. */ +export const SUMMARY_MAX_FAILURES = 20; + /** * Surface the dashboard run URL wherever a CI pipeline can pick it up, so a * downstream step (a custom email, a Slack post, a deploy gate) can consume it @@ -35,8 +41,8 @@ export interface RunOutput { * 2. `outputFile` (opt-in): a portable JSON file every CI can read * (`cat piwi-run.json`, Jenkins `readJSON`, etc.). * 3. GitHub Actions (auto): step outputs on `$GITHUB_OUTPUT` - * (`steps.<id>.outputs.piwi_run_url`), a markdown link on - * `$GITHUB_STEP_SUMMARY`, and a `::notice::` workflow annotation. + * (`steps.<id>.outputs.piwi_run_url`), a markdown link plus the failed + * tests on `$GITHUB_STEP_SUMMARY`, and a `::notice::` workflow annotation. * 4. GitLab CI (auto): a dotenv report file (default `piwi.env`) to be wired as * `artifacts:reports:dotenv:` so later jobs inherit `$PIWI_RUN_URL`. */ @@ -68,6 +74,8 @@ function writeOutputFile(file: string, output: RunOutput, logger: Logger): void projectName: output.projectName, status: output.status, ciBuildUrl: output.ciBuildUrl ?? null, + failedCount: output.failures.length, + failures: output.failures, }, null, 2, @@ -79,17 +87,18 @@ function writeOutputFile(file: string, output: RunOutput, logger: Logger): void } } -/** GitHub Actions: step outputs, a job-summary link, and a workflow annotation. */ +/** GitHub Actions: step outputs, a job summary with the failed tests, and a workflow annotation. */ function emitGitHubActions(output: RunOutput, env: NodeJS.ProcessEnv, logger: Logger): void { const pairs: Array<[string, string]> = [ ['piwi_run_url', output.runUrl], ['piwi_run_id', String(output.runId)], ['piwi_run_status', output.status], + ['piwi_failed_count', String(output.failures.length)], ]; if (output.projectId != null) pairs.push(['piwi_project_id', String(output.projectId)]); - // Values are single-line (URL / id / status), so the plain `key=value` form is - // safe — no heredoc delimiter needed. + // Values are single-line (URL / id / status / count), so the plain + // `key=value` form is safe — no heredoc delimiter needed. if (env.GITHUB_OUTPUT) { appendFileLines( env.GITHUB_OUTPUT, @@ -101,7 +110,13 @@ function emitGitHubActions(output: RunOutput, env: NodeJS.ProcessEnv, logger: Lo if (env.GITHUB_STEP_SUMMARY) { appendFileLines( env.GITHUB_STEP_SUMMARY, - ['### Piwi test run', '', `[View run](${output.runUrl}) — **${output.status}**`, ''], + [ + '### Piwi test run', + '', + `[View run](${output.runUrl}) — **${output.status}**`, + '', + ...summaryFailureLines(output), + ], logger, 'step summary', ); @@ -110,10 +125,31 @@ function emitGitHubActions(output: RunOutput, env: NodeJS.ProcessEnv, logger: Lo process.stdout.write(`::notice title=Piwi test run::${output.runUrl}\n`); } +/** Markdown list of the failed tests for the job summary, capped at `SUMMARY_MAX_FAILURES`. */ +function summaryFailureLines(output: RunOutput): string[] { + if (output.failures.length === 0) return []; + const lines = output.failures + .slice(0, SUMMARY_MAX_FAILURES) + .map((f) => `- ❌ [${escapeMarkdown(f.title)}](${f.url}) — \`${f.file}\``); + const hidden = output.failures.length - SUMMARY_MAX_FAILURES; + if (hidden > 0) lines.push(`- +${hidden} more`); + lines.push(''); + return lines; +} + +function escapeMarkdown(text: string): string { + return text.replace(/[\\`*_[\]]/g, (ch) => `\\${ch}`); +} + /** GitLab CI: a dotenv report file so later jobs inherit the run URL as a variable. */ function emitGitLabDotenv(output: RunOutput, env: NodeJS.ProcessEnv, logger: Logger): void { const file = env.PIWI_DOTENV_FILE || 'piwi.env'; - const lines = [`PIWI_RUN_URL=${output.runUrl}`, `PIWI_RUN_ID=${output.runId}`, `PIWI_RUN_STATUS=${output.status}`]; + const lines = [ + `PIWI_RUN_URL=${output.runUrl}`, + `PIWI_RUN_ID=${output.runId}`, + `PIWI_RUN_STATUS=${output.status}`, + `PIWI_FAILED_COUNT=${output.failures.length}`, + ]; if (output.projectId != null) lines.push(`PIWI_PROJECT_ID=${output.projectId}`); if (output.ciBuildUrl) lines.push(`PIWI_CI_BUILD_URL=${output.ciBuildUrl}`); try { diff --git a/packages/reporter/src/internal/support/failure-links.ts b/packages/reporter/src/internal/support/failure-links.ts new file mode 100644 index 00000000..1ace3d71 --- /dev/null +++ b/packages/reporter/src/internal/support/failure-links.ts @@ -0,0 +1,74 @@ +import type { Logger } from './logger.js'; + +/** A test whose final attempt failed, with what the dashboard needs to find its execution. */ +export interface FailedTest { + title: string; + /** Spec path relative to the working directory, POSIX separators. */ + file: string; + /** Attempt index of the failing (final) attempt. */ + retry: number; + /** Playwright project name the test ran under, when known. */ + browser: string | null; +} + +/** A failed test paired with the dashboard link that resolves to its execution. */ +export interface FailureLink extends FailedTest { + url: string; +} + +/** + * Deterministic dashboard URL for one execution, built from what the reporter + * knows before the server assigns execution ids: the run id plus the test's + * file, title, retry and project. The dashboard's `/test-runs/:id/locate` + * route resolves it to `/test-run-cases/:id`. + */ +export function caseLocateUrl(serverUrl: string, runId: number | string, test: FailedTest): string { + const params = [ + `file=${encodeURIComponent(test.file)}`, + `title=${encodeURIComponent(test.title)}`, + `retry=${test.retry}`, + ]; + if (test.browser) params.push(`browser=${encodeURIComponent(test.browser)}`); + return `${serverUrl.replace(/\/+$/, '')}/test-runs/${runId}/locate?${params.join('&')}`; +} + +/** The terminal line printed for one failed test. */ +export function formatFailureLine(link: FailureLink): string { + return `✗ ${link.title} → ${link.url}`; +} + +/** + * Collects the failed tests of a run and prints one link line per failure as + * soon as a run id is known: right after the test while streaming, after the + * submit in batch mode. Lines are never printed twice. + */ +export class FailureLinks { + private readonly failures: FailedTest[] = []; + private printed = 0; + + constructor( + private readonly serverUrl: string, + private readonly logger: Logger, + ) {} + + /** Number of failed tests recorded so far. */ + get count(): number { + return this.failures.length; + } + + add(test: FailedTest): void { + this.failures.push(test); + } + + /** Every recorded failure with its link under `runId`. */ + resolve(runId: number | string): FailureLink[] { + return this.failures.map((test) => ({ ...test, url: caseLocateUrl(this.serverUrl, runId, test) })); + } + + /** Print the lines that have not been printed yet. */ + printPending(runId: number | string): void { + const links = this.resolve(runId); + for (const link of links.slice(this.printed)) this.logger.info(formatFailureLine(link)); + this.printed = links.length; + } +} diff --git a/packages/reporter/src/public/reporter.ts b/packages/reporter/src/public/reporter.ts index cd047d82..705a941a 100644 --- a/packages/reporter/src/public/reporter.ts +++ b/packages/reporter/src/public/reporter.ts @@ -31,6 +31,7 @@ import { collectTestMetadata, collectTestTags } from '../internal/collect/test-m import { buildErrorText } from '../internal/collect/error-text.js'; import { RunSubmitter } from '../internal/submit/run-submitter.js'; import { Logger } from '../internal/support/logger.js'; +import { FailureLinks } from '../internal/support/failure-links.js'; import type { CollectedTestCase, StreamEvent, SetupStep, FilterDetails, TestAnnotation } from '../types.js'; /** @@ -39,8 +40,12 @@ import type { CollectedTestCase, StreamEvent, SetupStep, FilterDetails, TestAnno * retry command) match on every platform — `path.relative` yields backslashes on Windows. */ function testLocation(test: TestCase): string { - const relativeFilePath = path.relative(process.cwd(), test.location.file).split(path.sep).join('/'); - return `${relativeFilePath}:${test.location.line}:${test.location.column}`; + return `${testFile(test)}:${test.location.line}:${test.location.column}`; +} + +/** Spec path relative to the working directory, POSIX separators — the `filePath` the dashboard stores. */ +function testFile(test: TestCase): string { + return path.relative(process.cwd(), test.location.file).split(path.sep).join('/'); } /** @@ -91,6 +96,7 @@ export class PiwiDashboardReporter { private streamManager: StreamManager | null = null; private recovery: CrashRecovery; private submitter: RunSubmitter; + private readonly failureLinks: FailureLinks; private readonly logger: Logger; static wrapConfig = wrapConfig; @@ -110,6 +116,7 @@ export class PiwiDashboardReporter { this.uploader = new Uploader(this.httpClient, this.fileHandler, logger); this.recovery = new CrashRecovery(this.options.projectName!, logger); this.metadataCollector = new MetadataCollector(logger); + this.failureLinks = new FailureLinks(this.httpClient.baseUrl, logger); const streamBuffer = new StreamBuffer(this.options.projectName!); streamBuffer.clearStale(); @@ -126,7 +133,14 @@ export class PiwiDashboardReporter { ); } - this.submitter = new RunSubmitter(this.httpClient, this.uploader, this.recovery, this.streamManager, logger); + this.submitter = new RunSubmitter( + this.httpClient, + this.uploader, + this.recovery, + this.streamManager, + logger, + this.failureLinks, + ); } /** Playwright reporter hook: called once at the start of the test run */ @@ -398,6 +412,20 @@ export class PiwiDashboardReporter { // the link even though the whole run isn't collected yet. if (status === 'didnotrun') linkBlockedTests(this.testCases); + // The final attempt of a failing test gets a dashboard link, printed as + // soon as a run id exists (immediately while streaming, after the submit otherwise). + const isFailure = status === 'failed' || status === 'timedOut'; + if (isFailure && result.retry >= (test.retries ?? 0)) { + this.failureLinks.add({ + title: test.title, + file: testFile(test), + retry: result.retry, + browser: typeof testCase.browser?.projectName === 'string' ? testCase.browser.projectName : null, + }); + } + const liveRunId = this.streamManager?.runId; + if (liveRunId != null) this.failureLinks.printPending(liveRunId); + if (this.streamManager) { this.streamManager.queueEvent(toWireTestCase(testCase) as StreamEvent); if (this.options.liveFileUploads) this.streamManager.scheduleLiveUpload(testCase); diff --git a/packages/reporter/tests/ci-output.spec.ts b/packages/reporter/tests/ci-output.spec.ts index cf16ed00..8e120774 100644 --- a/packages/reporter/tests/ci-output.spec.ts +++ b/packages/reporter/tests/ci-output.spec.ts @@ -2,7 +2,13 @@ import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { emitRunOutputs, ciBuildUrlFromMetadata, type RunOutput } from '../src/internal/support/ci-output.js'; +import { + emitRunOutputs, + ciBuildUrlFromMetadata, + SUMMARY_MAX_FAILURES, + type RunOutput, +} from '../src/internal/support/ci-output.js'; +import type { FailureLink } from '../src/internal/support/failure-links.js'; import { Logger } from '../src/internal/support/logger.js'; const OUTPUT: RunOutput = { @@ -12,8 +18,19 @@ const OUTPUT: RunOutput = { projectName: 'checkout', status: 'passed', ciBuildUrl: 'https://ci.example.com/build/9', + failures: [], }; +function failure(i: number): FailureLink { + return { + title: `test ${i}`, + file: 'tests/checkout.spec.ts', + retry: 1, + browser: 'chromium', + url: `https://dash.example.com/test-runs/42/locate?file=tests%2Fcheckout.spec.ts&title=test%20${i}&retry=1&browser=chromium`, + }; +} + let tmpDir: string; const silentLogger = new Logger(false); @@ -38,9 +55,19 @@ describe('emitRunOutputs — universal output file', () => { projectName: 'checkout', status: 'passed', ciBuildUrl: 'https://ci.example.com/build/9', + failedCount: 0, + failures: [], }); }); + it('lists the failed tests with their links in the file', () => { + const file = path.join(tmpDir, 'piwi-run.json'); + emitRunOutputs({ ...OUTPUT, status: 'failed', failures: [failure(1)] }, silentLogger, file, {}); + const parsed = JSON.parse(fs.readFileSync(file, 'utf8')); + expect(parsed.failedCount).toBe(1); + expect(parsed.failures).toEqual([failure(1)]); + }); + it('does not write a file when outputFile is not set', () => { emitRunOutputs(OUTPUT, silentLogger, undefined, {}); expect(fs.readdirSync(tmpDir)).toEqual([]); @@ -71,14 +98,63 @@ describe('emitRunOutputs — GitHub Actions', () => { expect(outputs).toContain('piwi_run_id=42'); expect(outputs).toContain('piwi_run_status=passed'); expect(outputs).toContain('piwi_project_id=7'); + expect(outputs).toContain('piwi_failed_count=0'); const summary = fs.readFileSync(summaryFile, 'utf8'); expect(summary).toContain('[View run](https://dash.example.com/test-runs/42)'); expect(summary).toContain('**passed**'); + expect(summary).not.toContain('❌'); expect(stdout).toHaveBeenCalledWith('::notice title=Piwi test run::https://dash.example.com/test-runs/42\n'); }); + it('lists each failed test with its link in the job summary and counts them in the outputs', () => { + const outputFile = path.join(tmpDir, 'gh-output'); + const summaryFile = path.join(tmpDir, 'gh-summary'); + vi.spyOn(process.stdout, 'write').mockReturnValue(true); + + emitRunOutputs({ ...OUTPUT, status: 'failed', failures: [failure(1), failure(2)] }, silentLogger, undefined, { + GITHUB_ACTIONS: 'true', + GITHUB_OUTPUT: outputFile, + GITHUB_STEP_SUMMARY: summaryFile, + }); + + expect(fs.readFileSync(outputFile, 'utf8')).toContain('piwi_failed_count=2'); + const summary = fs.readFileSync(summaryFile, 'utf8'); + expect(summary).toContain(`- ❌ [test 1](${failure(1).url}) — \`tests/checkout.spec.ts\``); + expect(summary).toContain(`- ❌ [test 2](${failure(2).url})`); + expect(summary).not.toContain('more'); + }); + + it('caps the job summary list and counts the rest', () => { + const summaryFile = path.join(tmpDir, 'gh-summary'); + vi.spyOn(process.stdout, 'write').mockReturnValue(true); + const failures = Array.from({ length: SUMMARY_MAX_FAILURES + 3 }, (_, i) => failure(i)); + + emitRunOutputs({ ...OUTPUT, status: 'failed', failures }, silentLogger, undefined, { + GITHUB_ACTIONS: 'true', + GITHUB_STEP_SUMMARY: summaryFile, + }); + + const summary = fs.readFileSync(summaryFile, 'utf8'); + expect(summary.match(/- ❌ /g)).toHaveLength(SUMMARY_MAX_FAILURES); + expect(summary).toContain('- +3 more'); + }); + + it('escapes markdown in a test title', () => { + const summaryFile = path.join(tmpDir, 'gh-summary'); + vi.spyOn(process.stdout, 'write').mockReturnValue(true); + + emitRunOutputs( + { ...OUTPUT, status: 'failed', failures: [{ ...failure(1), title: 'renders [a] *b*' }] }, + silentLogger, + undefined, + { GITHUB_ACTIONS: 'true', GITHUB_STEP_SUMMARY: summaryFile }, + ); + + expect(fs.readFileSync(summaryFile, 'utf8')).toContain('[renders \\[a\\] \\*b\\*]('); + }); + it('still prints the annotation when GITHUB_OUTPUT/SUMMARY files are absent', () => { const stdout = vi.spyOn(process.stdout, 'write').mockReturnValue(true); emitRunOutputs(OUTPUT, silentLogger, undefined, { GITHUB_ACTIONS: 'true' }); @@ -96,6 +172,7 @@ describe('emitRunOutputs — GitLab CI', () => { expect(dotenv).toContain('PIWI_RUN_URL=https://dash.example.com/test-runs/42'); expect(dotenv).toContain('PIWI_RUN_ID=42'); expect(dotenv).toContain('PIWI_RUN_STATUS=passed'); + expect(dotenv).toContain('PIWI_FAILED_COUNT=0'); expect(dotenv).toContain('PIWI_PROJECT_ID=7'); expect(dotenv).toContain('PIWI_CI_BUILD_URL=https://ci.example.com/build/9'); } finally { diff --git a/packages/reporter/tests/failure-links.spec.ts b/packages/reporter/tests/failure-links.spec.ts new file mode 100644 index 00000000..1ffef3f9 --- /dev/null +++ b/packages/reporter/tests/failure-links.spec.ts @@ -0,0 +1,52 @@ +import { describe, it, expect, vi } from 'vitest'; +import { FailureLinks, caseLocateUrl, formatFailureLine } from '../src/internal/support/failure-links.js'; +import { Logger } from '../src/internal/support/logger.js'; + +const TEST = { title: 'applies the discount', file: 'tests/checkout.spec.ts', retry: 2, browser: 'chromium' }; + +describe('caseLocateUrl', () => { + it('builds the resolver URL from the run id and the test identity', () => { + expect(caseLocateUrl('https://dash.example.com/', 42, TEST)).toBe( + 'https://dash.example.com/test-runs/42/locate?file=tests%2Fcheckout.spec.ts&title=applies%20the%20discount&retry=2&browser=chromium', + ); + }); + + it('omits the browser when the test ran under no project', () => { + expect(caseLocateUrl('https://dash.example.com', 42, { ...TEST, browser: null })).toBe( + 'https://dash.example.com/test-runs/42/locate?file=tests%2Fcheckout.spec.ts&title=applies%20the%20discount&retry=2', + ); + }); + + it('encodes characters a title may carry', () => { + const url = caseLocateUrl('https://dash.example.com', 1, { ...TEST, title: 'a+b & c=d #1' }); + expect(url).toContain('title=a%2Bb%20%26%20c%3Dd%20%231'); + }); +}); + +describe('FailureLinks', () => { + it('prints one line per failure, each only once', () => { + const logger = new Logger(false); + const info = vi.spyOn(logger, 'info').mockImplementation(() => {}); + const links = new FailureLinks('https://dash.example.com', logger); + + links.add(TEST); + links.add({ ...TEST, title: 'second' }); + links.printPending(7); + links.printPending(7); + links.add({ ...TEST, title: 'third' }); + links.printPending(7); + + expect(info.mock.calls.map((c) => c[0])).toEqual([ + formatFailureLine({ ...TEST, url: caseLocateUrl('https://dash.example.com', 7, TEST) }), + `✗ second → ${caseLocateUrl('https://dash.example.com', 7, { ...TEST, title: 'second' })}`, + `✗ third → ${caseLocateUrl('https://dash.example.com', 7, { ...TEST, title: 'third' })}`, + ]); + expect(links.count).toBe(3); + }); + + it('resolves every failure against the run id it is given', () => { + const links = new FailureLinks('https://dash.example.com', new Logger(false)); + links.add(TEST); + expect(links.resolve(9)).toEqual([{ ...TEST, url: caseLocateUrl('https://dash.example.com', 9, TEST) }]); + }); +}); From 8460daa066c4da168137b0fa95deb3a0f911ae05 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 2 Sep 2026 04:24:14 +0000 Subject: [PATCH 3/5] feat(reporter): upload attachments that carry a body instead of a path testInfo.attach(name, { body }) attachments were skipped silently because only path-backed ones were collected. Stage a body as a temp file under os.tmpdir() for the upload (removed when the run ends) and apply one size ceiling to inline and path-backed attachments alike, warning once per attachment when it is skipped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145ws9TJpSi3vjZfzdBmB4c --- apps/docs/reporter.md | 2 +- .../src/internal/files/file-handler.ts | Bin 7009 -> 11297 bytes packages/reporter/src/public/reporter.ts | 49 +++++---- packages/reporter/tests/file-handler.spec.ts | 100 ++++++++++++++++++ 4 files changed, 128 insertions(+), 23 deletions(-) create mode 100644 packages/reporter/tests/file-handler.spec.ts diff --git a/apps/docs/reporter.md b/apps/docs/reporter.md index 3ea6db7f..c341a931 100644 --- a/apps/docs/reporter.md +++ b/apps/docs/reporter.md @@ -85,7 +85,7 @@ export { expect } from '@playwright/test' These are only collected when `collectPerformanceMetrics` is `true` (the default). If fixture data does not appear in the dashboard, the most likely cause is that your test files import `test` from `@playwright/test` directly instead of from your fixtures file (see options A/B above). -Any attachments Playwright records — including **videos** (`video: 'retain-on-failure'`) and screenshots — are uploaded automatically and shown as first-class evidence on the test-case and failure-cluster pages, alongside traces. Videos can be large, so pair `retain-on-failure` with periodic [storage cleanup](./storage#storage-management). +Any attachments Playwright records — including **videos** (`video: 'retain-on-failure'`) and screenshots — are uploaded automatically and shown as first-class evidence on the test-case and failure-cluster pages, alongside traces. That includes what a test attaches itself: both `testInfo.attach('payload', { path })` and the inline form `testInfo.attach('payload', { body: JSON.stringify(data), contentType: 'application/json' })` reach the dashboard (an inline body is staged as a temp file under the OS temp directory for the upload and removed when the run ends). One attachment above 500 MB — the dashboard's default upload ceiling — is skipped with a warning naming it rather than failing the upload. Videos can be large, so pair `retain-on-failure` with periodic [storage cleanup](./storage#storage-management). ## Configuration options diff --git a/packages/reporter/src/internal/files/file-handler.ts b/packages/reporter/src/internal/files/file-handler.ts index bb41c0117d230f142f586d98eb13cd7e42fc0cad..3f731a6db2156786b011dbe4b2da92611afaec48 100644 GIT binary patch literal 11297 zcmcgyeQ(>w8Q<Uj6qh2XsKlbuVjTu0cHlVak|B12*j-y>X>}G)5+jP_c%)_36|fJn zPq<IA-}BrHPqgf0LpvjgB;MU~&-*LWWm#0Ucp#(}Q~axnrRe2FqDE8Qd%Twh%f;4m z5iDN|RaHfGqO_JXB{mjJ91nF<68s^r>-b}Eh_5VVJr6IX-<@kG)w-;UaFcn`Zo3wf zVp&$W{drocNnKRy7Kfo*ON)H#{%i5Nn9WqxZf()cc%`%b==}WX_~prq)AOHCk4|2k z?Tc5Z=P%x!9=-m#yA;Bl$g*xKHI#KNC-bGs>uo;N>r#npaa?2>a97E>()F>_YG1sQ zt0RACUz}HRqFzcpf0a**@CNXifLO45!v_!cAkee)7bPYt&C+}(rbQ)+T#2U43Yo}@ zOo<kseL-v=T*%2nB?3Fs9EVgaPGmiqD^1Yqxe^KPz9?js^t4FSR5n>HmiR9%B~-|O z7FoJX>wTdMAw?!DV(>J@b(5mW5|OJ*RS~>Q=82H`dL`G7MOuqg3%y9o5(vyyrNk<Q zVi3kFS>;4Qn(JCgpf?p$X^0}6%&jp%4)@d*>++<)R&jFlLnxMKKc2rh69?j(M~`s( zSC78_qy2ciM+_6+5{~MsR=I|vFuw>qVBt|Mn7^Q?hHyyRmPMYeEen<Q3<DD8_GfRN z|M>HZAI@K#p1pc=dNva8)TF4AC%Udc4A{e;hd}6hPl%o@%PgJ98dncL>LTxrL=W1B zFZ<o)GMVx!eco97B`tZ8K6x=+!XSqqOEohf%USrm%*|u^47Tgb*~6=4#wYMSTw19M za|3_jGc~w2oajs5PM_`5d2ONam-hKmR*R%q88VkiaH7m)Y97~DHJ)#*c#iTwUj<CP z$S$EO@C(wY7i5;Cda;i+nXG96ELL&}(a@&csV59!83)ROWmQdKSmHZx8m-Z>baJ^= zBP+4T5C<7c_}l8pIZQBoTvX{S&1H7Fb$r9>MixLCH_&Tn^%5$dm8yb&h4F!mNdYS% zrLPogbYk6Fs`S37$qg{k_yH?o>m;>o4Vo6TSg9<dPwxPC0wfT;rKxcv1EUKDY_xM& zJk*=fQVDF$D(SYpWm0P<=OtieFnH2wGOBa6EDfe&Ri!m7i9uZy$$AhN5c|Y^u~2J> z^un1HJta2+9w-UFD6*nhG^NIaMl1FWgd+<RLaEYA>|*gVXu5=0tYKrhT8ZzKT%5@A zNyn*LLyY<?Sqp^DB9u63ieE+EfR_M-kM2gg<hW>n?16Yhi$r;=LR!eo=%f)Q2!TvU zz#0xZVNa<L7`(H?Vh#aAA4JvUw~^-od=LK!n1;wRRa+NxNE87Xc1Kl1K{3M59gH%u zZsek&0c1jB)F+$Wab_v6S_+@7;T1>V&GG^`AMEtJ1g9^qq5XzGuM2oP#31oDlWXY4 zY+j3(=O?eb8owq2YWM`QhH;uF>MAB@GLFTYlH8@t;D?DBRhlS{Qd-Fh>?A9-%$wpI z0e{EV<a2~*(&)Na;`dW0>Aw5oL{6G)nJCIa>lBd_2S};>Y2G5lQ}NzEG7&ZYSfqJW zR>cIVNjzC4;AeZR|G^#{iT6GDJ<EoHQIp<2BEWFhV<v1D<Q+izA6N+ln_Xl=lYtK+ zD3pdYK|U6vbSk2$j@4DFYkjuPCy_tY7hisfL^0O2tUD`l+U8$TzlA2mO?WL49x@1S z<=g86K&6m4<yIwtFc}y9ZTQptZ}&LE^a^%7SCa)@JWkSTjL1%;u&+a=A>m|gqWBo* z2`7UzXR^>@+~U;{8xHix6gR+2p!u$&N?3NPbEghX;33lls!tm99;fcm6uJ`fBZ|oZ zTk&}aBtHK`x9q;jY`^P<@Y<EG@GM{b27|}lJpT*Qj)99d6m#Lh!RMPcZDO*}FluC7 z7tM64su(E}i5o&7VUFPiInhJvY|U6|Xrhm1T?u$S3gqw>)l90@lV^N`wdq5v<~6DW zr|_2ANUkJuQAfgarzC;;_r<R!8d!!v1jvy^#05?(gnFLp*j5pg!A9e6k+i~3BN=hv zZ}R2-wWne;68Epes><H58FQo}rRb=>u?ueihMU-AQzg=D6UpH+Z4wb=48(4i-lF(u zHJz&t4x|w!zf_rwZWb$E>EC~_Of}9%D+-Gge><Ux2p~;$FyjE1mW_elOVandcN>T@ zri4u#92|%q=Sn?tSu!RD^aT)#Ws*c}=$1K=UG$vY;b4xcO0Hunk0XHKosQLHp%1L~ z!kc6wFY*E3yw6`b#YCVA6EsuKh770ro6;2n&8D-lI73kclVEUT_Fv;NU8REu_~C0q z9g8EeE2}cVqA~G}sz%G@h&Z&5qLa>uO@G{{`pczgoKzz#HL06u5Oh(p5sl+Q$Mv#= zV~XGmIZdJLkrOh`+%kB!3MP?d8jdwz!we9&<plz?g>W--7>BhVmwnQC23z|f+r;h8 z^&Vwen|o-lK)aQ4S5n_mQ78dw*wN1XA6wz3trPx#scWBeo-}Wi@i8EtO~ooUl=;@a zLyUwS+Dzg_{G`-r8;r{UtWP+wW`W#=9hS9+x3H9!#gRnKVJ*KeMq4|dL4R)9xsAJX zLlo4B-ErCIIozs2<SxTA`w5z}H}PC$3dtkY=Uz=)^(CLN%NAfH$7G$XwsaQRH5wVF zfq<L<fhs}6!3LX2rexlf+XU(ee(thJ6YJwhBknQh=t6`Gm$Mk+!w7?vsakTv6z(S* zL?Y6l7Py34t*ZuR2?8ee2W-=T+I&WwNz_F%i^ix$sL@i?fV_m^86E}B0*(W*B}C(s z!Nxem@CbQzTMV&W+WLz{+EpPmK(?D%PNP2GY`0T#X1(#-6p`<3r>oAPg9V&II#gXM zPo)<q2=iEbB&H?9bGm{z9vYp$>AKoL#jGKB^~M!;q1m9w15~f&H;~#&tFFo0;g5`X z1Ow%01`>O%2)SWGdnk&HOsQjS$;C2LxG}xdz%tDV8Iwz}KZ$Kyji&<L$9iDeCjN%) zE(S-<3#0&q)b>^A)L0YksKT}hMe_Gj1u{Wre_l78z-bWzH;Eu}UGoJLm{U)|XWlY$ zTR76&hX~EzA&CE(a{wzvQVu#1XDXkeRrwp}NC56rs$%?jC`@wY;qN<zYxLUy2?Y!e zjqhJ;IaQSKof;YM(!c)Z{&lCt`98S)gLwK>^m?1Ixd`5xa~pCBxWc;;&MM=Xsv{Y2 z^&X``v^B7<AXTUO%d7(w>m%2~9%vhm;E$=`21O4a`enM4yEU7^{p*nSAhE_3I;L!h z*}zWQ-MP=9#fwLDi9v2ihVQofjko!43t+2KXdO6;Y>ZP{yBA;FzcvnG^99$0CU||i zRM=50>@1_N@Af*@-#F^f%xK-oJ_ZME!&BUJ2c+H|;kZ)m_HgRu24O<pkjrwBP|#<o zwBipEQ}huyL-x~(LCZirKx4rurFFvL1vflc$D#i>p3As=*A?4N8;s$h-D&L2g+kPe zW(2VLgBP>4Ao4bhDRsIimL%VE+mG`-0P@WeL)*@+RQ1r-U`wcWHtLM2;3zq3_vOWw z+MzOm7J8iPw9eG#3sOPkElXLSq|Z2N_BUv{Oq5DACu-p`qqifPZd{X|XGrnb#4twK z=YDA6GIl5islPo~<d`EM%yXZdrZlX<xU-;Zp)_t#RWx<k)V}e9R=O!E;C`N)S(fi8 zaGH0^z~-9yWR=JK5P~|AnAq&-)>*D%Tm;!xs4{x)tMBvKEl5q#>c9r`rMrBQ6&F~f zkL{(_(5UbHR#qt9-%^FRqwFN-MTAGDhRjvHDyqd8K9Vg4Go8U@TB9N6ACamSnexA7 zmCAw6WvS;yO=TlZSCBHG!3^vZI6jp@;!qNkozt|;w-YcX)aot9Eu<*uakt*!&N2?e z46lmlGjAo5??3D^E$Yz0wz$l7KBg}Y4nsgOZHf?i_<7qipl}D3W2qQJ1)IqA(Dv?} zH?}OaMnz_dCr@ow;nX9n1K{v&E#=O>Q~zl2h#G=_J$rMCJ_WcG5ge0}Ap0=cN<XfP zGoBlwyK1Z$PTUp-?k;Mti*%M3C}+52rZmy@GXkQFoNuKIs_-|~gn%-0<THu$06JtD zAipPNR%J24xRpA)C7`W6^qKUv2fBi5SAI__$q;X)j~36Ykp@K4V@ICX!f-DDM)dm* z#J4P?M0#?DK>*iMJMF{o2+;7)94=0GZ?O}<zoSb2`5oxmYyTHJ84krcx&%kBP!g$9 zH2i7{{8l=JwTcMuyyP%OntYiVB6>xezWDb)|3#f|=X}}X0IBt0<RK8mIYk_p{7ofD zQ)xzMUVXqgf}E$%Utr;Irx?mE9JJLuMb8<1WV~uYlTn8wB20=|HVrzcyCLwkK{#1c ze8hlBCEQ7)tc}LS>32-hZLZC(2ZFOtK2vNh{Lro0r@y(#$jN~{6a$a(GEf7ZJbQUG z`1+ea2^wN^=ah?iY7hAaBUu3lC^Uk&#mZv2F^X8tF}Wt0a4Z8nv{&qY->7A0+m0xJ zz!{G>&fQx(pXgM&S%u%vZ~2&6gBw{G!7-v#Ad%<cT#8~7pQGxgjipv}RDNMZU(*q% ztge~Tc5$|bOm)fKc0!#mYD+N+aofF?x<vNr;1foriA^Tx(EfXFwNsu4;fQ5~nJT~Q z6;EIcHNWZ<_4Hgqh&@Y-8PL!?!SRdQafgv>C6XO|#A{R>)tz4fz6Rnu*cc)xUJHpF zO85GV1x3lY$fF)+a}u-HXfki|h3f_|G(s_MN*dmo)qdxw%JC)(?&k(rca7eU<5kAa za>x?Z$tJygN1m;&n?pE^SfS%2ok4k{-dtVv?3i9~n`yMw)^c?Axv^#7Y^RJzdcnG_ zRYiWu_!h>2FG9MAk(hgj0SFL@)KsGyfouXLFDVP5hQ0M1-9_r9V(>=uJ(Gl3yPzTK ziFwZIs?942^SUmNvlRYxgnlSS(lvPu9?>{`r7`K6UR^^+EMFrdZ0fn6+FNBtCg_v` zdk)3MP+L|7@tnFD9vaJ>X?zK^QB#cYu3P!Uyuv_DS;J`K*v011K%z?>2zw_j1oxh` z?_W5Z<<iah09?Xy;OW$()*Q@y%)Er+T2_{!m_NjE9O4>Z$_yD})c+7KWf?_ilpwX8 zQ!O}$Mp@e0;imB#(905jpF*`UqL%BiXL)ZZ%XG+>=^@X=p|cQ4%ffvp=#3GARbhv; zllp-$1G7}~Cz&VT9@`Clj2ybpFk3J*8|!F$7M45is5PEW;%kk_jv6Ba$zn&YXtRd^ zvHfZemx@3mX(o0uJC~3YMp$?m5lt{eafAx$cJNM~3{0DRLhqgM26DIDC8MIjX$I86 z>J9lHhZZ)(9EB$;8ovMO@2hz5;o)$GW(8Uj?aEJshrbRU-sgoL&zi%w?_;9DW6;*y v7vUQ?JMTvm>;&#VuJED~svbTdh9U}HgoGET7bE+4{MDn4Kpl_f!)EV4@ljR` delta 404 zcmZ1&@z897+D6x@jFYQa@AD+*=M|R}l_r<u7imoX$ga0Jo8t!CWDkCa$@BQNWVQ6U z6co}j^HLmhavV!a5|cA>Q}arSHGrZD3X=~CNZ0ECnVMD#jzvX@l{VE1d5O8HRti89 zGxO4|6$%ndG7vnVRV6^(A(aJ4O7e>`(=+oDbNrArD%9F3RD-PmyH24fwYW5=1gj;s z6xlLaLGYYcW}1RVq8`Yp3TkQ!X~lY}6`92)#le+%$xv}kO{jA~zE*%bN3Wo?I70*C zOo+I`WPT|*cCfpFfwMzYWAZIgx5-js`oa+TT1{)Hq=G{2WIleW$qR*qH*b)!<pTiT C)q$q~ diff --git a/packages/reporter/src/public/reporter.ts b/packages/reporter/src/public/reporter.ts index 705a941a..1654618d 100644 --- a/packages/reporter/src/public/reporter.ts +++ b/packages/reporter/src/public/reporter.ts @@ -491,27 +491,32 @@ export class PiwiDashboardReporter { }); this.materializeUnrunTests(unrunReason); - await this.submitter.submit( - { - options: this.options, - testCases: this.testCases, - startTime: this.startTime, - playwrightVersion: this.playwrightVersion, - reporterVersion: this.reporterVersion, - totalTests: this.totalTests, - passedTests: this.passedTests, - failedTests: this.failedTests, - skippedTests: this.skippedTests, - timedOutTests: this.timedOutTests, - didNotRunTests: this.didNotRunTests, - metadata: this.metadata, - instanceId: this.instanceId, - shardInfo: this.shardInfo, - setupSteps: this.setupSteps, - isFullRun: this.isFullRun, - filterDetails: this.filterDetails, - }, - result, - ); + try { + await this.submitter.submit( + { + options: this.options, + testCases: this.testCases, + startTime: this.startTime, + playwrightVersion: this.playwrightVersion, + reporterVersion: this.reporterVersion, + totalTests: this.totalTests, + passedTests: this.passedTests, + failedTests: this.failedTests, + skippedTests: this.skippedTests, + timedOutTests: this.timedOutTests, + didNotRunTests: this.didNotRunTests, + metadata: this.metadata, + instanceId: this.instanceId, + shardInfo: this.shardInfo, + setupSteps: this.setupSteps, + isFullRun: this.isFullRun, + filterDetails: this.filterDetails, + }, + result, + ); + } finally { + // Body-only attachments were staged as temp files for the uploads above. + this.fileHandler.cleanupBodyAttachments(); + } } } diff --git a/packages/reporter/tests/file-handler.spec.ts b/packages/reporter/tests/file-handler.spec.ts new file mode 100644 index 00000000..8652ed61 --- /dev/null +++ b/packages/reporter/tests/file-handler.spec.ts @@ -0,0 +1,100 @@ +import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { FileHandler, MAX_ATTACHMENT_BYTES } from '../src/internal/files/file-handler.js'; +import { Logger } from '../src/internal/support/logger.js'; +import type { CollectedTestCase, RawAttachment } from '../src/types.js'; + +function testCase(attachments: RawAttachment[]): CollectedTestCase { + return { title: 'attaches things', location: 'tests/attach.spec.ts:4:3', attachments }; +} + +let tmpDir: string; +let logger: Logger; +let warn: ReturnType<typeof vi.spyOn>; + +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'piwi-file-handler-')); + logger = new Logger(false); + warn = vi.spyOn(logger, 'warn').mockImplementation(() => {}); +}); + +afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + vi.restoreAllMocks(); +}); + +describe('FileHandler.findAllAttachments', () => { + it('stages a body-only attachment as a temp file under os.tmpdir()', () => { + const handler = new FileHandler(logger); + const body = Buffer.from(JSON.stringify({ order: 42 })); + const tc = testCase([{ name: 'order payload', contentType: 'application/json', body }]); + + const [found] = handler.findAllAttachments(tc); + expect(found).toBeDefined(); + expect(found!.name).toBe('order payload'); + expect(found!.contentType).toBe('application/json'); + expect(found!.originalName).toBe('order-payload.json'); + expect(path.dirname(found!.path).startsWith(os.tmpdir())).toBe(true); + expect(fs.readFileSync(found!.path)).toEqual(body); + + handler.cleanupBodyAttachments(); + expect(fs.existsSync(found!.path)).toBe(false); + }); + + it('reuses one temp file across repeated lookups of the same attachment', () => { + const handler = new FileHandler(logger); + const tc = testCase([{ name: 'note', contentType: 'text/plain', body: Buffer.from('hello') }]); + + const first = handler.findAllAttachments(tc)[0]!.path; + const second = handler.findAllAttachments(tc)[0]!.path; + expect(second).toBe(first); + expect(fs.readdirSync(path.dirname(first))).toHaveLength(1); + handler.cleanupBodyAttachments(); + }); + + it('keeps path-backed attachments as they are and skips internal ones', () => { + const handler = new FileHandler(logger); + const file = path.join(tmpDir, 'shot.png'); + fs.writeFileSync(file, 'png'); + const tc = testCase([ + { name: 'screenshot', contentType: 'image/png', path: file }, + { name: 'piwi-network', contentType: 'application/json', body: Buffer.from('[]') }, + { name: 'trace', contentType: 'application/zip', path: file }, + ]); + + const found = handler.findAllAttachments(tc); + expect(found).toEqual([{ name: 'screenshot', path: file, contentType: 'image/png', originalName: 'shot.png' }]); + }); + + it('skips a body above the size limit and warns once per attachment', () => { + const handler = new FileHandler(logger, 8); + const tc = testCase([ + { name: 'huge', contentType: 'application/octet-stream', body: Buffer.alloc(9) }, + { name: 'small', contentType: 'text/plain', body: Buffer.from('ok') }, + ]); + + expect(handler.findAllAttachments(tc).map((a) => a.name)).toEqual(['small']); + handler.findAllAttachments(tc); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0]![0]).toContain('"huge"'); + expect(warn.mock.calls[0]![0]).toContain('attaches things'); + handler.cleanupBodyAttachments(); + }); + + it('applies the same limit to path-backed attachments', () => { + const handler = new FileHandler(logger, 8); + const file = path.join(tmpDir, 'video.webm'); + fs.writeFileSync(file, Buffer.alloc(16)); + const tc = testCase([{ name: 'video', contentType: 'video/webm', path: file }]); + + expect(handler.findAllAttachments(tc)).toEqual([]); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0]![0]).toContain('"video"'); + }); + + it('defaults to the dashboard upload ceiling', () => { + expect(MAX_ATTACHMENT_BYTES).toBe(500 * 1024 * 1024); + }); +}); From 392a0d5f2c3c637145f2988e8b574e393d8b12e5 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 2 Sep 2026 04:24:15 +0000 Subject: [PATCH 4/5] fix(notifications): link alerts to the execution and quote the error head Slack and email run notifications linked each failure to the test's history page even though the payload carries the execution id; link to /test-run-cases/:id and fall back to the history page only without one. The excerpt embedded in notifications, cluster alerts and pull-request comments was the raw first characters of the error, so a timeout read as a bare "Timeout 30000ms exceeded" with the call log cut off. Build it from the message head shared with the fingerprint, and append the last call-log state line when the head is only a timeout. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145ws9TJpSi3vjZfzdBmB4c --- apps/application/server/utils/email.ts | 12 ++- .../server/utils/notifications/dispatch.ts | 5 +- .../utils/notifications/run-notifications.ts | 4 +- .../server/utils/scm/pr-feedback.ts | 13 +-- apps/application/shared/error-fingerprint.ts | 3 +- .../application/shared/notification-events.ts | 60 +++++++++++-- apps/application/shared/pr-feedback.ts | 2 + .../tests/unit/notification-events.test.ts | 84 +++++++++++++++++++ .../tests/unit/pr-feedback.test.ts | 24 ++++++ apps/docs/notifications.md | 12 ++- 10 files changed, 193 insertions(+), 26 deletions(-) diff --git a/apps/application/server/utils/email.ts b/apps/application/server/utils/email.ts index 8c6f28ce..57d1d3b2 100644 --- a/apps/application/server/utils/email.ts +++ b/apps/application/server/utils/email.ts @@ -1,6 +1,6 @@ import nodemailer from 'nodemailer'; import type { Transporter } from 'nodemailer'; -import { renderEventSubject, notificationTargetPath } from '#shared/notification-events'; +import { renderEventSubject, notificationTargetPath, failureTargetPath } from '#shared/notification-events'; import type { NotificationEvent, NotificationPayload, @@ -173,6 +173,12 @@ export function renderTestEmail(to: string): { html: string; text: string } { return { html, text }; } +/** Where a failing test links to: its execution, else its history, else the run. */ +function failureUrl(failure: TopFailure, runUrl: string): string { + const path = failureTargetPath(failure); + return path ? `${siteUrl()}${path}` : runUrl; +} + export function renderRunNotificationEmail(opts: { projectName: string; runId: number; @@ -191,7 +197,7 @@ export function renderRunNotificationEmail(opts: { if (failures.length > 0) { const rows = failures .map((f) => { - const caseUrl = f.testCaseId ? `${siteUrl()}/test-cases/${f.testCaseId}` : url; + const caseUrl = failureUrl(f, url); const title = escapeHtml(f.title); const titleHtml = `<a href="${caseUrl}" style="color:#18181b;font-weight:600;text-decoration:none;">${title}</a>`; const loc = f.filePath ? `<div style="color:#a1a1aa;font-size:12px;">${escapeHtml(f.filePath)}</div>` : ''; @@ -204,7 +210,7 @@ export function renderRunNotificationEmail(opts: { failuresHtml = `<ul style="margin:0 0 24px;padding:0;">${rows}</ul>`; failuresText = failures .map((f) => { - const caseUrl = f.testCaseId ? `${siteUrl()}/test-cases/${f.testCaseId}` : url; + const caseUrl = failureUrl(f, url); const loc = f.filePath ? ` (${f.filePath})` : ''; const excerpt = f.errorExcerpt ? `\n ${f.errorExcerpt.replace(/\n/g, '\n ')}` : ''; return `- ${f.title}${loc}\n ${caseUrl}${excerpt}`; diff --git a/apps/application/server/utils/notifications/dispatch.ts b/apps/application/server/utils/notifications/dispatch.ts index 69c18242..fe352c99 100644 --- a/apps/application/server/utils/notifications/dispatch.ts +++ b/apps/application/server/utils/notifications/dispatch.ts @@ -16,7 +16,7 @@ import type { RunFinishedPayload, ClusterNewPayload, } from '#shared/notification-events'; -import { renderEventSubject, notificationTargetPath } from '#shared/notification-events'; +import { renderEventSubject, notificationTargetPath, failureTargetPath } from '#shared/notification-events'; import type { LibSQLDatabase } from 'drizzle-orm/libsql'; const MAX_ATTEMPTS = 5; @@ -120,7 +120,8 @@ async function sendToSlack(config: Record<string, unknown>, event: NotificationE if (event.startsWith('run.')) { const p = payload as RunFinishedPayload; for (const f of p.topFailures ?? []) { - const link = f.testCaseId ? `<${base}/test-cases/${f.testCaseId}|${f.title}>` : f.title; + const path = failureTargetPath(f); + const link = path ? `<${base}${path}|${f.title}>` : f.title; const excerpt = f.errorExcerpt ? `\n\`\`\`${slackExcerpt(f.errorExcerpt)}\`\`\`` : ''; blocks.push({ type: 'section', text: { type: 'mrkdwn', text: `• *${link}*${excerpt}` } }); } diff --git a/apps/application/server/utils/notifications/run-notifications.ts b/apps/application/server/utils/notifications/run-notifications.ts index 106e0248..1f4d7dfe 100644 --- a/apps/application/server/utils/notifications/run-notifications.ts +++ b/apps/application/server/utils/notifications/run-notifications.ts @@ -8,7 +8,7 @@ import { projects, testRuns, failureClusters, testRunsCases, testCases } from '. import { emitNotification } from './emit'; import { buildTopFailures, - truncateExcerpt, + errorExcerpt, computePerfBaseline, TOP_FAILURES_LIMIT, PERF_BASELINE_RUNS, @@ -153,7 +153,7 @@ export async function emitRunNotifications(db: DbClient, runId: number): Promise projectName: project.label || project.name, signature: cluster.signature, runId, - sampleErrorExcerpt: truncateExcerpt(cluster.sampleError), + sampleErrorExcerpt: errorExcerpt(cluster.sampleError), affectedCases: affected.length, }); } diff --git a/apps/application/server/utils/scm/pr-feedback.ts b/apps/application/server/utils/scm/pr-feedback.ts index 46969019..196d29ea 100644 --- a/apps/application/server/utils/scm/pr-feedback.ts +++ b/apps/application/server/utils/scm/pr-feedback.ts @@ -29,6 +29,7 @@ import { buildPrComment, DEFAULT_PR_FEEDBACK, PR_COMMENT_MARKER, + PR_EXCERPT_MAX, PR_FEEDBACK_KEY, resolvePrFeedbackSettings, type PrFailureEntry, @@ -39,6 +40,7 @@ import type { VerifiedFix } from '../fix-verification'; import type { RunMetadata } from '../run-json-types'; import type { DbClient } from '../../database'; import type { FilterDetails } from '#shared/types'; +import { errorExcerpt } from '#shared/notification-events'; /** Read the resolved settings, falling back to the (disabled) defaults. */ export async function getPrFeedbackSettings(db: DbClient): Promise<PrFeedbackSettings> { @@ -51,15 +53,6 @@ const FAIL_STATUSES = ['failed', 'timedOut', 'timedout']; /** Recent default-branch runs scanned to decide whether a failure is already flaky there. */ const DEFAULT_BRANCH_FLAKY_RUNS = 50; -/** First line of an error, capped so a comment stays readable. */ -function excerpt(error: string | null): string | null { - if (!error) return null; - const firstLine = error.split('\n').find((line) => line.trim().length > 0); - if (!firstLine) return null; - const trimmed = firstLine.trim(); - return trimmed.length > 200 ? `${trimmed.slice(0, 199)}…` : trimmed; -} - /** What the dashboard is reachable at, for the links inside the comment. */ function resolveSiteUrl(): string | null { const configured = process.env.PIWI_SITE_URL?.trim(); @@ -123,7 +116,7 @@ async function buildFailureEntries( return { title: row.title, filePath: row.filePath, - errorExcerpt: excerpt(row.error), + errorExcerpt: errorExcerpt(row.error, PR_EXCERPT_MAX) ?? null, executionId: row.id, clusterId: row.failureClusterId, clusterSignature: row.failureClusterId ? (clusterSignatures.get(row.failureClusterId) ?? null) : null, diff --git a/apps/application/shared/error-fingerprint.ts b/apps/application/shared/error-fingerprint.ts index 0bf59e71..77d7f8ba 100644 --- a/apps/application/shared/error-fingerprint.ts +++ b/apps/application/shared/error-fingerprint.ts @@ -90,8 +90,9 @@ function classifyError(text: string): ErrorType { * Cut the error down to its message head: everything before the Playwright * call log and the JS stack trace, capped at 5 non-empty lines so long * element dumps (strict-mode violations) don't destabilize the fingerprint. + * Notifications and pull-request comments quote the same head. */ -function extractMessageHead(text: string): string { +export function extractMessageHead(text: string): string { let head = text; const callLogIdx = head.indexOf('\nCall log:'); if (callLogIdx !== -1) head = head.slice(0, callLogIdx); diff --git a/apps/application/shared/notification-events.ts b/apps/application/shared/notification-events.ts index 5e03bbfb..d8452069 100644 --- a/apps/application/shared/notification-events.ts +++ b/apps/application/shared/notification-events.ts @@ -1,3 +1,5 @@ +import { extractMessageHead, stripAnsi } from '#shared/error-fingerprint'; + /** All notification event keys supported by the subscription system. */ export const NOTIFICATION_EVENTS = [ 'run.finished', @@ -70,18 +72,63 @@ export interface ClusterNewPayload { } /** - * Trim, strip ANSI colour codes, and cap an error message so it can be embedded - * in a notification payload (and rendered in email/Slack) without bloating it. + * Trim, strip ANSI colour codes, and cap a text so it can be embedded in a + * notification payload (and rendered in email/Slack) without bloating it. * Returns undefined for empty input. */ export function truncateExcerpt(text?: string | null, max: number = ERROR_EXCERPT_MAX): string | undefined { if (!text) return undefined; - const esc = String.fromCharCode(27); - const clean = text.replace(new RegExp(esc + '\\[[0-9;]*m', 'g'), '').trim(); + const clean = stripAnsi(text).trim(); if (!clean) return undefined; return clean.length > max ? clean.slice(0, max).trimEnd() + '…' : clean; } +/** A message head that says nothing but that a timeout elapsed. */ +const BARE_TIMEOUT_RE = /^(?:\w*Error:\s*)?(?:[\w.]+:\s*)?(?:Test )?[Tt]imeout(?: of)? \d+m?s exceeded\.?$/; +/** Call-log lines that say where Playwright was when the timeout hit. */ +const CALL_LOG_STATE_RE = /^\s*-\s*((?:waiting for|locator resolved to)\b.*)$/; + +/** + * The part of an error worth quoting outward — in a notification, a + * pull-request comment, a digest: the message head (the lines before the + * Playwright call log and the stack trace, at most five), with the last + * `waiting for …` / `locator resolved to …` call-log line appended when the + * head is only a bare timeout. ANSI codes are stripped and the result is + * capped at `max` characters. Returns undefined for empty input. + */ +export function errorExcerpt(text?: string | null, max: number = ERROR_EXCERPT_MAX): string | undefined { + if (!text) return undefined; + const clean = stripAnsi(text).trim(); + if (!clean) return undefined; + let head = extractMessageHead(clean) || clean.split('\n')[0]!.trim(); + if (BARE_TIMEOUT_RE.test(head)) { + const state = lastCallLogState(clean); + if (state) head = `${head}\n${state}`; + } + return truncateExcerpt(head, max); +} + +function lastCallLogState(text: string): string | null { + const start = text.indexOf('Call log:'); + if (start === -1) return null; + let last: string | null = null; + for (const line of text.slice(start).split('\n')) { + const match = CALL_LOG_STATE_RE.exec(line); + if (match) last = match[1]!.trim(); + } + return last; +} + +/** + * Dashboard path for one failing test: the execution with its evidence when + * the payload carries one, otherwise the test's history page. + */ +export function failureTargetPath(failure: Pick<TopFailure, 'testCaseId' | 'executionId'>): string | null { + if (failure.executionId != null) return `/test-run-cases/${failure.executionId}`; + if (failure.testCaseId != null) return `/test-cases/${failure.testCaseId}`; + return null; +} + /** Raw failing-case row shape consumed by {@link buildTopFailures}. */ export interface TopFailureInput { title: string; @@ -93,7 +140,8 @@ export interface TopFailureInput { /** * Map raw failing-case rows to the compact {@link TopFailure} shape embedded in - * run notifications: capped to `limit` entries with truncated error excerpts. + * run notifications: capped to `limit` entries, each error cut to its + * {@link errorExcerpt}. */ export function buildTopFailures(rows: TopFailureInput[], limit: number = TOP_FAILURES_LIMIT): TopFailure[] { return rows.slice(0, limit).map((r) => { @@ -101,7 +149,7 @@ export function buildTopFailures(rows: TopFailureInput[], limit: number = TOP_FA if (r.filePath) failure.filePath = r.filePath; if (r.testCaseId != null) failure.testCaseId = r.testCaseId; if (r.executionId != null) failure.executionId = r.executionId; - const excerpt = truncateExcerpt(r.error); + const excerpt = errorExcerpt(r.error); if (excerpt) failure.errorExcerpt = excerpt; return failure; }); diff --git a/apps/application/shared/pr-feedback.ts b/apps/application/shared/pr-feedback.ts index 9fa7368b..9a1a2a34 100644 --- a/apps/application/shared/pr-feedback.ts +++ b/apps/application/shared/pr-feedback.ts @@ -119,6 +119,8 @@ export interface PrSummaryInput { // ── Rendering ──────────────────────────────────────────────────────────────── const MAX_LISTED = 5; +/** Max characters of an error excerpt quoted in the pull-request comment. */ +export const PR_EXCERPT_MAX = 200; /** Escape the characters that would break out of a markdown table cell. */ function escapeCell(text: string): string { diff --git a/apps/application/tests/unit/notification-events.test.ts b/apps/application/tests/unit/notification-events.test.ts index 123d5b76..aa6f7cb3 100644 --- a/apps/application/tests/unit/notification-events.test.ts +++ b/apps/application/tests/unit/notification-events.test.ts @@ -3,6 +3,8 @@ import { renderEventSubject, buildTopFailures, truncateExcerpt, + errorExcerpt, + failureTargetPath, TOP_FAILURES_LIMIT, ERROR_EXCERPT_MAX, type RunFinishedPayload, @@ -91,6 +93,81 @@ describe('truncateExcerpt', () => { }); }); +const TIMEOUT_ERROR = `TimeoutError: locator.click: Timeout 30000ms exceeded. +Call log: + - waiting for getByRole('button', { name: 'Submit' }) + - locator resolved to <button disabled>Submit</button> + - attempting click action + - waiting for element to be visible, enabled and stable + - element is not enabled + - retrying click action + - waiting for getByRole('button', { name: 'Submit' }) + - locator resolved to <button disabled>Submit</button> + + at tests/checkout.spec.ts:12:40`; + +const ASSERTION_ERROR = `Error: expect(locator).toBeVisible() failed + +Locator: getByText('Order confirmed') +Expected: visible +Received: <element(s) not found> +Timeout: 5000ms + +Call log: + - Expect "toBeVisible" with timeout 5000ms + - waiting for getByText('Order confirmed') + + at tests/checkout.spec.ts:20:45 + at node_modules/@playwright/test/lib/worker.js:1:1`; + +describe('errorExcerpt', () => { + test('returns undefined for empty input', () => { + expect(errorExcerpt(undefined)).toBeUndefined(); + expect(errorExcerpt(' \n ')).toBeUndefined(); + }); + + test('quotes the message head without the call log or the stack', () => { + expect(errorExcerpt(ASSERTION_ERROR)).toBe( + [ + 'Error: expect(locator).toBeVisible() failed', + "Locator: getByText('Order confirmed')", + 'Expected: visible', + 'Received: <element(s) not found>', + 'Timeout: 5000ms', + ].join('\n'), + ); + }); + + test('appends the last call-log state line to a bare timeout', () => { + expect(errorExcerpt(TIMEOUT_ERROR)).toBe( + 'TimeoutError: locator.click: Timeout 30000ms exceeded.\nlocator resolved to <button disabled>Submit</button>', + ); + expect(errorExcerpt("Test timeout of 30000ms exceeded.\nCall log:\n - waiting for getByTestId('cart')\n")).toBe( + "Test timeout of 30000ms exceeded.\nwaiting for getByTestId('cart')", + ); + }); + + test('leaves a bare timeout alone when there is no call log', () => { + expect(errorExcerpt('Test timeout of 30000ms exceeded.')).toBe('Test timeout of 30000ms exceeded.'); + }); + + test('strips ANSI codes and caps the result', () => { + const esc = String.fromCharCode(27); + expect(errorExcerpt(`${esc}[31mError: boom${esc}[0m`)).toBe('Error: boom'); + const out = errorExcerpt(`Error: ${'x'.repeat(400)}`, 50)!; + expect(out.length).toBe(51); + expect(out.endsWith('…')).toBe(true); + }); +}); + +describe('failureTargetPath', () => { + test('prefers the execution over the test history page', () => { + expect(failureTargetPath({ testCaseId: 5, executionId: 90 })).toBe('/test-run-cases/90'); + expect(failureTargetPath({ testCaseId: 5 })).toBe('/test-cases/5'); + expect(failureTargetPath({})).toBeNull(); + }); +}); + describe('buildTopFailures', () => { test('caps to the limit and maps fields, dropping empty ones', () => { const rows = Array.from({ length: 5 }, (_, i) => ({ @@ -113,6 +190,13 @@ describe('buildTopFailures', () => { expect(out[1]).toEqual({ title: 'test 1', testCaseId: 1, executionId: 101 }); }); + test('quotes the error head, not its call log', () => { + const [failure] = buildTopFailures([{ title: 'a', error: TIMEOUT_ERROR }]); + expect(failure!.errorExcerpt).toBe( + 'TimeoutError: locator.click: Timeout 30000ms exceeded.\nlocator resolved to <button disabled>Submit</button>', + ); + }); + test('respects a custom limit', () => { const rows = [{ title: 'a' }, { title: 'b' }]; expect(buildTopFailures(rows, 1)).toHaveLength(1); diff --git a/apps/application/tests/unit/pr-feedback.test.ts b/apps/application/tests/unit/pr-feedback.test.ts index 4d47c4b6..e94bfea8 100644 --- a/apps/application/tests/unit/pr-feedback.test.ts +++ b/apps/application/tests/unit/pr-feedback.test.ts @@ -4,10 +4,12 @@ import { buildPrComment, DEFAULT_PR_FEEDBACK, PR_COMMENT_MARKER, + PR_EXCERPT_MAX, resolvePrFeedbackSettings, type PrFailureEntry, type PrSummaryInput, } from '#shared/pr-feedback'; +import { errorExcerpt } from '#shared/notification-events'; function entry(overrides: Partial<PrFailureEntry> = {}): PrFailureEntry { return { @@ -139,6 +141,28 @@ describe('buildPrComment', () => { expect(body).toContain('a \\| b'); }); + test('quotes a timeout with where Playwright was waiting, the way notifications do', () => { + const raw = `TimeoutError: locator.click: Timeout 30000ms exceeded. +Call log: + - waiting for getByRole('button', { name: 'Pay' }) + - locator resolved to <button disabled>Pay</button> + + at tests/checkout.spec.ts:12:40`; + const excerpt = errorExcerpt(raw, PR_EXCERPT_MAX)!; + const body = buildPrComment(summary({ failedTests: 1, newRegressions: [entry({ errorExcerpt: excerpt })] })); + expect(body).toContain( + 'TimeoutError: locator.click: Timeout 30000ms exceeded. locator resolved to <button disabled>Pay</button>', + ); + expect(body).not.toContain('Call log'); + expect(body).not.toContain('checkout.spec.ts:12:40'); + }); + + test('keeps a pull-request excerpt within its own cap', () => { + const excerpt = errorExcerpt(`Error: ${'x'.repeat(500)}`, PR_EXCERPT_MAX)!; + expect(excerpt.length).toBe(PR_EXCERPT_MAX + 1); + expect(excerpt.endsWith('…')).toBe(true); + }); + test('flattens a multi-line error excerpt onto one line', () => { const body = buildPrComment( summary({ failedTests: 1, newRegressions: [entry({ errorExcerpt: 'line one\nline two' })] }), diff --git a/apps/docs/notifications.md b/apps/docs/notifications.md index 4cf9bb38..83a98846 100644 --- a/apps/docs/notifications.md +++ b/apps/docs/notifications.md @@ -68,7 +68,7 @@ The body is `{ "event": "run.failed", "payload": { … }, "timestamp": "…" }`. { "title": "applies discount code", "filePath": "tests/checkout.spec.ts", - "errorExcerpt": "TimeoutError: locator.click: Timeout 30000ms exceeded", + "errorExcerpt": "TimeoutError: locator.click: Timeout 30000ms exceeded.\nlocator resolved to <button disabled>Pay</button>", "testCaseId": 815, "executionId": 9001 } @@ -78,7 +78,15 @@ The body is `{ "event": "run.failed", "payload": { … }, "timestamp": "…" }`. } ``` -`cluster.new` payloads similarly carry `sampleErrorExcerpt` and `affectedCases`. These fields are **additive** — existing consumers keep working, but if you re-serialize the payload to re-check the HMAC, sign the exact bytes you received. +`errorExcerpt` is the error's message head — the lines before Playwright's call log and the stack trace, +at most five, capped at 300 characters. When that head is only a bare timeout line, the last +`waiting for …` / `locator resolved to …` line of the call log is appended so the excerpt says what +Playwright was waiting on. Slack and email messages quote the same excerpt, and link each failure to its +execution (`/test-run-cases/<executionId>`), falling back to the test's history page when a payload +carries no execution id. The [pull-request comment](./ci#pull-request-feedback) quotes failures the same +way. + +`cluster.new` payloads similarly carry `sampleErrorExcerpt` (cut the same way) and `affectedCases`. These fields are **additive** — existing consumers keep working, but if you re-serialize the payload to re-check the HMAC, sign the exact bytes you received. ### Global channels & subscriptions From 81d677381260791c1d99a49e2aabf58e174f58eb Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Wed, 2 Sep 2026 04:25:43 +0000 Subject: [PATCH 5/5] docs(reporter): list the failure-links module in the architecture map Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0145ws9TJpSi3vjZfzdBmB4c --- packages/reporter/ARCHITECTURE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/reporter/ARCHITECTURE.md b/packages/reporter/ARCHITECTURE.md index 0974aec9..c478797c 100644 --- a/packages/reporter/ARCHITECTURE.md +++ b/packages/reporter/ARCHITECTURE.md @@ -81,7 +81,7 @@ src/ files/ file-handler, compression capture/ capture-fixtures, locator-healing, attachments ← runs in the worker config/ env (PIWI_* ↔ options) - support/ logger, limiter, ci, ci-output, run-url, instance-id, + support/ logger, limiter, ci, ci-output, failure-links, run-url, instance-id, cli-filters, setup-file, source-snippet, worker-index, errors, selection-client, selection-env types/