From 699fef4e68a08b3ee22ffb0f2e2505886f663e28 Mon Sep 17 00:00:00 2001
From: Niek Otten <not@respellion.nl>
Date: Fri, 4 Sep 2026 11:40:35 +0200
Subject: [PATCH] test(ci): the e2e job summary must name why a spec failed
 (refs #161)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

#161 lost a 36-minute verify-stack job whose only surviving output was a
single ✘ line: the per-spec summary (#136) renders a verdict icon and nothing
else, so a red e2e still costs a log dive — and when the log is truncated or
the run is killed, there is nothing to dive into.

Adds a stdlib assert-based self-check for infra/playwright-summary.py (no
framework) and rides it on `make unit` so CI catches a broken summary.
Fails with "AssertionError: Test timeout of 90000ms exceeded" not in the
rendered markdown.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 Makefile                         |  3 ++
 infra/test_playwright_summary.py | 88 ++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+)
 create mode 100644 infra/test_playwright_summary.py

diff --git a/Makefile b/Makefile
index 75d7163..d9efbc2 100644
--- a/Makefile
+++ b/Makefile
@@ -71,8 +71,11 @@ build:
 
 ## unit: run unit tests (excludes the container-backed Integration lane)
 # TRX per test project (→ TestResults/) feeds the CI per-service summary (#136); harmless locally.
+# The CI reporting scripts are stdlib Python with their own assert-based self-checks (#161) — they
+# ride this lane so a broken job summary is caught by CI rather than by the next red pipeline.
 unit:
 	dotnet test $(SLN) -c Release --filter "Category!=Integration" --logger trx --results-directory TestResults
+	python3 infra/test_playwright_summary.py
 
 ## mutation: run the Stryker.NET ratchet on each service with branching logic (fails below baseline)
 # Stryker is pinned as a local dotnet tool (.config/dotnet-tools.json); `tool restore`
diff --git a/infra/test_playwright_summary.py b/infra/test_playwright_summary.py
new file mode 100644
index 0000000..613ac0f
--- /dev/null
+++ b/infra/test_playwright_summary.py
@@ -0,0 +1,88 @@
+#!/usr/bin/env python3
+"""Self-check for infra/playwright-summary.py — stdlib asserts, no framework.
+
+Run: python3 infra/test_playwright_summary.py  (also runs in `make unit`).
+
+A red e2e is only useful if the job summary says WHY it failed: #161 lost a 36-minute
+verify-stack job whose only surviving output was one ✘ line with no assertion detail.
+"""
+import importlib.util
+import io
+import json
+import os
+import tempfile
+from contextlib import redirect_stdout
+
+# The script's filename is not a valid module name, so load it by path.
+spec = importlib.util.spec_from_file_location(
+    "playwright_summary",
+    os.path.join(os.path.dirname(os.path.abspath(__file__)), "playwright-summary.py"),
+)
+summary = importlib.util.module_from_spec(spec)
+spec.loader.exec_module(summary)
+
+
+def render(report):
+    """Run the renderer over a report dict and return its markdown."""
+    with tempfile.NamedTemporaryFile("w", suffix=".json", delete=False) as fh:
+        json.dump(report, fh)
+        path = fh.name
+    try:
+        out = io.StringIO()
+        with redirect_stdout(out):
+            summary.main(path)
+        return out.getvalue()
+    finally:
+        os.unlink(path)
+
+
+def spec_entry(title, status, errors=()):
+    return {
+        "title": title,
+        "file": "catalogus.spec.ts",
+        "ok": status == "expected",
+        "tests": [{"status": status, "results": [{"errors": [{"message": m} for m in errors]}]}],
+    }
+
+
+def test_failing_spec_reports_why():
+    md = render({
+        "stats": {"expected": 4, "unexpected": 1, "flaky": 0, "skipped": 0, "duration": 108_000},
+        "suites": [{"file": "catalogus.spec.ts", "specs": [
+            spec_entry("a beheerder sees the published zaaktypen in the catalogus", "unexpected",
+                       ["locator.fill: Test timeout of 90000ms exceeded.\n"
+                        "Call log:\n  - waiting for locator('#username')\n"]),
+        ]}],
+    })
+    assert "❌" in md, md
+    # The point of the slice: the summary names the cause, not just the verdict.
+    assert "Test timeout of 90000ms exceeded" in md, md
+    assert "waiting for locator('#username')" in md, md
+    # A multi-line Playwright error must not break out of its table row.
+    assert not any(line.startswith("Call log:") for line in md.splitlines()), md
+
+
+def test_passing_run_stays_quiet():
+    md = render({
+        "stats": {"expected": 1, "unexpected": 0, "flaky": 0, "skipped": 0, "duration": 5_000},
+        "suites": [{"file": "catalogus.spec.ts",
+                    "specs": [spec_entry("a beheerder sees the catalogus", "expected")]}],
+    })
+    assert "✅" in md, md
+    assert "timeout" not in md.lower(), md
+
+
+def test_missing_report_is_not_a_crash():
+    out = io.StringIO()
+    with redirect_stdout(out):
+        rc = summary.main("/nonexistent/playwright-report.json")
+    assert rc == 0
+    assert "did not reach the e2e step" in out.getvalue()
+
+
+if __name__ == "__main__":
+    for name, fn in sorted(globals().items()):
+        if name.startswith("test_") and callable(fn):
+            fn()
+            print(f"  ok  {name}")
+    print("playwright-summary self-check passed")