fix: the HTML report lists every failed screenshot

CHANGELOG.md

@@ -396,6 +396,16 @@ One removal 2.0 cannot warn about, written down instead: the legacy `LOADED_DRIV
   After each capture it used to click the input it had blurred. The click moved the
   pointer onto it, so the input rendered hovered in later screenshots (Firefox 157
   draws that darker), and it fired the page's click handlers. It now calls `focus()`
+- **A screenshot that changed size is in the HTML report.** It was counted in the
+  summary line but left out of `snap_diff_report.html` (and a run whose only failures
+  were size changes wrote no report at all): the null comparison result is frozen, and
+  reading its `area_size` raised `FrozenError`, which the reporter swallowed unless
+  `DEBUG` was set. The entry shows the baseline, the new capture and both sizes, and
+  says why it has no heatmap or annotations
+- **An error while building a report entry no longer hides the screenshot.** The HTML
+  reporter warns on stderr without `DEBUG` (`[snap_diff] HTML report: could not render
+  'name' (Error: message)`) and keeps the screenshot in the report as "could not render",
+  counted as failed, instead of dropping it and counting it as passed
 
 ### Known limitations
 - **One process per worker** (`parallel_tests`, RSpec, CI sharding) still writes one

README.md

@@ -332,8 +332,9 @@ You do not need a flag to keep the diff images — a failing run leaves `.diff.p
 deletes them (`SnapManager#cleanup!` is this repository's own test-harness call, not
 something your suite runs).
 
-`DEBUG=1` does one thing: it makes the HTML reporter print why it skipped an assertion
-instead of failing quietly — useful when `snap_diff_report.html` is missing entries.
+Nor do you need a flag to see report problems: if the HTML reporter cannot render a
+screenshot, it prints `[snap_diff] HTML report: could not render '<name>' (<error>)` and
+still lists the screenshot in `snap_diff_report.html`, marked "could not render".
 </details>
 
 ## Installation

lib/snap_diff/comparison_result.rb

@@ -59,8 +59,9 @@ def blank?
       region.nil? || region_area_size.zero?
     end
 
+    # Not memoized: build_null freezes its result, and Region#size is cheap.
     def region_area_size
-      @region_area_size ||= region&.size || 0
+      region&.size || 0
     end
 
     def ratio

lib/snap_diff/reporters/default.rb

@@ -52,11 +52,15 @@ def annotate_and_save_images
       end
 
       def build_error_for_different_dimensions
-        change_msg = [comparison.base_image, comparison.new_image]
+        "Dimensions have changed: #{dimension_change}\n#{base_image_path.to_path}\n#{image_path.to_path}"
+      end
+
+      # "WxH => WxH", baseline first. Public so Reporters::HTML says it the
+      # same way.
+      def dimension_change
+        [comparison.base_image, comparison.new_image]
           .map { |image| driver.dimension(image).join("x") }
           .join(" => ")
-
-        "Dimensions have changed: #{change_msg}\n#{base_image_path.to_path}\n#{image_path.to_path}"
       end
 
       NEW_LINE = "\n"

lib/snap_diff/reporters/html.rb

@@ -31,15 +31,22 @@ def record(assertions)
         total = 0
 
         assertions.each do |assertion|
+          counted = false
           compare = assertion.compare
           next unless compare
 
           total += 1
+          counted = true
           next unless compare.difference&.different?
 
           failures << failure_entry_for(assertion.name, compare)
         rescue => e
-          warn "[snap_diff] Reporter skipped '#{assertion.name}': #{e.message}" if ENV["DEBUG"]
+          # A report bug must not hide a screenshot: warn, and keep it listed
+          # (counted as failed: the report can't vouch for it).
+          error = "#{e.class}: #{e.message}"
+          warn "[snap_diff] HTML report: could not render '#{assertion.name}' (#{error})"
+          total += 1 unless counted
+          failures << {name: assertion.name, error: error}
         end
 
         @mutex.synchronize do
