Web UI smoke: a screenshot that differs from its baseline is reported, not failed (#5931)

.github/workflows/darling-ui-smoke.yml

@@ -5,6 +5,8 @@ name: Darling UI smoke
 # the seed says must have data and does not. #5744 (phase 2): four pages are also compared with screenshot baselines
 # (tools/ui-smoke/baselines/*.png, in Playwright's own pinned Chromium). See tools/ui-smoke/ (run-ui-smoke.ps1 is
 # the entry, locally and here).
+# The screenshot compare is advisory (#5931): a page that differs from its baseline is reported in the job summary and
+# the darling-ui-smoke artifact and does not fail the job. Real breakage still does.
 #
 # Triggers: a pull request touching the web files, the harness or this workflow (a draft pull request is skipped
 # until it is marked ready), the nightly (workflow_call), and a manual dispatch. `pull_request`, never `pull_request_target`: the job runs the
@@ -132,6 +134,25 @@ jobs:
           if ($env:UPDATE_BASELINES -eq 'true') { $update.UpdateBaselines = $true }
           ./tools/ui-smoke/run-ui-smoke.ps1 -PgBin "$env:GITHUB_WORKSPACE\Darling\artifacts\pg-runtime\pgsql\bin" -PgPort 5541 -WorkDir "$env:RUNNER_TEMP\ui-smoke" -NoBuild -Channel chrome @update
 
+      # #5931: the per-page screenshot pixel counts, from the report.json that smoke.mjs writes (nothing is parsed from the log).
+      # Nothing to show for an update_baselines dispatch (nothing is compared) or when the run died before the shots.
+      - name: Screenshot compare summary
+        if: always()
+        shell: pwsh
+        run: |
+          $report = "$env:RUNNER_TEMP\ui-smoke\out\report.json"
+          if (-not (Test-Path $report)) { return }
+          $shots = @((Get-Content $report -Raw | ConvertFrom-Json).screenshots)
+          if ($shots.Count -eq 0) { return }
+          $lines = @('### Screenshot compare (advisory)', '', '| Page | Pixels that differ | Percent | Result |', '| --- | ---: | ---: | --- |')
+          foreach ($s in $shots) {
+            $result = if ($s.differing -gt 0) { 'differs' } else { 'matches' }
+            $lines += "| $($s.name) | $($s.differing) | $($s.pct) | $result |"
+          }
+          $lines += ''
+          $lines += 'A differing screenshot does not fail the job. The baseline, actual and diff images are in the `darling-ui-smoke` artifact, under `ui-smoke/out/shot-failures`.'
+          $lines -join "`n" | Add-Content -Path $env:GITHUB_STEP_SUMMARY -Encoding utf8
+
       - name: Logs tail
         if: always()
         shell: pwsh
@@ -142,7 +163,7 @@ jobs:
           }
           if (Test-Path "$env:RUNNER_TEMP\darling-pg.log") { "== PostgreSQL log (last 20 lines)"; Get-Content "$env:RUNNER_TEMP\darling-pg.log" -Tail 20 }
 
-      - name: Upload logs, failure screenshots and traces
+      - name: Upload logs, screenshot differences and traces
         if: always()
         uses: actions/upload-artifact@v6
         with:

tools/ui-smoke/README.md

@@ -57,9 +57,23 @@ you call `node smoke.mjs` yourself.
 ## Screenshot baselines
 
 Four pages have a baseline PNG in `baselines/`: `fleet`, `ui-sql-overview`, `ui-sql-queries` and `admin-servers`. After the
