US09-05: Automate Documentation Acceptance

tests/e2e/test_docs_ui.py

@@ -18,6 +18,9 @@ import pytest
 
 from tests.e2e._pipeline_harness import Server, seed_library
 
+# Every check here belongs to the documentation gate (US09-05).
+pytestmark = pytest.mark.phase_i
+
 
 @pytest.fixture(scope="module")
 def server(tmp_path_factory):

tests/e2e/test_user_manual_screenshots.py

@@ -24,10 +24,13 @@ from contextlib import closing
 from pathlib import Path
 
 import pytest
+from PIL import Image
+from playwright.sync_api import TimeoutError as PlaywrightTimeout
 
 from tests.conftest import session_client
 from tests.e2e._pipeline_harness import (
     Server,
+    approve_album,
     seed_album,
     start_worker,
     wait_until,
@@ -39,21 +42,29 @@ IMAGES = DOCS / "images"
 ALBUM = "rome"
 VIEWPORT = {"width": 1280, "height": 900}
 
-# One per stage page of the manual: the route to visit, and the element whose
-# presence means the view has actually finished rendering.
+# One per stage page of the manual: the route, the heading that view renders, and the
+# element whose presence means it has finished.
+#
+# The heading matters more than it looks. Every route replaces the same container, so
+# waiting for "an h1" matches the *previous* view's heading and photographs the screen
+# you just left — which is how the statistics page first shipped a picture of the
+# archive view.
 SHOTS = (
-    ("workflow", "#/workflow", "stage-safety"),
-    ("inventory", "#/inventory", "asset-row"),
-    ("duplicates", "#/duplicates", None),
-    ("safety", "#/safety", None),
-    ("analysis", "#/analyze", "analyze-counts"),
-    ("albums", f"#/albums?album={ALBUM}", "suggested-name"),
-    ("renames", "#/renames", None),
-    ("uploads", "#/uploads", "upload-scope"),
-    ("archive", "#/archive", "archive-locations"),
-    ("statistics", "#/stats", None),
+    ("workflow", "#/workflow", "Workflow", "stage-safety"),
+    ("inventory", "#/inventory", "Inventory", "asset-row"),
+    ("duplicates", "#/duplicates", "Duplicate clusters", None),
+    ("safety", "#/safety", "Safety review", None),
+    ("analysis", "#/analyze", "Analyze", "analyze-counts"),
+    ("albums", f"#/albums?album={ALBUM}", "Albums", "suggested-name"),
+    ("renames", "#/renames", "Renames", None),
+    ("uploads", "#/uploads", "Upload", "upload-scope"),
+    ("archive", "#/archive", "Archive", "archive-locations"),
+    ("statistics", "#/stats", "Stats", None),
 )
 
+# Every check here belongs to the documentation gate (US09-05).
+pytestmark = pytest.mark.phase_i
+
 
 def writing() -> bool:
     return os.environ.get("PHOTO_PIPELINE_WRITE_SCREENSHOTS") == "1"
@@ -90,17 +101,23 @@ def _prepare(base: str, seeded) -> None:
         ).raise_for_status()
         wait_until(lambda: client.get("/api/v1/workflow").status_code == 200)
 
+    # An approved name and a built — deliberately unapplied — plan, so the renames
+    # screenshot shows the preview its page describes rather than an empty state.
+    # Building a plan moves nothing; applying it is what would, and nothing here does.
+    approve_album(base, album=ALBUM, name="2019 — Rome")
+    with closing(session_client(base, timeout=30)) as client:
+        client.post("/api/v1/rename-plans").raise_for_status()
+
 
 def _capture(page, server, target: Path) -> list[str]:
     target.mkdir(parents=True, exist_ok=True)
     page.set_viewport_size(VIEWPORT)
     written = []
-    for name, route, ready in SHOTS:
+    for name, route, heading, ready in SHOTS:
         page.goto(f"{server.base}/app/{route}")
+        page.locator("main h1", has_text=heading).first.wait_for(timeout=30_000)
         if ready:
             page.get_by_test_id(ready).first.wait_for(timeout=30_000)
-        else:
-            page.locator("main h1").first.wait_for(timeout=30_000)
         page.screenshot(path=str(target / f"{name}.png"))
         written.append(name)
     return written