@@ -115,18 +122,31 @@ def self.default_output_path
 
       # Build a report entry from a comparison, omitting unavailable images and metrics.
       def failure_entry_for(name, compare)
-        difference = compare.difference
+        details = compare.dimensions_changed? ? dimension_change_details(compare) : difference_details(compare)
         {
           name: name,
           original: resolve_image(compare.base_image_path),
-          new: resolve_image(compare.image_path),
+          new: resolve_image(compare.image_path)
+        }.merge(details).compact
+      end
+
+      # Images of different sizes get no annotated or heatmap images, and
+      # no region metrics. Any annotated images on disk for this name are
+      # left over from an earlier same-size failure, so they are not linked.
+      def dimension_change_details(compare)
+        {dimensions_changed: compare.reporter.dimension_change}
+      end
+
+      def difference_details(compare)
+        difference = compare.difference
+        {
           base_diff: resolve_image(compare.reporter.annotated_base_image_path),
           diff: resolve_image(compare.reporter.annotated_image_path),
           heatmap: resolve_image(compare.reporter.heatmap_diff_path),
           diff_level: difference.ratio && (difference.ratio * 100).round(2),
           area_size: difference.region_area_size,
           max_color_distance: difference.meta[:max_color_distance]&.round(1)
-        }.compact
+        }
       end
 
       # Contributed annotations (AI triage, ...), attached at RENDER

lib/snap_diff/reporters/templates/report.html.erb

@@ -129,6 +129,9 @@
     .img-box img { display: block; object-fit: contain; transform-origin: 0 0; }
     /* At 100% zoom, image fills the box */
     .img-box[data-zoom="100"] img { width: 100%; height: 100%; }
+    .img-empty { height: 100%; display: flex; align-items: center; justify-content: center; padding: 1rem; font-size: .75rem; line-height: 1.5; color: var(--text2); }
+    .img-empty p { max-width: 42em; margin: 0; }
+    .img-empty a { color: var(--accent); }
 
     /* Single image */
     .view-single { width: 100%; height: 100%; display: flex; flex-direction: column; }
@@ -266,12 +269,13 @@
       btn.dataset.idx = idx;
       /* Thumbnail: annotated mode → heatmap; normal → new image (no annotation) */
       var thumb = state.annotated ? (item.heatmap || item.new || item.original || '') : (item.new || item.original || '');
-      var hasDiff = item.diff || item.heatmap;
-      var badgeText = hasDiff
-        ? (item.diff_level != null ? item.diff_level + '%' : 'changed')
+      var hasDiff = item.diff || item.heatmap || item.dimensions_changed || item.error;
+      var badgeText = item.error ? 'error'
+        : item.dimensions_changed ? 'resized'
+        : hasDiff ? (item.diff_level != null ? item.diff_level + '%' : 'changed')
         : 'pass';
       btn.innerHTML =
