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/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 = `${title}`; const loc = f.filePath ? `
${escapeHtml(f.filePath)}
` : ''; @@ -204,7 +210,7 @@ export function renderRunNotificationEmail(opts: { failuresHtml = ``; 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, 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 { @@ -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): 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/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'); + }); +}); 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 + - 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 + + at tests/checkout.spec.ts:12:40`; + +const ASSERTION_ERROR = `Error: expect(locator).toBeVisible() failed + +Locator: getByText('Order confirmed') +Expected: visible +Received: +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: ', + '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 ', + ); + 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 ', + ); + }); + 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 { 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 + + 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 ', + ); + 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/ci.md b/apps/docs/ci.md index 7222dfff..57f40ab5 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: ` line in the log. +**Always** — one line per failed test, printed the moment its final attempt fails, then a +`View run: ` 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/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 ", "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/`), 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 diff --git a/apps/docs/reporter.md b/apps/docs/reporter.md index 9aad91ba..8531b604 100644 --- a/apps/docs/reporter.md +++ b/apps/docs/reporter.md @@ -88,7 +88,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 **screenshots** (`screenshot: 'only-on-failure'`) and **videos** (`video: 'retain-on-failure'`) — are uploaded automatically and shown as first-class evidence on the [execution](./evidence#one-execution-diagnosis-first) and failure-cluster pages, alongside traces. Screenshots are the evidence most pages on this site count on, and Playwright records none unless the option is set. Videos can be large, so pair `retain-on-failure` with periodic [storage cleanup](./storage#storage-management). +Any attachments Playwright records — including **screenshots** (`screenshot: 'only-on-failure'`) and **videos** (`video: 'retain-on-failure'`) — are uploaded automatically and shown as first-class evidence on the [execution](./evidence#one-execution-diagnosis-first) 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. Screenshots are the evidence most pages on this site count on, and Playwright records none unless the option is set. Videos can be large, so pair `retain-on-failure` with periodic [storage cleanup](./storage#storage-management). ## Configuration options @@ -192,7 +192,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 [execution page](./evidence#one-execution-diagnosis-first) 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] ✗ → <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/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/ diff --git a/packages/reporter/README.md b/packages/reporter/README.md index 052701ef..d36915c4 100644 --- a/packages/reporter/README.md +++ b/packages/reporter/README.md @@ -248,25 +248,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 @@ -275,8 +281,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/files/file-handler.ts b/packages/reporter/src/internal/files/file-handler.ts index bb41c011..3f731a6d 100644 Binary files a/packages/reporter/src/internal/files/file-handler.ts and b/packages/reporter/src/internal/files/file-handler.ts differ 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..1654618d 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); @@ -463,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/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) }]); + }); +}); 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); + }); +});