feat(shot): harness default capture fix — bounded networkidle settle after domcontentloaded + blank-frame retry (≤60s wait budget, R7 best-effort preserved); 6 unit tests; lint PASS, 205 unit tests pass via cc-ci-run
All checks were successful
continuous-integration/drone/push Build is passing
All checks were successful
continuous-integration/drone/push Build is passing
This commit is contained in:
@ -18,6 +18,7 @@ missing, app slow, navigation error) is swallowed and returns None so the run/ve
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import contextlib
|
||||
import os
|
||||
|
||||
from . import browser as harness_browser
|
||||
@ -28,6 +29,55 @@ VIEWPORT = {"width": 1280, "height": 800}
|
||||
# Hard cap so a wedged app can never hang the run on the screenshot step (R7 / Phase-1 timeouts).
|
||||
NAV_DEADLINE_S = 45
|
||||
|
||||
# ---- post-navigation settle (phase-shot fix, 2026-06-11) ----
|
||||
# SPAs (immich, n8n, cryptpad, the keycloak admin console, lasuite-*, mumble-web, mattermost) fire
|
||||
# `domcontentloaded` on their empty HTML shell and only paint after the JS bundle loads — snapping
|
||||
# immediately produced solid blank frames (byte-stable 4801-2 B) or loading spinners. After nav,
|
||||
# wait for network-idle up to SETTLE_TIMEOUT_MS (apps that never go idle — continuous polling —
|
||||
# simply spend the cap; bounded, never raises), then RENDER_GRACE_MS for the final paint.
|
||||
SETTLE_TIMEOUT_MS = 10_000
|
||||
RENDER_GRACE_MS = 500
|
||||
# A 1280x800 PNG below this is near-certainly a solid frame or a bare loading spinner (phase-shot
|
||||
# audit: blank frames were 4801-2 B across three different apps, lone spinners 5.9-8.8 KB; the
|
||||
# smallest real page was 12950 B). One bounded retry with an extra settle, then keep what we get —
|
||||
# an honest late frame beats none, and the retry only ever replaces a tiny frame with a later one.
|
||||
BLANK_SIZE_BYTES = 10_000
|
||||
BLANK_RETRY_SETTLE_MS = 4_000
|
||||
# Wait-budget arithmetic (plan-phase-shot §3 P3: step worst case ≤ ~60s): NAV_DEADLINE_S (45s,
|
||||
# spent only while the app isn't serving yet) + SETTLE_TIMEOUT_MS + RENDER_GRACE_MS +
|
||||
# BLANK_RETRY_SETTLE_MS + RENDER_GRACE_MS = 60s of bounded waiting; tested in unit tests.
|
||||
|
||||
|
||||
def _settle(page, idle_timeout_ms: int) -> None:
|
||||
"""Best-effort bounded settle: network-idle up to the cap, then a short render grace.
|
||||
Never raises (R7) — a timeout just means the page kept polling; we snap what's painted."""
|
||||
# cosmetic path (R7): a timeout on a never-idle app is expected — the cap IS the wait
|
||||
with contextlib.suppress(Exception):
|
||||
page.wait_for_load_state("networkidle", timeout=idle_timeout_ms)
|
||||
with contextlib.suppress(Exception):
|
||||
page.wait_for_timeout(RENDER_GRACE_MS)
|
||||
|
||||
|
||||
def _snap_with_blank_retry(page, out_path: str) -> None:
|
||||
"""Screenshot the page; if the PNG is blank/spinner-sized, retry ONCE after a longer settle.
|
||||
The retry overwrites the tiny frame with a strictly-later one (same page, more paint time)."""
|
||||
page.screenshot(path=out_path, full_page=False)
|
||||
try:
|
||||
first = os.path.getsize(out_path)
|
||||
except OSError:
|
||||
return
|
||||
if first >= BLANK_SIZE_BYTES:
|
||||
return
|
||||
print(
|
||||
f" screenshot: frame looks blank/loading ({first} B < {BLANK_SIZE_BYTES} B) — "
|
||||
"one retry after a longer settle",
|
||||
flush=True,
|
||||
)
|
||||
_settle(page, BLANK_RETRY_SETTLE_MS)
|
||||
page.screenshot(path=out_path, full_page=False)
|
||||
with contextlib.suppress(OSError):
|
||||
print(f" screenshot: retry frame {os.path.getsize(out_path)} B", flush=True)
|
||||
|
||||
|
||||
def screenshot_path(run_artifact_dir: str) -> str:
|
||||
"""Canonical on-disk path for a run's app screenshot (pure)."""
|
||||
@ -79,7 +129,7 @@ def capture(domain: str, out_path: str, *, recipe_meta: dict | None = None) -> s
|
||||
# the uniform ctx convention (rcust P3).
|
||||
hook(page, meta_mod.hook_ctx(domain, recipe_meta))
|
||||
if not os.path.exists(out_path):
|
||||
page.screenshot(path=out_path, full_page=False)
|
||||
_snap_with_blank_retry(page, out_path)
|
||||
else:
|
||||
# Default: landing page. Accept any rendered status (200 or an auth redirect to a
|
||||
# login form) — both are credential-free and representative of "the app is up".
|
||||
@ -90,7 +140,9 @@ def capture(domain: str, out_path: str, *, recipe_meta: dict | None = None) -> s
|
||||
deadline_seconds=NAV_DEADLINE_S,
|
||||
wait_until="domcontentloaded",
|
||||
)
|
||||
page.screenshot(path=out_path, full_page=False)
|
||||
# SPA paint race fix (phase-shot): settle before snapping, retry a blank frame.
|
||||
_settle(page, SETTLE_TIMEOUT_MS)
|
||||
_snap_with_blank_retry(page, out_path)
|
||||
finally:
|
||||
browser.close()
|
||||
if os.path.exists(out_path) and os.path.getsize(out_path) > 0:
|
||||
|
||||
@ -32,6 +32,90 @@ def test_hook_returned_when_callable():
|
||||
assert S._load_screenshot_hook({"SCREENSHOT": hook}) is hook
|
||||
|
||||
|
||||
class _FakePage:
|
||||
"""Minimal Playwright-page stand-in for the settle/blank-retry helpers (no browser needed)."""
|
||||
|
||||
def __init__(self, shot_sizes, idle_raises=False):
|
||||
self._shot_sizes = list(shot_sizes) # bytes written per successive screenshot() call
|
||||
self._idle_raises = idle_raises
|
||||
self.idle_waits = [] # (state, timeout) per wait_for_load_state call
|
||||
self.timeout_waits = [] # ms per wait_for_timeout call
|
||||
self.shots = 0
|
||||
|
||||
def wait_for_load_state(self, state, timeout=None):
|
||||
self.idle_waits.append((state, timeout))
|
||||
if self._idle_raises:
|
||||
raise TimeoutError(f"page kept polling past {timeout}ms")
|
||||
|
||||
def wait_for_timeout(self, ms):
|
||||
self.timeout_waits.append(ms)
|
||||
|
||||
def screenshot(self, path, full_page=False):
|
||||
self.shots += 1
|
||||
with open(path, "wb") as f:
|
||||
f.write(b"\x89PNG" + b"\0" * (self._shot_sizes.pop(0) - 4))
|
||||
|
||||
|
||||
def test_settle_swallows_never_idle_pages():
|
||||
"""R7: an app that never reaches network-idle (continuous polling) must not raise — the
|
||||
timeout cap IS the wait."""
|
||||
page = _FakePage([], idle_raises=True)
|
||||
S._settle(page, 1234) # must not raise
|
||||
assert page.idle_waits == [("networkidle", 1234)]
|
||||
assert page.timeout_waits == [S.RENDER_GRACE_MS]
|
||||
|
||||
|
||||
def test_snap_retries_blank_frame(tmp_path):
|
||||
"""A blank-sized first frame (audit fingerprint: 4801 B) triggers exactly one retry with a
|
||||
longer settle, overwriting the tiny frame with the later (painted) one."""
|
||||
out = str(tmp_path / "shot.png")
|
||||
page = _FakePage([4801, 30256])
|
||||
S._snap_with_blank_retry(page, out)
|
||||
assert page.shots == 2
|
||||
assert page.idle_waits == [("networkidle", S.BLANK_RETRY_SETTLE_MS)]
|
||||
assert os.path.getsize(out) == 30256
|
||||
|
||||
|
||||
def test_snap_no_retry_for_real_frame(tmp_path):
|
||||
"""A real-sized first frame is kept as-is — no second screenshot, no extra waiting."""
|
||||
out = str(tmp_path / "shot.png")
|
||||
page = _FakePage([35707])
|
||||
S._snap_with_blank_retry(page, out)
|
||||
assert page.shots == 1
|
||||
assert page.idle_waits == []
|
||||
assert os.path.getsize(out) == 35707
|
||||
|
||||
|
||||
def test_snap_retry_keeps_late_frame_even_if_still_blank(tmp_path):
|
||||
"""If the retry frame is still tiny we keep it (honest best-effort) — exactly one retry,
|
||||
never a loop."""
|
||||
out = str(tmp_path / "shot.png")
|
||||
page = _FakePage([4801, 4801])
|
||||
S._snap_with_blank_retry(page, out)
|
||||
assert page.shots == 2
|
||||
assert os.path.getsize(out) == 4801
|
||||
|
||||
|
||||
def test_blank_threshold_brackets_observed_sizes():
|
||||
"""Threshold sits between the audited defect sizes (blank 4801-2 B, lone spinners up to
|
||||
8764 B) and the smallest real page (custom-html-tiny, 12950 B)."""
|
||||
for defect in (4801, 4802, 5895, 6022, 7913, 8764):
|
||||
assert defect < S.BLANK_SIZE_BYTES
|
||||
assert S.BLANK_SIZE_BYTES < 12950
|
||||
|
||||
|
||||
def test_wait_budget_within_step_cap():
|
||||
"""plan-phase-shot §3 P3: the screenshot step's bounded waiting must stay ≤ ~60s worst case."""
|
||||
total_ms = (
|
||||
S.NAV_DEADLINE_S * 1000
|
||||
+ S.SETTLE_TIMEOUT_MS
|
||||
+ S.RENDER_GRACE_MS
|
||||
+ S.BLANK_RETRY_SETTLE_MS
|
||||
+ S.RENDER_GRACE_MS
|
||||
)
|
||||
assert total_ms <= 60_000, f"screenshot wait budget {total_ms}ms exceeds the ~60s step cap"
|
||||
|
||||
|
||||
def test_screenshot_reachable_through_real_load_path(tmp_path):
|
||||
"""R2 proof (rcust P1): a recipe SCREENSHOT hook declared in recipe_meta.py arrives at
|
||||
screenshot._load_screenshot_hook through the REAL orchestrator load path (meta.load — the
|
||||
|
||||
Reference in New Issue
Block a user