-        '<div class="thumb-img-wrap"><img src="' + esc(thumb) + '" alt="" loading="lazy"/></div>' +
+        '<div class="thumb-img-wrap">' + (thumb ? '<img src="' + esc(thumb) + '" alt="" loading="lazy"/>' : '') + '</div>' +
         '<div class="thumb-footer">' +
           '<div class="thumb-name">' + esc(item.name) + '</div>' +
           (item.annotations || []).map(function(note) {
@@ -303,6 +307,19 @@
   function selectNext() { var p = filtered.indexOf(state.current); if (p < filtered.length - 1) selectItem(filtered[p + 1]); }
   function selectPrev() { var p = filtered.indexOf(state.current); if (p > 0) selectItem(filtered[p - 1]); }
 
+  /* A failure has no heatmap when the images differ in size, or when the
+     image driver produced no difference mask. Say which, and what to do. */
+  function noHeatmapReason(item) {
+    if (item.dimensions_changed) {
+      return 'No heatmap: the screenshot changed size (' + item.dimensions_changed + '). ' +
+        'Heatmaps and annotations compare pixel by pixel, so they need both images at the same size; ' +
+        'compare Baseline and Current instead. If the new size is intended, commit the new screenshot to accept it. ' +
+        'If not, the layout changed, or the browser window size or pixel ratio differs from the run that recorded the baseline.';
+    }
+    return 'No heatmap: the image driver produced no difference mask for this comparison. ' +
+      'The chunky_png driver never produces one; vips does.';
+  }
+
   /* ── Render ───────────────────────────────────────────────────────── */
   function render() {
     var item = DATA[state.current];
@@ -311,8 +328,12 @@
     topTitle.textContent = item.name;
     var pos = filtered.indexOf(state.current);
     navCounter.textContent = (pos + 1) + '/' + filtered.length;
-    var hasDiff = item.diff || item.heatmap;
-    if (hasDiff && item.diff_level != null) {
+    var hasDiff = item.diff || item.heatmap || item.dimensions_changed || item.error;
+    if (item.error) {
+      topBadge.textContent = 'could not render';
+    } else if (item.dimensions_changed) {
+      topBadge.textContent = 'dimensions changed: ' + item.dimensions_changed;
+    } else if (hasDiff && item.diff_level != null) {
       topBadge.textContent = item.diff_level + '% diff';
     } else {
       topBadge.textContent = hasDiff ? 'changed' : 'pass';
@@ -353,11 +374,20 @@
     var newImg  = ann ? (item.diff || item.new || '')          : (item.new || '');
     var heatImg = item.heatmap || '';
 
-    var baseLbl = ann ? 'Annotated Base' : 'Baseline';
-    var newLbl  = ann ? 'Annotated New'  : 'Current';
+    /* Label what is shown: without annotated files the toggle falls back to the plain images */
+    var baseLbl = !ann ? 'Baseline' : item.base_diff ? 'Annotated Base' : 'Baseline (not annotated)';
+    var newLbl  = !ann ? 'Current'  : item.diff      ? 'Annotated New'  : 'Current (not annotated)';
 
     var html = '';
-    switch (state.view) {
+    /* The report failed to build this entry: say so in every view instead of empty images */
+    switch (item.error ? 'error' : state.view) {
+      case 'error':
+        html = '<div class="view-single"><div class="img-box"><div class="img-empty"><p>' +
+          'Could not render this screenshot: ' + esc(item.error) + '. ' +
+          'The test output has its result. Please report this error at ' +
+          '<a href="https://github.com/snap-diff/snap_diff-capybara/issues">github.com/snap-diff/snap_diff-capybara/issues</a>.' +
+          '</p></div></div></div>';
+        break;
       case 'both':
         html = '<div class="view-both">' +
           '<div class="img-panel"><div class="img-label"><span class="dot-base"></span> ' + baseLbl + '</div><div class="img-box"><img src="' + esc(baseImg) + '" alt="Base"/></div></div>' +
@@ -371,7 +401,9 @@
         html = '<div class="view-single"><div class="img-label"><span class="dot-new"></span> ' + newLbl + '</div><div class="img-box"><img src="' + esc(newImg) + '" alt="New"/></div></div>';
         break;
       case 'heatmap':
-        html = '<div class="view-single"><div class="img-label"><span class="dot-heat"></span> Heatmap</div><div class="img-box"><img src="' + esc(heatImg) + '" alt="Heatmap"/></div></div>';
+        html = '<div class="view-single"><div class="img-label"><span class="dot-heat"></span> Heatmap</div><div class="img-box">' +
+          (heatImg ? '<img src="' + esc(heatImg) + '" alt="Heatmap"/>' : '<div class="img-empty"><p>' + esc(noHeatmapReason(item)) + '</p></div>') +
+          '</div></div>';
         break;
     }
     viewInner.innerHTML = html;

scripts/generate_sample_report.rb

@@ -16,7 +16,8 @@
 pairs = [
   {name: "islands-map", base: "a", new: "b"},
   {name: "islands-variant", base: "a", new: "c"},
-  {name: "portrait-layout", base: "portrait", new: "portrait_b"}
+  {name: "portrait-layout", base: "portrait", new: "portrait_b"},
+  {name: "header-resized", base: "a", new: "a_cropped"} # dimensions change: 80x80 => 80x60
 ]
 
 embed = ARGV.include?("--embed") || !!ENV["CI"]

test/unit/diff_test.rb

@@ -70,7 +70,7 @@ class DiffTest < ActiveSupport::TestCase
   test "does not fail when fail_on_difference is false and screenshots differ" do
     SnapDiff.config.stub(:fail_on_difference, false) do
       test_case = SampleMiniTestCase.new(:_test_sample_screenshot_error)
-      test_case.run
+      without_reporters { test_case.run }
       assert_equal 0, test_case.failures.size
     end
   end
@@ -123,7 +123,7 @@ class DiffTest < ActiveSupport::TestCase
   test "aggregates failures on teardown for Minitest" do
     test_case = SampleMiniTestCase.new(:_test_sample_screenshot_error)
 
-    test_case.run
+    without_reporters { test_case.run }
 
     assert_equal 1, test_case.failures.size
     assert_includes test_case.failures.first.message, "expected error message"
@@ -135,7 +135,7 @@ class DiffTest < ActiveSupport::TestCase
 
     expected_message =
       "Screenshot does not match for 'sample_screenshot' expected error message for non minitest"
-    assert_raises(SnapDiff::ExpectationNotMet, expected_message) { test_case.teardown }
+    assert_raises(SnapDiff::ExpectationNotMet, expected_message) { without_reporters { test_case.teardown } }
     assert_empty(SnapDiff.session.assertions)
   end
 
@@ -234,6 +234,18 @@ class ScreenshotFormatTest < ActiveSupport::TestCase
     end
   end
 
+  # The sample cases script a Minitest::Mock comparison for exactly what
+  # verification reads. The registered reporters (HTML) read more, run off
+  # the script and warn about it, so keep them out of these cases.
+  def without_reporters
+    original = SnapDiff::Reporting.mutex.synchronize do
+      SnapDiff::Reporting.reporters.dup.tap { SnapDiff::Reporting.reporters.clear }
+    end
+    yield
+  ensure
+    SnapDiff::Reporting.mutex.synchronize { SnapDiff::Reporting.reporters.replace(original) } if original
+  end
+
   def screenshot_dir
     File.join(SnapDiff.config.screenshot_area, *screenshot_namer.directory_parts)
   end

test/unit/difference_test.rb

@@ -25,4 +25,15 @@ class DifferenceTest < ActiveSupport::TestCase
     assert_includes line, "difference_level="
     assert_not_includes line, "\n"
   end
+
+  # build_null freezes its result, so every reader must work on a frozen
+  # instance.
+  test "a frozen null result answers its readers" do
+    null = SnapDiff::ComparisonResult.build_null(nil, nil, nil, {different_dimensions: true})
+
+    assert_predicate null, :frozen?
+    assert_equal 0, null.region_area_size
+    assert_equal 0, null.to_h[:area_size]
+    assert_includes null.inspect, "area_size=0"
+  end
 end

test/unit/reporters/ai_simple_test.rb

@@ -308,6 +308,7 @@ def heatmap_diff_path = nil
   HtmlCompare = Struct.new(:difference) do
     def base_image_path = Pathname("/nonexistent/base.png")
     def image_path = Pathname("/nonexistent/current.png")
+    def dimensions_changed? = false
     def reporter = HtmlReporterStub.new
   end
   HtmlAssertion = Struct.new(:name, :compare)

test/unit/reporters/html_reporter_test.rb

@@ -69,21 +69,69 @@ class HTMLReporterTest < ActiveSupport::TestCase
     assert_includes html, "3 total"
   end
 
-  test "#record tolerates broken assertions without crashing" do
+  # An entry that fails to build must not vanish, nor sink the report: it is
+  # warned about, kept as "could not render", and counted as failed.
+  test "#record keeps an entry it could not build, says so, and renders the rest" do
     reporter = SnapDiff::Reporters::HTML.new(output_path: @output_path)
+    broken = SnapDiff::ScreenshotAssertion.new("broken").tap { |a| a.compare = Object.new }
 
+    _out, err = capture_io { reporter.record([broken, build_failing_assertion("valid")]) }
+
+    assert_match(/\[snap_diff\] HTML report: could not render 'broken' \(NoMethodError: .*difference/, err)
+    entry = reporter.failures.find { |e| e[:name] == "broken" }
+    assert_match(/\ANoMethodError: .*difference/, entry[:error])
+    assert_equal [2, 0], [reporter.failed, reporter.passed], "an entry the report can't vouch for is not a pass"
+
+    reporter.finalize
+    assert_includes @output_path.read, "valid"
+  end
+
+  test "#record counts an entry that fails before its comparison is read" do
+    reporter = SnapDiff::Reporters::HTML.new(output_path: @output_path)
     broken = SnapDiff::ScreenshotAssertion.new("broken")
-    broken.compare = Object.new # will raise on .difference
+    broken.define_singleton_method(:compare) { raise "no compare" }
 
-    valid = build_failing_assertion("valid")
+    capture_io { reporter.record([broken]) }
 
-    assert_nothing_raised do
-      reporter.record([broken, valid])
-    end
+    assert_equal [1, 1, 0], [reporter.total, reporter.failed, reporter.passed]
+  end
+
+  # A dimension change fails with a frozen null ComparisonResult (no
+  # region, failed_by: {different_dimensions: true}). Recording it used to
+  # raise FrozenError, which #record swallowed, so the screenshot was
+  # counted but silently left out of the report.
+  test "#record reports a dimension change instead of skipping it" do
+    reporter = SnapDiff::Reporters::HTML.new(output_path: @output_path)
+
+    reporter.record([build_dimension_change_assertion("resized")])
+
+    entry = reporter.failures.sole
+    assert_equal "resized", entry[:name]
+    assert_equal "80x80 => 80x60", entry[:dimensions_changed]
+    assert entry[:original], "the baseline must be shown"
+    assert entry[:new], "the new capture must be shown"
 
     reporter.finalize
-    assert @output_path.exist?
-    assert_includes @output_path.read, "valid"
+    assert @output_path.exist?, "a run whose only failure is a dimension change still gets a report"
+    assert_includes @output_path.read, "resized"
+  end
+
+  # Annotated and heatmap images are only drawn for same-size images. Any
+  # on disk for this name are left over from an earlier, same-size failure
+  # and describe a different comparison, so the entry must not link them.
+  test "#record links no annotated images for a dimension change" do
+    reporter = SnapDiff::Reporters::HTML.new(output_path: @output_path)
+    assertion = build_dimension_change_assertion("stale")
+    stale = assertion.compare.reporter
+    [stale.annotated_image_path, stale.annotated_base_image_path, stale.heatmap_diff_path]
+      .each { |path| FileUtils.cp(assertion.compare.image_path, path) }
+
+    reporter.record([assertion])
+
+    entry = reporter.failures.sole
+    assert_nil entry[:diff]
+    assert_nil entry[:base_diff]
+    assert_nil entry[:heatmap]
   end
 
   test "HTML reporter defaults to screenshot root path" do
@@ -226,4 +274,12 @@ def build_failing_assertion(name)
 
     SnapDiff::ScreenshotAssertion.new(name).tap { |a| a.compare = compare }
   end
+
+  # a.png is 80x80, a_cropped.png is 80x60.
+  def build_dimension_change_assertion(name)
+    compare = make_comparison(:a, :a_cropped, destination: "resized_#{name}")
+    compare.processed
+
+    SnapDiff::ScreenshotAssertion.new(name).tap { |a| a.compare = compare }
+  end
 end