From 73eac7ff18f30a92abe9282a5a95ab729445fefb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Thu, 13 Aug 2026 19:35:09 +0100 Subject: [PATCH] ci(e2e): log the flake cross-check verdict on every run, not just mismatches (#2639) Co-authored-by: Kandev Agent --- apps/web/e2e/README.md | 6 +++ apps/web/e2e/scripts/flake-report.test.ts | 65 +++++++++++++++++++++++ apps/web/e2e/scripts/flake-report.ts | 30 ++++++++--- 3 files changed, 95 insertions(+), 6 deletions(-) diff --git a/apps/web/e2e/README.md b/apps/web/e2e/README.md index 6a327bdba3..ef6f8f8538 100644 --- a/apps/web/e2e/README.md +++ b/apps/web/e2e/README.md @@ -172,6 +172,12 @@ test are classified the way Playwright classifies them. The report cross-checks its count against `stats.flaky` in the merged Playwright JSON report for the same run and prints **MISMATCH** plus a workflow warning if the two disagree. +The cross-check reports its verdict in the workflow log on every run, not only +on a mismatch, and annotates a skipped check as a warning. If only a mismatch +spoke, a cross-check that had silently stopped running would be indistinguishable +in the log from one that passed, which is the same shape of bug as a flake that +never fails the build. + The trend is carried between runs as the `e2e-flake-history` artifact (90 days, capped at 50 entries, newest first). `e2e-report` looks for the newest completed `main` run that published one, uses it as the baseline, appends this run's entry, diff --git a/apps/web/e2e/scripts/flake-report.test.ts b/apps/web/e2e/scripts/flake-report.test.ts index af32ce9cec..f97e381f13 100644 --- a/apps/web/e2e/scripts/flake-report.test.ts +++ b/apps/web/e2e/scripts/flake-report.test.ts @@ -1,11 +1,14 @@ +import { execFileSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; import { appendHistoryEntry, baselineFrom, crossCheckFlakeCount, + crossCheckLogLine, entryFromSummary, MAX_HISTORY_ENTRIES, median, @@ -17,6 +20,8 @@ import { } from "./flake-report"; import type { RetrySummary, RetryTestSummary } from "./retry-summary"; +const scriptsRoot = path.dirname(fileURLToPath(import.meta.url)); + function entry(overrides: Partial = {}): FlakeHistoryEntry { const runId = overrides.runId ?? "100"; return { @@ -179,6 +184,66 @@ describe("playwright cross-check", () => { }); }); + // A cross-check that silently stopped running must not look like one that + // passed, so every status speaks in the workflow log, not just a mismatch. + it("logs a line for every cross-check status, warning on anything but a match", () => { + expect(crossCheckLogLine({ status: "match", ours: 3, theirs: 3 })).toBe( + "E2E flake cross-check: match. retry-summary and the merged Playwright report both report 3 flaky.", + ); + expect(crossCheckLogLine({ status: "mismatch", ours: 3, theirs: 4 })).toBe( + "::warning::E2E flake cross-check: MISMATCH. retry-summary reports 3 flaky, the merged Playwright " + + "report reports 4. The trended flake rate is not trustworthy for this run.", + ); + expect( + crossCheckLogLine({ + status: "unavailable", + ours: 3, + reason: "no merged Playwright JSON report", + }), + ).toBe( + "::warning::E2E flake cross-check: SKIPPED (no merged Playwright JSON report). " + + "The reported flake count of 3 was not verified against Playwright.", + ); + }); + + // Asserting the strings only proves they are right, not that the CLI ever + // prints them, so this drives the real entry point. Without it, restoring the + // "only a mismatch speaks" behaviour passes every other test in this file. + it("prints the cross-check verdict from the CLI on a match, not only on a mismatch", () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-flake-cli-")); + const summaryPath = path.join(directory, "retry-summary.json"); + const mergedPath = path.join(directory, "merged-report.json"); + fs.writeFileSync(summaryPath, JSON.stringify(summary({ flaky: 3 }))); + fs.writeFileSync( + mergedPath, + '{"suites":[],"errors":[],"stats":{"expected":10,"skipped":0,"unexpected":0,"flaky":3}}', + ); + + const stdout = execFileSync( + path.join(scriptsRoot, "../../node_modules/.bin/tsx"), + [ + path.join(scriptsRoot, "flake-report.ts"), + "--summary", + summaryPath, + "--step-summary", + path.join(directory, "step-summary.md"), + "--playwright-json", + mergedPath, + "--run-id", + "7", + "--branch", + "main", + "--sha", + "abcdef1234567", + ], + { encoding: "utf8" }, + ); + + expect(stdout.trim()).toBe( + "E2E flake cross-check: match. retry-summary and the merged Playwright report both report 3 flaky.", + ); + }, 30_000); + it("reports match, mismatch, and unavailable distinctly", () => { const stats = { expected: 10, unexpected: 0, flaky: 4, skipped: 0 }; diff --git a/apps/web/e2e/scripts/flake-report.ts b/apps/web/e2e/scripts/flake-report.ts index c2706da080..df5019c549 100644 --- a/apps/web/e2e/scripts/flake-report.ts +++ b/apps/web/e2e/scripts/flake-report.ts @@ -204,6 +204,29 @@ function runLabel(entry: FlakeHistoryEntry): string { return entry.runUrl ? `[${label}](${entry.runUrl})` : label; } +/** + * The workflow-log counterpart of the summary's cross-check line. Every status + * gets a line, including a match: if only a mismatch spoke, a cross-check that + * silently stopped running would be indistinguishable in the log from one that + * passed, and an unverified flake rate is the failure this whole report exists + * to prevent. A skip is annotated as a warning for the same reason. + */ +export function crossCheckLogLine(check: CrossCheck): string { + if (check.status === "match") { + return `E2E flake cross-check: match. retry-summary and the merged Playwright report both report ${check.ours} flaky.`; + } + if (check.status === "mismatch") { + return ( + `::warning::E2E flake cross-check: MISMATCH. retry-summary reports ${check.ours} flaky, ` + + `the merged Playwright report reports ${check.theirs}. The trended flake rate is not trustworthy for this run.` + ); + } + return ( + `::warning::E2E flake cross-check: SKIPPED (${check.reason}). ` + + `The reported flake count of ${check.ours} was not verified against Playwright.` + ); +} + function crossCheckLine(check: CrossCheck): string { if (check.status === "match") { return `- Playwright cross-check: **match** (${check.ours} flaky in both retry-summary and the merged report)`; @@ -337,12 +360,7 @@ function runCli(): void { ); } - if (check.status === "mismatch") { - console.log( - `::warning::E2E flake count mismatch: retry-summary reports ${check.ours}, ` + - `the merged Playwright report reports ${check.theirs}.`, - ); - } + console.log(crossCheckLogLine(check)); } if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) {