diff --git a/src/htmlcmp/common.py b/src/htmlcmp/common.py
index 9caa5f2..94b30b1 100644
--- a/src/htmlcmp/common.py
+++ b/src/htmlcmp/common.py
@@ -6,6 +6,8 @@
from htmlcmp.html_render_diff import get_browser, html_render_diff
+logger = logging.getLogger(__name__)
+
class bcolors:
HEADER = "\033[95m"
@@ -40,22 +42,41 @@ def compare_json(a: Path, b: Path) -> bool:
return json_a == json_b
-def compare_html(a: Path, b: Path, browser=None, diff_output: Path = None) -> bool:
+def compare_html(
+ a: Path, b: Path, browser=None, diff_output: Path = None, retries: int = 1
+) -> bool:
+ """Whether `a` and `b` render to the same pixels.
+
+ A mismatch is rendered again before it is reported, `retries` times. A real
+ difference is in the markup and comes back every time; one that does not is
+ the page having been caught mid-render, and a browser gives no promise that
+ two runs of the same page paint alike at the same instant. The retry costs
+ nothing on the matching files, which are almost all of them.
+ """
if not isinstance(a, Path) or not isinstance(b, Path):
raise TypeError("Both arguments must be of type Path")
if not a.is_file() or not b.is_file():
raise FileNotFoundError("Both arguments must be files")
+ if not isinstance(retries, int) or retries < 0:
+ raise ValueError(f"retries must be a non-negative int, got {retries!r}")
if browser is None:
browser = get_browser("firefox")
- diff, (image_a, image_b) = html_render_diff(a, b, browser=browser)
- result = diff.getbbox() is None
- if diff_output is not None and not result:
+
+ for attempt in range(retries + 1):
+ diff, (image_a, image_b) = html_render_diff(a, b, browser=browser)
+ if diff.getbbox() is None:
+ return True
+ logger.debug(
+ "%s and %s differ on attempt %d of %d", a, b, attempt + 1, retries + 1
+ )
+
+ if diff_output is not None:
diff_output.mkdir(parents=True, exist_ok=True)
image_a.save(diff_output / "a.png")
image_b.save(diff_output / "b.png")
diff.save(diff_output / "diff.png")
- return result
+ return False
def compare_files(a: Path, b: Path, **kwargs) -> bool:
diff --git a/src/htmlcmp/compare_output_cli.py b/src/htmlcmp/compare_output_cli.py
index 5b20d27..159412f 100644
--- a/src/htmlcmp/compare_output_cli.py
+++ b/src/htmlcmp/compare_output_cli.py
@@ -34,11 +34,14 @@ class Config:
class Task:
"""A single file comparison between the reference (A) and monitored (B) tree."""
- def __init__(self, rel: Path, a: Path, b: Path, diff_output: Path = None):
+ def __init__(
+ self, rel: Path, a: Path, b: Path, diff_output: Path = None, retries: int = 1
+ ):
self.rel = rel
self.a = a
self.b = b
self.diff_output = diff_output
+ self.retries = retries
class Failure:
@@ -54,7 +57,7 @@ def __init__(self, rel: Path, kind: str, reason: str):
def collect_tasks(
- a: Path, b: Path, root: Path = None, diff_output: Path = None
+ a: Path, b: Path, root: Path = None, diff_output: Path = None, retries: int = 1
) -> tuple[list[Task], list[Failure]]:
"""Walk both trees once and return (comparable tasks, structural failures).
@@ -91,6 +94,7 @@ def collect_tasks(
a / name,
b / name,
None if diff_output is None else diff_output / name,
+ retries=retries,
)
)
elif name in left_files:
@@ -107,6 +111,7 @@ def collect_tasks(
b / name,
root=root,
diff_output=None if diff_output is None else diff_output / name,
+ retries=retries,
)
tasks.extend(sub_tasks)
failures.extend(sub_failures)
@@ -126,7 +131,13 @@ def collect_tasks(
def run_task(task: Task) -> bool:
logger.debug("Comparing %s", task.rel)
browser = getattr(Config.thread_local, "browser", None)
- return compare_files(task.a, task.b, browser=browser, diff_output=task.diff_output)
+ return compare_files(
+ task.a,
+ task.b,
+ browser=browser,
+ diff_output=task.diff_output,
+ retries=task.retries,
+ )
def make_executor(max_workers: int, driver: str | None) -> ThreadPoolExecutor:
@@ -248,6 +259,7 @@ def run(
driver: str | None,
max_workers: int,
diff_output: Path | None,
+ retries: int,
console: Console,
live: bool,
github: bool,
@@ -256,7 +268,7 @@ def run(
f"[bold]Comparing[/bold] {escape(str(a))} [dim]→[/dim] {escape(str(b))}"
)
- tasks, failures = collect_tasks(a, b, diff_output=diff_output)
+ tasks, failures = collect_tasks(a, b, diff_output=diff_output, retries=retries)
logger.info(
"Collected %d comparable file(s), %d structural difference(s)",
len(tasks),
@@ -326,6 +338,13 @@ def main():
default=0,
help="Increase verbosity (-v, -vv, -vvv)",
)
+ parser.add_argument(
+ "--retries",
+ type=int,
+ default=1,
+ help="Re-render a mismatch this many times before reporting it "
+ "(default: 1; 0 reports the first render)",
+ )
parser.add_argument("--log-file", type=Path, help="Path to log file")
parser.add_argument(
"--log-file-verbosity", type=int, help="Log file verbosity level"
@@ -351,6 +370,7 @@ def main():
driver=driver,
max_workers=args.max_workers,
diff_output=args.diff_output,
+ retries=args.retries,
console=console,
live=live,
github=github,
diff --git a/src/htmlcmp/html_render_diff.py b/src/htmlcmp/html_render_diff.py
index 13f0f51..cfac7a9 100755
--- a/src/htmlcmp/html_render_diff.py
+++ b/src/htmlcmp/html_render_diff.py
@@ -9,6 +9,7 @@
from PIL import Image, ImageChops
from selenium import webdriver
+from selenium.common.exceptions import WebDriverException
from selenium.webdriver.common.by import By
from selenium.webdriver.support import expected_conditions
from selenium.webdriver.support.ui import WebDriverWait
@@ -25,7 +26,9 @@ def to_url(path: str | Path) -> str:
return path
-def screenshot(browser: webdriver.Remote, url: str) -> Image.Image:
+def screenshot(
+ browser: webdriver.Remote, url: str, settling_time: float = 0
+) -> Image.Image:
if not isinstance(url, str):
raise TypeError(f"Expected str, got {type(url)}")
if not isinstance(browser, webdriver.Remote):
@@ -35,11 +38,6 @@ def screenshot(browser: webdriver.Remote, url: str) -> Image.Image:
target_find_by = By.TAG_NAME
target = "body"
- loaded_page_settling_time = 0
-
- # TODO for pdf2htmlex the second screenshot sometimes fades in from white... not sure why, but a sleep solves it
- if "poppler" in url:
- loaded_page_settling_time = 0.3
web_driver_wait = WebDriverWait(browser, 10)
web_driver_wait.until(
@@ -49,12 +47,45 @@ def screenshot(browser: webdriver.Remote, url: str) -> Image.Image:
lambda driver: driver.execute_script("return document.readyState") == "complete"
)
- time.sleep(loaded_page_settling_time)
+ settle(browser, settling_time)
png = browser.get_screenshot_as_png()
return Image.open(io.BytesIO(png))
+#: Waits for the fonts and then for two frames, and answers when both are done.
+#: `readyState` does not cover a web font: the load event fires while the face
+#: is still arriving, and the text is laid out again once it lands. Two frames
+#: then say a paint has happened rather than merely been asked for.
+_SETTLE = """
+const done = arguments[arguments.length - 1];
+const frames = () =>
+ requestAnimationFrame(() => requestAnimationFrame(() => done(true)));
+(document.fonts ? document.fonts.ready : Promise.resolve()).then(frames, frames);
+"""
+
+
+def settle(browser: webdriver.Remote, settling_time: float = 0) -> None:
+ """Waits until the page has finished painting what it loaded.
+
+ A screenshot taken before that catches the page mid-render, which is what
+ makes an otherwise identical pair compare as different from one run to the
+ next.
+ """
+ if not isinstance(browser, webdriver.Remote):
+ raise TypeError(f"Expected webdriver.Remote, got {type(browser)}")
+
+ browser.set_script_timeout(10)
+ try:
+ browser.execute_async_script(_SETTLE)
+ except WebDriverException:
+ # an old driver without async scripts still gets the sleep below
+ pass
+
+ if settling_time:
+ time.sleep(settling_time)
+
+
def content_bottom(image: Image.Image) -> int:
"""Row just below the last pixel that differs from the page background.
diff --git a/tests/test_compare_html_retries.py b/tests/test_compare_html_retries.py
new file mode 100644
index 0000000..9ce2622
--- /dev/null
+++ b/tests/test_compare_html_retries.py
@@ -0,0 +1,80 @@
+"""A mismatch is rendered again before it is reported.
+
+A browser gives no promise that two runs of the same page paint alike at the
+same instant, so a single mismatching render does not say the two files differ.
+These drive `compare_html` over a stub renderer, because what is under test is
+what it does with a mismatch rather than how a page paints.
+"""
+
+from pathlib import Path
+
+import pytest
+from PIL import Image
+
+from htmlcmp import common
+
+TEST1 = Path(__file__).parent / "test1.html"
+
+
+def images(same: bool):
+ """A `html_render_diff` result that says the pair matches, or does not."""
+ diff = Image.new("RGB", (4, 4), (0, 0, 0) if same else (255, 0, 0))
+ return diff, (Image.new("RGB", (4, 4)), Image.new("RGB", (4, 4)))
+
+
+def renderer(pattern, calls):
+ """Stands in for `html_render_diff`, answering `pattern` in turn."""
+
+ def render(a, b, browser=None):
+ calls.append((a, b))
+ return images(pattern[min(len(calls) - 1, len(pattern) - 1)])
+
+ return render
+
+
+def test_a_mismatch_that_does_not_come_back_is_a_match(monkeypatch):
+ calls = []
+ monkeypatch.setattr(common, "html_render_diff", renderer([False, True], calls))
+
+ assert common.compare_html(TEST1, TEST1, browser=object()) is True
+ assert len(calls) == 2
+
+
+def test_a_mismatch_that_comes_back_is_reported(monkeypatch, tmp_path):
+ calls = []
+ monkeypatch.setattr(common, "html_render_diff", renderer([False], calls))
+
+ assert (
+ common.compare_html(TEST1, TEST1, browser=object(), diff_output=tmp_path)
+ is False
+ )
+ assert len(calls) == 2
+ assert (tmp_path / "a.png").is_file()
+ assert (tmp_path / "b.png").is_file()
+ assert (tmp_path / "diff.png").is_file()
+
+
+def test_a_match_is_rendered_once(monkeypatch):
+ calls = []
+ monkeypatch.setattr(common, "html_render_diff", renderer([True], calls))
+
+ assert common.compare_html(TEST1, TEST1, browser=object()) is True
+ assert len(calls) == 1
+
+
+def test_retries_zero_reports_the_first_render(monkeypatch):
+ calls = []
+ monkeypatch.setattr(common, "html_render_diff", renderer([False, True], calls))
+
+ assert common.compare_html(TEST1, TEST1, browser=object(), retries=0) is False
+ assert len(calls) == 1
+
+
+def test_retries_is_a_count(monkeypatch):
+ calls = []
+ monkeypatch.setattr(common, "html_render_diff", renderer([True], calls))
+
+ with pytest.raises(ValueError):
+ common.compare_html(TEST1, TEST1, browser=object(), retries=-1)
+ with pytest.raises(ValueError):
+ common.compare_html(TEST1, TEST1, browser=object(), retries="two")