@@ -110,7 +127,7 @@ def test_the_generator_produces_every_screenshot_the_manual_references(page, sta
     destination = IMAGES if writing() else tmp_path / "images"
     written = _capture(page, stack, destination)
 
-    assert sorted(written) == sorted(name for name, _, _ in SHOTS)
+    assert sorted(written) == sorted(name for name, *_ in SHOTS)
     for name in written:
         produced = destination / f"{name}.png"
         assert produced.stat().st_size > 5_000, f"{name}.png is too small to be a view"
@@ -133,6 +150,60 @@ def test_no_screenshot_shows_a_real_path_or_a_secret(page, stack, tmp_path):
     assert all(ALBUM in path or "images" in path for path in paths)
 
 
+def test_the_committed_screenshots_still_show_what_the_application_shows(page, stack, tmp_path):
+    """A UI change that invalidates the manual should be a red build, not a discovery
+    months later by somebody following a picture of a screen that no longer exists.
+
+    The tolerance is on *content*, not pixels. Comparing the committed PNGs with a
+    fresh capture was tried first and rejected: PNG output is not reproducible, font
+    rasterisation differs between the machine that generated an image and the machine
+    running the gate, and several views legitimately print the fixture library's
+    absolute path, which is a fresh temporary directory every run. A pixel or
+    perceptual comparison therefore fails for reasons that have nothing to do with
+    the manual being wrong — and a check that cries wolf gets deleted.
+
+    What actually invalidates a screenshot is the view no longer showing what its
+    page says it shows. That is what is compared here: the heading, the elements the
+    page describes, and the fact that the image was captured at the same size.
+
+    ponytail: content comparison, not pixels. A visual-diff service with per-platform
+    baselines is the upgrade if cosmetic regressions ever need catching too.
+    """
+    if not IMAGES.is_dir():
+        pytest.skip("no screenshots have been generated into the repository yet")
+
+    fresh = tmp_path / "fresh"
+    _capture(page, stack, fresh)
+
+    drifted = []
+    for name, route, heading, ready in SHOTS:
+        committed = IMAGES / f"{name}.png"
+        if not committed.is_file():
+            drifted.append(f"{name}: never committed")
+            continue
+        with Image.open(committed) as image:
+            if image.size != (VIEWPORT["width"], VIEWPORT["height"]):
+                drifted.append(f"{name}: committed at {image.size}, captured at {VIEWPORT}")
+        # The views render asynchronously, so each check waits rather than asking a
+        # question the page has not finished answering.
+        page.goto(f"{stack.base}/app/{route}")
+        try:
+            page.locator("main h1", has_text=heading).first.wait_for(timeout=15_000)
+        except PlaywrightTimeout:
+            drifted.append(f"{name}: the view no longer shows the heading '{heading}'")
+            continue
+        if ready:
+            try:
+                page.get_by_test_id(ready).first.wait_for(timeout=15_000)
+            except PlaywrightTimeout:
+                drifted.append(f"{name}: the view no longer renders '{ready}'")
+
+    assert drifted == [], (
+        f"the manual's screenshots no longer match the application: {drifted}. "
+        "Regenerate them with PHOTO_PIPELINE_WRITE_SCREENSHOTS=1 and review the result."
+    )
+
+
 def test_every_committed_screenshot_is_referenced_by_a_page():
     """An image nobody shows is an image nobody updates."""
     if not IMAGES.is_dir():

tests/integration/test_architecture_overview.py

@@ -14,10 +14,15 @@ from __future__ import annotations
 import re
 from pathlib import Path
 
+import pytest
+
 import photo_pipeline.models  # noqa: F401  (registers every table on Base.metadata)
 from photo_pipeline.db import Base
 from photo_pipeline.services import jobs, rename_journal, upload_batches
 
+# Every check here belongs to the documentation gate (US09-05).
+pytestmark = pytest.mark.phase_i
+
 REPO = Path(__file__).resolve().parents[2]
 PACKAGE = REPO / "photo_pipeline"
 OVERVIEW = REPO / "docs" / "architecture.md"

tests/integration/test_docs_gate.py

@@ -0,0 +1,196 @@
+"""US09-05: the documentation gate's own contract, checked without a browser.
+
+The gate itself needs a browser and a server and takes minutes; what is checkable
+offline is what makes it a *gate* rather than a long test run:
+
+* it selects every documentation check by marker, so adding one is enough to put it
+  in front of a release;
+* it accepts no skip at all — a check that did not run is a page nobody compared
+  with the code;
+* it retains checksummed evidence per run;
+* CI runs it on pull requests, which is when a fix is still cheap.
+
+The checks it runs are the four suites written by US09-01 through US09-04.
+"""
+
+from __future__ import annotations
+
+import json
+import re
+from pathlib import Path
+
+import pytest
+import yaml
+
+from photo_pipeline.config import Config
+from photo_pipeline.services import release
+
+REPO = Path(__file__).resolve().parents[2]
+DOCS = REPO / "docs"
+TEST_WORKFLOW = yaml.safe_load((REPO / ".gitea" / "workflows" / "test.yml").read_text())
+
+pytestmark = pytest.mark.phase_i
+
+# The suites the gate exists to run. Each is mapped to a story in the traceability
+# matrix, so this list is also what stops one being quietly dropped.
+DOCUMENTATION_SUITES = (
+    "tests/integration/test_documentation.py",
+    "tests/integration/test_installation_manual.py",
+    "tests/integration/test_architecture_overview.py",
+    "tests/integration/test_user_manual.py",
+    "tests/integration/test_docs_gate.py",
+    "tests/e2e/test_docs_ui.py",
+    "tests/e2e/test_user_manual_screenshots.py",
+)
+
+
+def _config(tmp_path) -> Config:
+    (tmp_path / "data").mkdir(exist_ok=True)
+    return Config.from_env({"PHOTO_PIPELINE_DATA_DIR": str(tmp_path / "data")})
+
+
+# ── what the gate runs ───────────────────────────────────────────────────────
+
+
+def test_the_gate_selects_every_documentation_check_by_marker():
+    assert release.DOCS_STAGES == (("documentation", ("tests", "-m", "phase_i")),)
+    declared = [
+        line
+        for line in (REPO / "pyproject.toml").read_text().splitlines()
+        if line.strip().startswith('"phase_i:')
+    ]
+    assert declared, "an unregistered marker selects nothing and fails no gate"
+
+
+def test_every_documentation_suite_carries_the_marker():
+    """A documentation test without the marker is a check the gate never runs."""
+    unmarked = [
+        suite
+        for suite in DOCUMENTATION_SUITES
+        if "pytest.mark.phase_i" not in (REPO / suite).read_text()
+    ]
+    assert unmarked == [], f"suites missing the phase_i marker: {unmarked}"
+
+
+def test_the_marker_selects_nothing_outside_the_documentation_suites():
+    """Scope matters: the gate's promise is that it ran *the documentation checks*."""
+    marked = sorted(
+        str(path.relative_to(REPO))
+        for path in (REPO / "tests").rglob("test_*.py")
+        if "pytest.mark.phase_i" in path.read_text()
+    )
+    assert marked == sorted(DOCUMENTATION_SUITES)
+
+
+# ── no skip is an environment limit here ─────────────────────────────────────
+
+
+def test_the_gate_accepts_no_skipped_check_at_all():
+    assert release.DOCS_ALLOWED_SKIP_REASONS == ()
+    skipped = release.StageResult(
+        "documentation", [], 0, 0.1, "1 skipped", ["SKIPPED [1] x.py:1: exiftool not installed"]
+    )
+    # The release gate tolerates this one because it describes the machine. The
+    # documentation gate cannot: nothing here depends on the machine.
+    assert release.unexpected_skips([skipped]) == []
+    assert release.unexpected_skips([skipped], release.DOCS_ALLOWED_SKIP_REASONS) == [
+        "SKIPPED [1] x.py:1: exiftool not installed"
+    ]
+
+
+def test_a_skipped_check_fails_the_run_and_the_evidence_says_so(tmp_path):
+    skipping = tmp_path / "test_skipping.py"
+    skipping.write_text(
+        "import pytest\n\ndef test_x():\n    pytest.skip('exiftool not installed')\n"
+    )
+    evidence = tmp_path / "evidence"
+
+    report = release.run_gate(
+        _config(tmp_path),
+        output=evidence,
+        stages=(("documentation", (str(skipping),)),),
+        allowed_skips=release.DOCS_ALLOWED_SKIP_REASONS,
+    )
+
+    assert report["ok"] is False
+    assert report["failures"] == [], "the stage passed; the skip is what fails the gate"
+    assert report["unexpected_skips"], report
+    written = json.loads((evidence / release.REPORT_NAME).read_text())
+    assert written["ok"] is False
+    assert (evidence / "logs" / "documentation.log").exists()
+    for line in (evidence / release.CHECKSUMS_NAME).read_text().splitlines():
+        digest, name = line.split("  ", 1)
+        assert release.sha256_file(evidence / name) == digest
+
+
+def test_a_broken_documentation_check_fails_the_gate(tmp_path):
+    """The failure mode that matters: a page and the code disagreeing."""
+    failing = tmp_path / "test_disagreement.py"
+    failing.write_text(
+        "def test_the_manual_matches_the_code():\n"
+        "    documented = {'lock_held'}\n"
+        "    emitted = {'lock_held', 'renamed_since'}\n"
+        "    assert emitted <= documented\n"
+    )
+
+    report = release.run_gate(
+        _config(tmp_path),
+        output=tmp_path / "evidence",
+        stages=(("documentation", (str(failing),)),),
+        allowed_skips=release.DOCS_ALLOWED_SKIP_REASONS,
+    )
+
+    assert report["ok"] is False
+    assert report["failures"], report
+
+
+def test_a_clean_tree_passes(tmp_path):
+    passing = tmp_path / "test_passing.py"
+    passing.write_text("def test_x():\n    assert True\n")
+
+    report = release.run_gate(
+        _config(tmp_path),
+        output=tmp_path / "evidence",
+        stages=(("documentation", (str(passing),)),),
+        allowed_skips=release.DOCS_ALLOWED_SKIP_REASONS,
+    )
+
+    assert report["ok"] is True and report["unexpected_skips"] == []
+    assert report["revision"], "the evidence must say which commit it covers"
+
+
+# ── the command, and CI ──────────────────────────────────────────────────────
+
+
+def test_the_command_is_documented_and_wired():
+    assert "docs-gate" in (REPO / "photo_pipeline" / "__main__.py").read_text()
+    assert "docs-gate" in (DOCS / "index.md").read_text()
+
+
+def test_ci_runs_the_gate_on_pull_requests_and_keeps_its_evidence():
+    job = TEST_WORKFLOW["jobs"]["documentation"]
+    # No `if:` — documentation drifts on the same commits that change behaviour, and
+    # the pull request is when saying so is still cheap.
+    assert "if" not in job
+    assert "pull_request" in TEST_WORKFLOW[True] or "pull_request" in TEST_WORKFLOW.get("on", {})
+    script = "\n".join(step["run"] for step in job["steps"] if "run" in step)
+    assert "photo_pipeline docs-gate" in script
+    evidence = next(step for step in job["steps"] if "upload-artifact" in str(step.get("uses")))
+    assert evidence["if"] == "always()", "a failed gate's evidence is the one worth keeping"
+
+
+# ── the pages and the repository point at each other ─────────────────────────
+
+
+def test_the_readme_and_the_documentation_index_link_to_each_other():
+    """Neither may become the forgotten copy."""
+    readme = (REPO / "README.md").read_text()
+    index = (DOCS / "index.md").read_text()
+    assert "docs/index.md" in readme
+    assert re.search(r"\.\./README\.md", index), "the index does not link back to the README"
+
+
+def test_every_documentation_story_is_mapped_in_the_traceability_matrix():
+    matrix = json.loads((REPO / "tests" / "story_traceability.json").read_text())["stories"]
+    for story in ("US09-01", "US09-02", "US09-03", "US09-04", "US09-05"):
+        assert matrix.get(story), f"{story} is not mapped to any test"

tests/integration/test_documentation.py

@@ -16,12 +16,17 @@ import json
 import re
 from pathlib import Path
 
+import pytest
+
 from starlette.testclient import TestClient
 
 from photo_pipeline.api.app import create_app
 from photo_pipeline.api.security import DEFAULT_HEADERS
 from photo_pipeline.config import Config
 
+# Every check here belongs to the documentation gate (US09-05).
+pytestmark = pytest.mark.phase_i
+
 REPO = Path(__file__).resolve().parents[2]
 DOCS = REPO / "docs"
 VENDOR = REPO / "frontend" / "js" / "vendor"

tests/integration/test_installation_manual.py

@@ -16,8 +16,13 @@ import sys
 from functools import lru_cache
 from pathlib import Path
 
+import pytest
+
 from photo_pipeline.config import ENV_PREFIX, LEGACY_ALIASES, Config
 
+# Every check here belongs to the documentation gate (US09-05).
+pytestmark = pytest.mark.phase_i
+
 REPO = Path(__file__).resolve().parents[2]
 MANUAL = REPO / "docs" / "installation.md"
 MAIN = REPO / "photo_pipeline" / "__main__.py"

tests/integration/test_user_manual.py

@@ -14,6 +14,8 @@ from __future__ import annotations
 import re
 from pathlib import Path
 
+import pytest
+
 REPO = Path(__file__).resolve().parents[2]
 DOCS = REPO / "docs"
 STAGES = DOCS / "stages"
@@ -55,6 +57,9 @@ STAGE_PAGES = (
     "diagnostics",
 )
 
+# Every check here belongs to the documentation gate (US09-05).
+pytestmark = pytest.mark.phase_i
+
 
 def emitted_codes() -> set[str]:
     found: set[str] = set()

tests/story_traceability.json

@@ -208,10 +208,11 @@
     "US09-04": [
       "tests/integration/test_user_manual.py",
       "tests/e2e/test_user_manual_screenshots.py"
+    ],
+    "US09-05": [
+      "tests/integration/test_docs_gate.py"
     ]
   },
-  "planned": [
-    "US09-05"
-  ],
+  "planned": [],
   "_planned_comment": "Accepted backlog stories that are not implemented yet. The release gate (US07-07) requires every story file to be either mapped to tests or listed here, so an unimplemented story is a visible decision rather than a hole in the matrix."
 }