diff --git a/lib/snap_diff/reporters/default.rb b/lib/snap_diff/reporters/default.rb index 5dc3e8bf..65d44d8a 100644 --- a/lib/snap_diff/reporters/default.rb +++ b/lib/snap_diff/reporters/default.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true +require "json" + require "snap_diff/annotation_service" +# For SnapDiff.config.root, the base for the relative artifact paths below. +require "snap_diff/config" module SnapDiff module Reporters @@ -57,20 +61,95 @@ def build_error_for_different_dimensions NEW_LINE = "\n" + # The thresholds a comparison is judged against, in the order they read + # best. Only the ones actually set are printed -- see #thresholds. + THRESHOLDS = [ + :tolerance, + :area_size_limit, + :color_distance_limit, + :shift_distance_limit, + :perceptual_threshold + ].freeze + + # The five artifacts a comparison can leave on disk, in the order a + # reader wants them: the two inputs first, then what we drew on them. + ARTIFACT_LABELS = { + "baseline" => :base_image_path, + "actual" => :image_path, + "baseline annotated" => :annotated_base_image_path, + "actual annotated" => :annotated_image_path, + "heatmap" => :heatmap_diff_path + }.freeze + def build_error_message - [ - "(#{difference.to_h.to_json})", - image_path.to_path, - annotated_base_image_path.to_path, - annotated_image_path.to_path, - heatmap_diff_path.to_path - ].join(NEW_LINE) + [headline, *metric_lines, *artifact_lines].join(NEW_LINE) end private attr_reader :annotation_service + # Reads as the tail of "Screenshot does not match for 'name': ". + def headline + width, height = driver.dimension(comparison.base_image) + total_pixels = width * height + area = difference.region_area_size + + "the change spans #{area.round} of #{total_pixels} px " \ + "(#{percent(area.to_f / total_pixels)} of the #{width}x#{height} image)" + end + + def metric_lines + lines = [" changed region: #{difference.coordinates.to_json} (left,top,right,bottom edges)"] + # difference_level is the changed share of the image area -- the + # number `tolerance` is compared against. Only computed when a + # tolerance is set, so only printed then. + if difference.ratio + lines << " difference level: #{difference.ratio} (#{percent(difference.ratio)} of the image area)" + end + max_color_distance = difference.meta[:max_color_distance] + lines << " max color distance: #{max_color_distance}" if max_color_distance&.positive? + max_shift_distance = difference.meta[:max_shift_distance] + lines << " max shift distance: #{max_shift_distance} px" if max_shift_distance&.positive? + lines << " judged against: #{thresholds}" + end + + def thresholds + applied = THRESHOLDS.filter_map do |name| + value = difference.options[name] + "#{name} #{value}" if value + end + + applied.empty? ? "no tolerance thresholds configured (any difference fails)" : applied.join(", ") + end + + # 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| + path = send(path_method) + [label, path] if path.exist? + end + width = present.map { |label, _path| label.length }.max.to_i + + present.map { |label, path| " #{"#{label}:".ljust(width + 1)} #{display_path(path)}" } + end + + # 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. + def display_path(path) + relative = path.expand_path.relative_path_from(SnapDiff.config.root).to_path + relative.start_with?("..") ? path.to_path : relative + end + + def percent(fraction) + value = fraction * 100 + (value.positive? && value < 0.01) ? "<0.01%" : format("%.2f%%", value) + end + def base_image_path comparison.base_image_path end diff --git a/test/unit/dsl_test.rb b/test/unit/dsl_test.rb index 39015f35..03cfe2eb 100644 --- a/test/unit/dsl_test.rb +++ b/test/unit/dsl_test.rb @@ -34,12 +34,18 @@ def after_teardown test "#assert_image_not_changed generates correct error message for image mismatch" do message = assert_image_not_changed(["my_test.rb:42"], "name", make_comparison(:a, :c, destination: "screenshot.png")) value = (RUBY_VERSION >= "2.4") ? 187.4 : 188 + # Paths are relative to SnapDiff.config.root, and only the artifacts + # that exist are listed: chunky_png produces no diff mask, so there is + # no heatmap on disk for this comparison. assert_equal <<~MSG.chomp, message - Screenshot does not match for 'name': ({"area_size":629,"region":[11,3,48,20],"max_color_distance":#{value}}) - #{SnapDiff.config.root}/doc/screenshots/screenshot.png - #{SnapDiff.config.root}/doc/screenshots/screenshot.base.diff.png - #{SnapDiff.config.root}/doc/screenshots/screenshot.diff.png - #{SnapDiff.config.root}/doc/screenshots/screenshot.heatmap.diff.png + Screenshot does not match for 'name': the change spans 629 of 6400 px (9.83% of the 80x80 image) + changed region: [11,3,48,20] (left,top,right,bottom edges) + max color distance: #{value} + judged against: no tolerance thresholds configured (any difference fails) + baseline: doc/screenshots/screenshot.base.png + actual: doc/screenshots/screenshot.png + baseline annotated: doc/screenshots/screenshot.base.diff.png + actual annotated: doc/screenshots/screenshot.diff.png my_test.rb:42 MSG end @@ -52,11 +58,15 @@ def after_teardown ) value = (RUBY_VERSION >= "2.4") ? 5.0 : 5 assert_equal <<~MSG.chomp, message - Screenshot does not match for 'name': ({"area_size":629,"region":[11,3,48,20],"max_color_distance":#{value},"max_shift_distance":15}) - #{SnapDiff.config.root}/doc/screenshots/screenshot.png - #{SnapDiff.config.root}/doc/screenshots/screenshot.base.diff.png - #{SnapDiff.config.root}/doc/screenshots/screenshot.diff.png - #{SnapDiff.config.root}/doc/screenshots/screenshot.heatmap.diff.png + Screenshot does not match for 'name': the change spans 629 of 6400 px (9.83% of the 80x80 image) + changed region: [11,3,48,20] (left,top,right,bottom edges) + max color distance: #{value} + max shift distance: 15 px + judged against: shift_distance_limit 1 + baseline: doc/screenshots/screenshot.base.png + actual: doc/screenshots/screenshot.png + baseline annotated: doc/screenshots/screenshot.base.diff.png + actual annotated: doc/screenshots/screenshot.diff.png my_test.rb:42 MSG end diff --git a/test/unit/reporters/default_test.rb b/test/unit/reporters/default_test.rb index 825242e3..fd33ae63 100644 --- a/test/unit/reporters/default_test.rb +++ b/test/unit/reporters/default_test.rb @@ -74,21 +74,107 @@ class DefaultReporterTest < ActiveSupport::TestCase difference.meta[:difference_level] = 0.42 message = SnapDiff::Reporters::Default.new(difference).generate - metrics = message.lines.first - assert_includes metrics, "area_size" - assert_includes metrics, "region" - assert_includes metrics, "difference_level" - assert_not_includes metrics, "Vips::Image" - assert_not_includes metrics, "0x" + assert_includes message, "changed region" + assert_includes message, "difference level" + assert_not_includes message, "Vips::Image" + # The signature of a leaked Ruby object in a message: `#`. + assert_not_includes message, "#<" + end + + # Every number in the message needs a denominator or a stated unit: an + # `area_size` on its own and a bare four-number `region` were the two + # things readers could not interpret. + test "failure message gives the changed area a denominator and the region its interpretation" do + driver = SnapDiff::Drivers::VipsDriver.new + difference = driver.find_difference_region(build_comparison_for(driver, "a.png", "b.png", tolerance: 0.001)) + + message = SnapDiff::Reporters::Default.new(difference).generate + + assert_includes message, "100 of 6400 px (1.56% of the 80x80 image)" + assert_includes message, "changed region: [20.0,15.0,30.0,25.0] (left,top,right,bottom edges)" + assert_includes message, "difference level: 0.00765625 (0.77% of the image area)" + end + + # VCR's error restates the config it applied; half of "why did this fail?" + # is answered by the threshold the comparison was judged against. + test "failure message states the thresholds that were applied" do + driver = SnapDiff::Drivers::VipsDriver.new + difference = driver.find_difference_region( + build_comparison_for(driver, "a.png", "b.png", tolerance: 0.001, color_distance_limit: 20) + ) + + message = SnapDiff::Reporters::Default.new(difference).generate + + assert_includes message, "judged against: tolerance 0.001, color_distance_limit 20" + end + + test "failure message says so when no threshold was configured" do + driver = SnapDiff::Drivers::VipsDriver.new + difference = driver.find_difference_region(build_comparison_for(driver, "a.png", "b.png")) + + message = SnapDiff::Reporters::Default.new(difference).generate + + assert_includes message, "judged against: no tolerance thresholds configured (any difference fails)" + end + + test "failure message labels every artifact it lists" do + driver = SnapDiff::Drivers::VipsDriver.new + comparison = build_comparison_for(driver, "a.png", "b.png") + FileUtils.cp(TEST_IMAGES_DIR.join("a.png"), comparison.new_image_path) + FileUtils.cp(TEST_IMAGES_DIR.join("b.png"), comparison.base_image_path) + reporter = SnapDiff::Reporters::Default.new(driver.find_difference_region(comparison)) + + message = reporter.generate.squeeze(" ") + + assert_includes message, "baseline: #{comparison.base_image_path}" + assert_includes message, "actual: #{comparison.new_image_path}" + assert_includes message, "baseline annotated: #{reporter.annotated_base_image_path}" + assert_includes message, "actual annotated: #{reporter.annotated_image_path}" + assert_includes message, "heatmap: #{reporter.heatmap_diff_path}" + end + + # Issue #260: the message must never name a file that is not on disk. + test "failure message omits artifacts that were never written" 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)) + + message = reporter.generate.squeeze(" ") + + assert_not_includes message, "baseline: " + assert_not_includes message, "actual: " + assert_includes message, "heatmap: #{reporter.heatmap_diff_path}" + end + + test "artifact paths are printed relative to the configured root, absolute when outside it" 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)) + + with_config_root(@_tmpdir) do + assert_includes reporter.generate.squeeze(" "), "heatmap: a.heatmap.diff.png" + end + + with_config_root(@_tmpdir / "elsewhere") do + assert_includes reporter.generate.squeeze(" "), "heatmap: #{reporter.heatmap_diff_path}" + end end private - def build_comparison_for(driver, *images) + def with_config_root(root) + previous = SnapDiff.config.root + SnapDiff.config.root = root + yield + ensure + SnapDiff.config.root = previous + end + + def build_comparison_for(driver, *images, **options) new_image = driver.from_file(TEST_IMAGES_DIR.join(images.first)) base_image = driver.from_file(TEST_IMAGES_DIR.join(images.last)) - SnapDiff::Comparison::Images.new(new_image, base_image, {}, driver, @_tmpdir / images.first, @_tmpdir / images.last) + SnapDiff::Comparison::Images.new(new_image, base_image, options, driver, @_tmpdir / images.first, @_tmpdir / images.last) end end