Skip to content

Commit 7bf6d00

Browse files
authored
fix: message leak, dead report tests, gem packaging (3.0 readiness backlog) (#234)
Three independent beta2-review backlog items. 1. Vips::Image leaked into failure messages. ComparisonResult#to_h merged the whole meta hash, including the vips-only :diff_mask image object, so a failing assertion printed "diff_mask":"#<Vips::Image:0x...>". #to_h now excludes it; the object stays reachable via #diff_mask. 2. test/integration/report_screenshot_test.rb skipped all five tests unless RECORD_SCREENSHOTS -- the mode that records baselines rather than verifying them. Baselines existed only for macos/cuprite, so on the only CI platform (linux) there was nothing to compare against. Deleted, along with the orphan baselines. The HTML reporter keeps 18 unit tests, and `rake report:sample` already produces a sample report for eyeballing. 3. The gemspec shipped gems.rb, Rakefile and itself while omitting README.md, leaving a dead ../README.md link in the packaged docs. Replaced the deny-list regex with an allow-list: lib/, docs/, README, LICENSE, CHANGELOG.
1 parent 5f3fee6 commit 7bf6d00

9 files changed

Lines changed: 23 additions & 82 deletions

File tree

capybara-screenshot-diff.gemspec

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@ Gem::Specification.new do |spec|
1515
spec.required_ruby_version = ">= 3.2"
1616
spec.license = "MIT"
1717
spec.metadata["allowed_push_host"] = "https://rubygems.org/"
18-
spec.files = `git ls-files -z`.split("\x0").reject do |f|
19-
f.match(%r{(^(\.|tmp|bin|test|spec|features|gemfiles|scripts|foo)/)|(^(\.|Dockerfile|CONTRIBUTING|README))})
20-
end
18+
# Allow-list: everything a consumer needs at runtime plus the shipped docs.
19+
# Build/dev files (gems.rb, Rakefile, the gemspec itself, tests, CI) stay out.
20+
spec.files = `git ls-files -z`.split("\x0")
21+
.grep(%r{\A(lib/|docs/|README\.md\z|LICENSE\.txt\z|CHANGELOG\.md\z)})
2122

2223
spec.bindir = "exe"
2324
spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) }

lib/snap_diff/comparison_result.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ def ratio
6767
meta[:difference_level]
6868
end
6969

70+
# Serializable difference metrics. The raw diff mask image is excluded —
71+
# it is an image object, not a metric, and is reachable via #diff_mask.
7072
def to_h
71-
{area_size: region_area_size, region: coordinates}.merge!(meta)
73+
{area_size: region_area_size, region: coordinates}.merge!(meta.except(:diff_mask))
7274
end
7375

7476
def coordinates
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.

test/integration/report_screenshot_test.rb

Lines changed: 0 additions & 78 deletions
This file was deleted.

test/unit/reporters/default_test.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,22 @@ class Reporters::DefaultTest < ActiveSupport::TestCase
4141
assert_not reporter.heatmap_diff_path.exist?, "heatmap diff should be cleaned"
4242
end
4343

44+
test "failure message reports metrics without leaking image objects" do
45+
driver = SnapDiff::Drivers::VipsDriver.new
46+
comparison = build_comparison_for(driver, "a.png", "b.png")
47+
difference = driver.find_difference_region(comparison)
48+
difference.meta[:difference_level] = 0.42
49+
50+
message = SnapDiff::Reporters::Default.new(difference).generate
51+
metrics = message.lines.first
52+
53+
assert_includes metrics, "area_size"
54+
assert_includes metrics, "region"
55+
assert_includes metrics, "difference_level"
56+
assert_not_includes metrics, "Vips::Image"
57+
assert_not_includes metrics, "0x"
58+
end
59+
4460
private
4561

4662
def build_comparison_for(driver, *images)

0 commit comments

Comments
 (0)