-smoke walk, `shots.mjs` renders each one at 1280 by 800 and compares it with its baseline. The check fails when any pixel
-differs by more than a small color threshold. The renders are the same from run to run, so there is no allowance. A page is marked by a `screenshot` key in `pages.json`.
-The key lists CSS selectors to paint over, for text that changes from run to run.
+smoke walk, `shots.mjs` renders each one at 1280 by 800 and compares it with its baseline. A pixel counts as different when
+it differs by more than a small color threshold. The renders are the same from run to run, so there is no allowance.
+A page is marked by a `screenshot` key in `pages.json`. The key lists CSS selectors to paint over, for text that changes
+from run to run.
+
+The compare is advisory (#5931). A page that differs from its baseline prints one `NOTICE screenshot <page>: <n> pixels differ`
+line and writes its baseline, actual and diff images. It does not fail the job. The last lines of the run say how many
+pages matched, how many differ and how many had problems. The workflow also writes a table of the per-page pixel counts to
+the job summary.
+
+What still fails the job:
+
+- A page that does not load, or a console or page error.
+- A panel or grid that the page's `expect` entry requires and that is missing or empty.
+- A step that fails (the stage that shoots the pages throws, the seed cannot be applied, the service does not start).
+- A screenshot problem that is not a pixel difference: no baseline for a page, a size that differs from the baseline, a
+  page that never settles, or a fleet that never turns collection stale.
 
 How the page is kept the same on every run:
 
@@ -77,22 +91,24 @@ How the page is kept the same on every run:
 - The database size in the status bar is not the same on every run. Its width changes when it gains a digit (99 MB to 100 MB).
   In one run the item was missing, because the read of the size had not finished. A mask hides the text, not the box. For the
   shots, `shots.mjs` adds a style that hides the item and the separator before it, so the bar is the same either way.
-- `report.json` is `{ pages, screenshotProblems }`. The page walk is written first, then rewritten with the screenshot
-  problems, so a failure in the screenshot step cannot lose the walk.
+- `report.json` is `{ pages, screenshotProblems, screenshots }`. `screenshots` has the name, pixel count and percentage of
+  each compared page. The page walk is written first, then rewritten with the screenshot results, so a failure in the
+  screenshot step cannot lose the walk.
 - Playwright's Chromium lives in the folder named by `PLAYWRIGHT_BROWSERS_PATH`. The workflow sets it and caches that folder.
 
 To refresh the baselines (after a change to the page that you want to keep):
 
-1. Start the workflow "Darling UI smoke" by hand on your branch, with `update_baselines` set. For example:
+1. Start the workflow "Darling UI smoke" by hand on your branch with `update_baselines=true`. For example:
    `gh workflow run darling-ui-smoke.yml --ref <branch> -f update_baselines=true`.
 2. Download the artifact `darling-ui-smoke-new-baselines` from that run.
 3. Copy the PNGs into `tools/ui-smoke/baselines/` and commit them on the pull request.
 4. Review GitHub's image diff of each PNG. Never commit a PNG that you rendered on your own machine. Fonts and the browser
    differ, so most pixels differ in CI.
 
-When a compare fails, the job uploads the artifact `darling-ui-smoke-screenshot-diffs`. For each page that differs, it holds
-`<page>.baseline.png`, `<page>.actual.png` and `<page>.diff.png` (the changed pixels). If the change is wanted, refresh the
-baselines as above. If a runner image update moved the pixels, refresh them the same way.
+Every run, pass or fail, uploads the artifact `darling-ui-smoke`. For each page that differs, its `ui-smoke/out/shot-failures` folder
+holds `<page>.baseline.png`, `<page>.actual.png` and `<page>.diff.png` (the changed pixels). If the change is wanted, refresh
+the baselines as above. If a runner image update moved the pixels, refresh them the same way. A drift that nobody refreshes
+does not fail anything, so look at the NOTICE lines on a pull request that changes a page.
 
 ## Add a page or a seeded table
 

tools/ui-smoke/shots.mjs

@@ -129,14 +129,16 @@ async function shoot(browser, spec, ctx) {
   }
 }
 
-/** Run the screenshot pages. Returns the problems (strings); writes PNGs and, on a mismatch, the baseline/actual/diff triple. */
+/** Run the screenshot pages. Returns { problems, shots }. `problems` (strings) fail the run: a page that would not shoot, no baseline, a
+ *  size change. `shots` is one { name, differing, pct } per compared page and is advisory (#5931): a page that differs from its baseline
+ *  prints a NOTICE and writes its baseline/actual/diff triple, but is not a problem. */
 export async function runShots({ config, baseUrl, outDir, only, settle }) {
   const mode = (process.env.UI_SMOKE_SHOTS || "compare").toLowerCase();
-  if (mode === "off") return [];
+  if (mode === "off") return { problems: [], shots: [] };
   const specs = config.pages.filter((p) => p.screenshot && (!only.length || only.includes(p.name)));
-  if (specs.length === 0) return [];
+  if (specs.length === 0) return { problems: [], shots: [] };
   const anchorMs = parseAnchor(process.env.UI_SMOKE_ANCHOR);
-  if (Number.isNaN(anchorMs)) return ["screenshots: UI_SMOKE_ANCHOR (the seed's anchor, UTC) is not set; run through run-ui-smoke.ps1"];
+  if (Number.isNaN(anchorMs)) return { problems: ["screenshots: UI_SMOKE_ANCHOR (the seed's anchor, UTC) is not set; run through run-ui-smoke.ps1"], shots: [] };
   const baselines = process.env.UI_SMOKE_BASELINES || join(dirname(fileURLToPath(import.meta.url)), "baselines");
   const newDir = join(outDir, "new-baselines");
   const failDir = join(outDir, "shot-failures");
@@ -170,13 +172,13 @@ export async function runShots({ config, baseUrl, outDir, only, settle }) {
         break;
       }
       if (offline > 0) {
-        return [`screenshots: the fleet is already past collection stale (${seen}); the cards go Offline 30 minutes after the seed anchor, so the walk ran too long for the seed to stay stale. Shorten the walk or re-seed.`];
+        return { problems: [`screenshots: the fleet is already past collection stale (${seen}); the cards go Offline 30 minutes after the seed anchor, so the walk ran too long for the seed to stay stale. Shorten the walk or re-seed.`], shots: [] };
       }
     } catch {
       /* the service answers again on the next poll */
     }
     if (Date.now() > staleDeadline) {
-      return [`screenshots: the fleet never reached collection stale in ${STALE_WAIT_MS / 60000} minutes (${seen}); the band opens 2 minutes after the seed anchor, so the service clock or the seed anchor is wrong, or /api/fleet stopped answering`];
+      return { problems: [`screenshots: the fleet never reached collection stale in ${STALE_WAIT_MS / 60000} minutes (${seen}); the band opens 2 minutes after the seed anchor, so the service clock or the seed anchor is wrong, or /api/fleet stopped answering`], shots: [] };
     }
     await new Promise((r) => setTimeout(r, 5000));
   }
