fix(shot): A1 — blank-retry keeps the LARGER frame (retry snapped to temp path, os.replace only if >= first; worse late frame discarded + temp cleaned); regression test [9999,4801]->9999; 207 unit tests pass, lint PASS
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:
@ -66,7 +66,9 @@ def settle(page, idle_timeout_ms: int = SETTLE_TIMEOUT_MS) -> None:
|
||||
|
||||
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)."""
|
||||
The retry is snapped to a temp path and kept only if it is >= the first frame's size — later
|
||||
is usually more painted, but a page can also regress (redirect, error overlay) and a worse
|
||||
frame must never overwrite a better one (adversary finding A1)."""
|
||||
page.screenshot(path=out_path, full_page=False)
|
||||
try:
|
||||
first = os.path.getsize(out_path)
|
||||
@ -80,9 +82,19 @@ def _snap_with_blank_retry(page, out_path: str) -> None:
|
||||
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)
|
||||
retry_path = out_path + ".retry"
|
||||
try:
|
||||
page.screenshot(path=retry_path, full_page=False)
|
||||
retry = os.path.getsize(retry_path)
|
||||
if retry >= first:
|
||||
os.replace(retry_path, out_path)
|
||||
print(f" screenshot: retry frame kept ({retry} B >= {first} B)", flush=True)
|
||||
else:
|
||||
os.remove(retry_path)
|
||||
print(f" screenshot: retry frame discarded ({retry} B < {first} B)", flush=True)
|
||||
finally:
|
||||
with contextlib.suppress(OSError):
|
||||
os.remove(retry_path)
|
||||
|
||||
|
||||
def screenshot_path(run_artifact_dir: str) -> str:
|
||||
|
||||
@ -94,6 +94,19 @@ def test_snap_retry_keeps_late_frame_even_if_still_blank(tmp_path):
|
||||
S._snap_with_blank_retry(page, out)
|
||||
assert page.shots == 2
|
||||
assert os.path.getsize(out) == 4801
|
||||
assert not os.path.exists(out + ".retry"), "temp retry frame must be cleaned up"
|
||||
|
||||
|
||||
def test_snap_retry_never_regresses_to_smaller_frame(tmp_path):
|
||||
"""Adversary finding A1: a partial-but-real first frame (just under the threshold) must
|
||||
survive a retry that comes back WORSE (page regressed to blank during the extra settle) —
|
||||
the larger frame wins."""
|
||||
out = str(tmp_path / "shot.png")
|
||||
page = _FakePage([9999, 4801])
|
||||
S._snap_with_blank_retry(page, out)
|
||||
assert page.shots == 2
|
||||
assert os.path.getsize(out) == 9999, "retry must never overwrite a larger frame (A1)"
|
||||
assert not os.path.exists(out + ".retry"), "temp retry frame must be cleaned up"
|
||||
|
||||
|
||||
def test_blank_threshold_brackets_observed_sizes():
|
||||
|
||||
Reference in New Issue
Block a user