From 27d0a854659880e86754410a8131c2a514087834 Mon Sep 17 00:00:00 2001 From: Diego Algorta Date: Mon, 12 Oct 2026 16:38:17 -0300 Subject: [PATCH 1/2] fix: a size change labels its images like every other failure The "Dimensions have changed" message still printed its two images as bare absolute paths, in the format #264 replaced for every other failure: no labels, so only the order said which file was the baseline. It now reuses #264's artifact block, limited to baseline and actual. Images of different sizes are never annotated, so any .diff or .heatmap.diff files on disk under that name are leftovers from an earlier same-size failure and must not be listed. The headline is unchanged; the README quotes it. Rendering these paths routes the size-change message through display_path for the first time, so display_path now keeps a path it can't relate to the root (another Windows drive) as given instead of raising, as ScreenshotAssertion's call-site rendering already does. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +++ lib/snap_diff/reporters/default.rb | 17 +++++++++++----- test/unit/reporters/default_test.rb | 31 +++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a174da2..8e511d1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -413,6 +413,9 @@ One removal 2.0 cannot warn about, written down instead: the legacy `LOADED_DRIV `skip_stack_frames: 1` (see docs/configuration.md) - **RSpec prints a failed screenshot once.** The adapter re-raised it inside a `rescue`, so RSpec repeated the whole failure under `Caused by:` +- **A size change labels its images like every other failure.** `Dimensions have + changed` now lists `baseline:` and `actual:`, relative to the project root, instead of + two bare absolute paths, and never names annotations left over from an earlier run ### Known limitations - **One process per worker** (`parallel_tests`, RSpec, CI sharding) still writes one diff --git a/lib/snap_diff/reporters/default.rb b/lib/snap_diff/reporters/default.rb index dbe1988d..c95f8759 100644 --- a/lib/snap_diff/reporters/default.rb +++ b/lib/snap_diff/reporters/default.rb @@ -51,8 +51,10 @@ def annotate_and_save_images annotation_service.annotate_and_save_images end + # Images of different sizes are never annotated, so only the two inputs + # are listed: annotations on disk are leftovers from an earlier failure. def build_error_for_different_dimensions - "Dimensions have changed: #{dimension_change}\n#{base_image_path.to_path}\n#{image_path.to_path}" + ["Dimensions have changed: #{dimension_change}", *artifact_lines(INPUT_ARTIFACTS)].join(NEW_LINE) end # "WxH => WxH", baseline first. Public so Reporters::HTML says it the @@ -93,6 +95,8 @@ def self.percent(fraction) "heatmap" => :heatmap_diff_path }.freeze + INPUT_ARTIFACTS = ARTIFACT_LABELS.slice("baseline", "actual").freeze + def build_error_message [headline, *metric_lines, *artifact_lines].join(NEW_LINE) end @@ -138,8 +142,8 @@ def thresholds # Only what is on disk gets a line: the heatmap exists solely for # drivers that produce a diff mask, and a comparison can be reported # before either input has been written out. - def artifact_lines - present = ARTIFACT_LABELS.filter_map do |label, path_method| + def artifact_lines(labels = ARTIFACT_LABELS) + present = labels.filter_map do |label, path_method| path = send(path_method) [label, path] if path.exist? end @@ -150,11 +154,14 @@ def artifact_lines # Relative to the configured root: shorter to read, and still # click-through-able in terminals that resolve paths against the - # working directory. Anything outside the root stays absolute, because - # a "../../.." path is neither. + # working directory. Anything outside the root stays as given, because + # a "../../.." path is neither; so does a path that can't be related to + # the root at all (another drive), rather than failing the message. def display_path(path) relative = path.expand_path.relative_path_from(SnapDiff.config.root).to_path relative.start_with?("..") ? path.to_path : relative + rescue ArgumentError + path.to_path end def percent(fraction) diff --git a/test/unit/reporters/default_test.rb b/test/unit/reporters/default_test.rb index fd33ae63..84f0038e 100644 --- a/test/unit/reporters/default_test.rb +++ b/test/unit/reporters/default_test.rb @@ -161,6 +161,37 @@ class DefaultReporterTest < ActiveSupport::TestCase end end + # A size change draws no annotations, so its message lists only the two + # inputs, even when an earlier same-size failure left annotations on disk. + test "a size change labels its two inputs like any other failure" do + base = @_tmpdir / "header.base.png" + new = @_tmpdir / "header.png" + FileUtils.cp(TEST_IMAGES_DIR / "a.png", base) + FileUtils.cp(TEST_IMAGES_DIR / "a_cropped.png", new) + %w[header.diff.png header.base.diff.png header.heatmap.diff.png].each { |stale| FileUtils.touch(@_tmpdir / stale) } + + with_config_root(@_tmpdir) do + assert_equal <<~MESSAGE.chomp, SnapDiff::Comparison.new(new, base).processed.error_message + Dimensions have changed: 80x80 => 80x60 + baseline: header.base.png + actual: header.png + MESSAGE + end + end + + # A path that can't be related to the root (another Windows drive; a + # relative root gives Ruby's same "different prefix" error anywhere) must + # not replace the screenshot failure with an ArgumentError. + test "artifact paths that can't be related to the root are printed as given" do + driver = SnapDiff::Drivers::VipsDriver.new + comparison = build_comparison_for(driver, "a.png", "b.png") + reporter = SnapDiff::Reporters::Default.new(driver.find_difference_region(comparison)) + + SnapDiff.config.stub(:root, Pathname("spec/dummy")) do + assert_includes reporter.generate.squeeze(" "), "heatmap: #{reporter.heatmap_diff_path}" + end + end + private def with_config_root(root) From 730444f1eb11386c4d5663252d08865f51530a24 Mon Sep 17 00:00:00 2001 From: Diego Algorta Date: Mon, 12 Oct 2026 21:43:45 -0300 Subject: [PATCH 2/2] fix: drop the display_path guard nothing on a supported setup can reach config.root= always expands the root and artifact paths are expanded before relative_path_from, so the "different prefix" ArgumentError the rescue swallowed needs either a root the config API cannot produce (the removed test stubbed one) or artifacts on another Windows drive than the root, which nothing here can test and no user has reported. If it ever raises, a misconfiguration should surface, not be hidden. display_path is back to exactly what master has; size changes now share it with every other failure, guard-free. Co-Authored-By: Claude Opus 5.5 --- lib/snap_diff/reporters/default.rb | 7 ++----- test/unit/reporters/default_test.rb | 13 ------------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/lib/snap_diff/reporters/default.rb b/lib/snap_diff/reporters/default.rb index c95f8759..021a9fa8 100644 --- a/lib/snap_diff/reporters/default.rb +++ b/lib/snap_diff/reporters/default.rb @@ -154,14 +154,11 @@ def artifact_lines(labels = ARTIFACT_LABELS) # Relative to the configured root: shorter to read, and still # click-through-able in terminals that resolve paths against the - # working directory. Anything outside the root stays as given, because - # a "../../.." path is neither; so does a path that can't be related to - # the root at all (another drive), rather than failing the message. + # working directory. Anything outside the root stays absolute, because + # a "../../.." path is neither. def display_path(path) relative = path.expand_path.relative_path_from(SnapDiff.config.root).to_path relative.start_with?("..") ? path.to_path : relative - rescue ArgumentError - path.to_path end def percent(fraction) diff --git a/test/unit/reporters/default_test.rb b/test/unit/reporters/default_test.rb index 84f0038e..fdace459 100644 --- a/test/unit/reporters/default_test.rb +++ b/test/unit/reporters/default_test.rb @@ -179,19 +179,6 @@ class DefaultReporterTest < ActiveSupport::TestCase end end - # A path that can't be related to the root (another Windows drive; a - # relative root gives Ruby's same "different prefix" error anywhere) must - # not replace the screenshot failure with an ArgumentError. - test "artifact paths that can't be related to the root are printed as given" do - driver = SnapDiff::Drivers::VipsDriver.new - comparison = build_comparison_for(driver, "a.png", "b.png") - reporter = SnapDiff::Reporters::Default.new(driver.find_difference_region(comparison)) - - SnapDiff.config.stub(:root, Pathname("spec/dummy")) do - assert_includes reporter.generate.squeeze(" "), "heatmap: #{reporter.heatmap_diff_path}" - end - end - private def with_config_root(root)