@@ -185,6 +187,7 @@ export async function runShots({ config, baseUrl, outDir, only, settle }) {
      updates with the image and would move pixels. */
   const browser = await chromium.launch({ headless: true });
   const problems = [];
+  const shots = [];
   const ctx = { baseUrl, anchorMs, deltaMs: FIXED_ANCHOR_MS - anchorMs, settle, config };
   try {
     for (const spec of specs) {
@@ -220,10 +223,11 @@ export async function runShots({ config, baseUrl, outDir, only, settle }) {
       const diff = new PNG({ width: base.width, height: base.height });
       const differing = pixelmatch(base.data, actual.data, diff.data, base.width, base.height, { threshold: PIXEL_THRESHOLD });
       const ratio = differing / (base.width * base.height);
+      shots.push({ name: spec.name, differing, pct: Number((ratio * 100).toFixed(3)) });
       if (differing > MAX_DIFF_PIXELS) {
+        /* Advisory (#5931): reported with its images, never a problem, so a layout change on a page does not fail the job. */
         writeFileSync(join(failDir, `${spec.name}.diff.png`), PNG.sync.write(diff));
-        problems.push(`screenshot ${spec.name}: ${differing} pixels differ (${(ratio * 100).toFixed(3)}%, none allowed); baseline, actual and diff are in shot-failures`);
-        console.log(`FAIL shot ${spec.name}: ${differing} pixels (${(ratio * 100).toFixed(3)}%) differ`);
+        console.log(`NOTICE screenshot ${spec.name}: ${differing} pixels differ (${(ratio * 100).toFixed(3)}%); baseline, actual and diff are in shot-failures`);
       } else {
         /* A pass keeps no triple: the failure folder holds only what failed. */
         for (const suffix of ["actual", "baseline"]) rmQuiet(join(failDir, `${spec.name}.${suffix}.png`));
@@ -233,7 +237,7 @@ export async function runShots({ config, baseUrl, outDir, only, settle }) {
   } finally {
     await browser.close();
   }
-  return problems;
+  return { problems, shots };
 }
 
 function rmQuiet(p) {

tools/ui-smoke/smoke.mjs

@@ -216,20 +216,26 @@ for (const spec of config.pages) {
 }
 await browser.close();
 
-/* The walk's result lands first (report.json is { pages, screenshotProblems }), so nothing the screenshot stage does can lose it. */
+/* The walk's result lands first (report.json is { pages, screenshotProblems, screenshots }), so nothing the screenshot stage does can lose it. */
 writeFileSync(join(outDir, "report.json"), JSON.stringify({ pages: results, screenshotProblems: [] }, null, 2));
 
 /* Screenshot baselines (#5744): the pages marked "screenshot" in pages.json, in Playwright's own pinned Chromium. A throw out of the
    stage (Chromium not installed, an unwritable folder) is a screenshot problem like any other: it fails the run, it does not crash it. */
 let shotProblems;
+let shots = [];
 try {
-  shotProblems = await runShots({ config, baseUrl, outDir, only, settle });
+  ({ problems: shotProblems, shots } = await runShots({ config, baseUrl, outDir, only, settle }));
 } catch (e) {
   shotProblems = [`screenshots: the stage threw: ${e && e.message ? e.message.split("\n")[0] : e}`];
 }
 for (const p of shotProblems) console.log(`       PROBLEM ${p}`);
+/* The compare is advisory (#5931): a differing screenshot is in `shots` and printed as a NOTICE; only shotProblems fail the run. */
+if ((process.env.UI_SMOKE_SHOTS || "compare").toLowerCase() === "compare" && (shots.length || shotProblems.length)) {
+  const differ = shots.filter((x) => x.differing > 0).length;
+  console.log(`screenshots: ${shots.length - differ} matched, ${differ} differ (advisory), ${shotProblems.length} problems`);
+}
 
-writeFileSync(join(outDir, "report.json"), JSON.stringify({ pages: results, screenshotProblems: shotProblems }, null, 2));
+writeFileSync(join(outDir, "report.json"), JSON.stringify({ pages: results, screenshotProblems: shotProblems, screenshots: shots }, null, 2));
 const failed = results.filter((r) => r.problems.length);
 console.log(`\nui-smoke: ${results.length - failed.length}/${results.length} pages clean, ${results.filter((r) => r.flaked).length} flaked`);
 if (results.length === 0) {