test: close measured coverage gaps + actually run the 3.0 deletion

[pftg]

Closes the gaps a measured coverage+mutation audit found. The audit ran on 9822cc6, before #236; every item was re-verified against current master first, and two of the three "blockers" turned out to be already closed by #236 — reported below as negative results.

No lib/ changes, no version/CHANGELOG changes. Test-only.

Already closed by #236 (negative results)

Blocker A — lib/snap_diff-capybara.rb has zero coverage. No longer true. #236 added "snap_diff-capybara" to SupportLoadProbeTest::CANONICAL_ENTRY_POINTS, CANONICAL_ADVERTISED_CONSTANTS and the dual-install-guard probe, so the full canonical SnapDiff surface is asserted for it today. What was still missing is its post-3.0 shape — covered here by the deletion test (see below), which requires it from the deleted tree with the repointing edit applied.

Blocker C — config default values and require-time freezing pinned only through legacy holders. No longer true. ConfigDefaultTimingTest is already canonical: it lives in test/unit/, reads through SnapDiff.config only, and pins the 27-setting default table plus both freezings. The Capybara::Screenshot.* half was split out to test/legacy/legacy_config_default_timing_test.rb.

Verified by mutation rather than by reading it — both freezings really are load-bearing:

Mutation applied to lib/snap_diff/config.rb	Result
fail_if_new made a lazy memoized reader	4 failures — fail_if_new: expected true, got false (probe B) and expected false, got true (probe A)
root made a lazy memoized reader	4 failures — root: expected /fake-rails-root-at-require, got /fake-rails-root-after-require (probe C) and expected <launch pwd>, got <tmpdir> (probe A)

Blocker B — nothing actually RAN the deletion

test/unit/deletion_3_0_test.rb. Copies lib/ to a tmpdir, deletes the 3.0 set (lib/capybara*, lib/capybara-screenshot-diff.rb, lib/capybara_screenshot_diff.rb, lib/snap_diff/legacy_shims.rb, lib/snap_diff/deprecation.rb), applies the two known edits, and requires every canonical entry point in a fresh subprocess — surface methods and advertised constants both.

The deletion set and the edits are exact-matched against the real files, so a reworded require line goes red here rather than silently not applying.

Gate-line design

An earlier lane's "green" run was later shown to have measured the intact tree, because BUNDLE_GEMFILE pointed at a gemspec that unshifts its own lib ahead of -Ilib. So before a single surface assertion, every probe hard-asserts that the deletion is in effect:

• SnapDiff.respond_to?(:start) is false (it is defined in legacy_shims.rb)
• defined?(CapybaraScreenshotDiff) is nil
• no legacy_shims / deprecation in $LOADED_FEATURES
• no lib/capybara* path in $LOADED_FEATURES — this is also Blocker A's post-3.0 assertion
• every loaded snap_diff file starts with the tmpdir path — the BUNDLE_GEMFILE trap itself

It is an abort, not a comment. And it is tested: "the gate line rejects an intact tree" runs the same probe against an untouched copy of lib/, where every surface assertion would pass, and asserts the gate rejects it (SnapDiff.start is still defined).

The isolation is real, and measured

While building this I confirmed the trap is not hypothetical, and that the obvious defense is not sufficient. Scrubbing RUBYOPT/BUNDLE_GEMFILE alone does not work if the cwd is still inside the project: RubyGems auto-discovers gems.rb, puts -rbundler/setup back into RUBYOPT, and the gemspec unshifts the real lib/ ahead of the -I dir.

cwd=<project root>   RUBYOPT="-r.../bundler/setup"   worktree lib on $LOAD_PATH: yes
cwd=<tmpdir>         RUBYOPT=nil                     worktree lib on $LOAD_PATH: no

So the probe does both — scrubs the env and chdirs to the tmpdir — and the comment on #probe says which half is load-bearing. The gate line is what notices if either stops working.

Red→green evidence

Reintroducing one core→legacy edge (require "capybara/screenshot/diff/os" in lib/snap_diff/region.rb, a leaf the whole core loads):

require "snap_diff/dsl" -> .../lib/snap_diff/region.rb:3:in 'Kernel#require':
  cannot load such file -- capybara/screenshot/diff/os (LoadError)
require "snap_diff/integrations/minitest" -> ... (LoadError)
require "snap_diff/integrations/rspec"    -> ... (LoadError)
require "snap_diff-capybara"              -> ... (LoadError)

Both deletion tests red. The snap_diff-capybara trace also shows snap_diff-capybara.rb:8 loading snap_diff/integrations/minitest from the deleted tree — i.e. the repointing edit applied and that entry point is genuinely exercised post-deletion. Reverted; green.

Gate globs — checked, deliberately not widened

lib/snap_diff-capybara.rb does fall outside both gates' globs, as the audit says. I did not widen either, and this is a call rather than an omission:

• Reverse gate (snap_diff/**/*.rb, survives 3.0): widening it goes red today — the file legitimately requires capybara_screenshot_diff/minitest for the whole 2.x line — so it would need an ALLOWED entry. That allowlist is deliberately empty and its own comment says an entry needs an ADR update, never a red build turned green. Not worth spending for a claim the deletion test now makes executably.
• Alias-only gate (test/legacy/, deleted in 3.0): its subject is the v1 compatibility surface. snap_diff-capybara.rb is the canonical gem's Bundler entry, and the guard would disappear in 3.0 anyway.

Happy to reverse either if a reviewer disagrees.

Three mutations that survived the full suite

Each was applied, the new test observed red, then reverted and observed green. git diff --stat lib/ is empty on this branch.

1. Vips tolerance floor (config.rb:155 then-arm, hit count 0) — tolerance: tolerance || ((driver == :vips) ? 0.001 : nil) → tolerance: tolerance

SnapDiffConfigTest#test_default_options_floors_tolerance_at_0.001_for_vips_and_only_for_vips
Expected: 0.001
  Actual: nil

2. SnapDiff.compare drops the defaults merge — config.default_options.merge(options) → options

SnapDiffTest#test_.compare_with_no_options_still_carries_the_configured_defaults
Expected: 0.0123
  Actual: nil

The existing test only pinned that an explicit option round-trips, so callers who configure once and then call .compare with no options were silently losing every configured default.

3. Reporters::Default#generate skips clean_tmp_files on the equal path

DefaultReporterTest#test_#generate_removes_the_previous_run's_diff_artifacts_when_the_images_are_equal
diff should be cleaned on the equal path

The existing test called clean_tmp_files directly, so #generate's own call had no coverage. The equal comparison is built with the same artifact paths as the differing one, so what gets cleaned is exactly what got written.

Two thin branches

Viewport.prepare!'s selenium arm — never taken (viewport_test stubbed selenium? => false only), so the whole session.driver.browser.manage.window.size chain, the only reason the message is ever useful, was unexecuted. Mutation — collapse the ternary to current_size = "unknown":

Expected "...Actual: unknown..." to include "Actual: (width: 1024, height: 768)".

screenshot_area's two path-segment flags — only ever exercised together (both true in system_test_case.rb and the rspec fixtures, both nil everywhere else), so either if could be swapped for the other flag unnoticed. All four combinations, with Os.name/Capybara.current_driver stubbed so the assertion names the expected path literally. Mutation — parts << Os.name if add_os_path → if add_driver_path:

screenshot_area with add_os_path=true, add_driver_path=nil.
Expected: "doc/screenshots/fake_os"
  Actual: "doc/screenshots"

Numbers

Gate	Before	After
rake test:unit	547 / 0	555 / 0
rake test	575 / 0 / 1	583 / 0 / 1
rake test:canonical	457 / 0 / 1	465 / 0 / 1

standardrb: 157 files, no offenses. Reverse gate and alias-only gate both green (they run inside the above).

🤖 Generated with Claude Code

Summary by Sourcery

Close measured coverage and mutation gaps while making the 3.0 legacy-surface deletion an executable compatibility test.

Bug Fixes:

• Add regression coverage for configured defaults, Vips tolerance handling, screenshot artifact cleanup, Selenium viewport reporting, and independent screenshot path segments.

Enhancements:

• Execute the planned 3.0 deletion in isolated subprocesses and verify all canonical entry points retain their methods and advertised constants after the legacy surface is removed.

Tests:

• Expand unit coverage to exercise previously untested branches and mutation-detected behavior, including the Selenium viewport path, all screenshot-area flag combinations, default option propagation, Vips tolerance floors, and equal-comparison cleanup.
• Add load-isolation and gate validation to ensure deletion tests cannot accidentally measure the intact source tree.

Summary by CodeRabbit

• Tests
  • Expanded coverage for accurate browser viewport mismatch reporting.
  • Added validation for legacy API and entry-point removal.
  • Verified cleanup of generated comparison artifacts for identical images.
  • Added coverage for screenshot areas, driver-specific tolerance defaults, and configured comparison options.

[sourcery-ai]

Sorry @pftg, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[sourcery-ai]

Reviewer's Guide

Adds targeted tests to close coverage and mutation gaps around 3.0 legacy deletion, configuration defaults, reporters, viewport sizing, and screenshot path composition, without modifying lib/; also introduces a subprocess-based gate to ensure the 3.0 deletion is actually exercised.

File-Level Changes

Change	Details	Files
Exercise Viewport.prepare! selenium branch so window-size mismatch errors report the real browser size.	Add selenium-specific test that stubs session/driver/browser/manage/window chain Assert both expected and actual window sizes appear in the error message	test/unit/capture/viewport_test.rb
Implement an executable 3.0 deletion probe that copies lib/ to a tmpdir, applies the planned deletions/edits, and requires all canonical entry points under a guarded subprocess.	Create Deletion30Test that materializes the 3.0 git rm set in a temporary lib tree Apply exact-matched edits to snap_diff.rb and snap_diff-capybara.rb in the temp tree Run subprocess probes for each canonical entry point and advertised constant set, guarded by a gate script verifying the deleted tree is actually in use Add a test that the gate script rejects an intact lib tree, proving the gate is effective	test/unit/deletion_3_0_test.rb test/unit/support_load_probe_test.rb (indirectly referenced)
Cover DefaultReporter#generate’s cleanup behavior so equal-image comparisons remove prior diff artifacts.	Add test that generates artifacts via a differing comparison and then reuses the same paths for an equal comparison Assert that annotated image, base image, and heatmap diff files are removed on the equal path	test/unit/reporters/default_test.rb
Strengthen SnapDiff configuration tests around screenshot path composition and vips tolerance defaults.	Add table-driven test for screenshot_area to verify OS and driver path segments are appended independently for all flag combinations Add test that Config#default_options floors tolerance to 0.001 only for vips when no explicit tolerance is set, and that explicit tolerance overrides the floor	test/unit/snap_diff_config_test.rb
Ensure SnapDiff.compare merges configured defaults when options are omitted.	Add test that changing SnapDiff.config.tolerance affects the driver_options in a compare call with no explicit options	test/unit/snap_diff_test.rb

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7ea80775-cba1-4186-982d-eaa8f7de629a

📥 Commits

Reviewing files that changed from the base of the PR and between bbf724f and f78da23.

📒 Files selected for processing (5)

• test/unit/capture/viewport_test.rb
• test/unit/deletion_3_0_test.rb
• test/unit/reporters/default_test.rb
• test/unit/snap_diff_config_test.rb
• test/unit/snap_diff_test.rb

---

📝 Walkthrough

Walkthrough

The pull request adds regression tests for viewport mismatch messages, comparison configuration, equal-image artifact cleanup, and removal of the declared 3.0 legacy surface.

Changes

Comparison and reporting regressions

Layer / File(s)	Summary
Comparison configuration propagation test/unit/snap_diff_config_test.rb, test/unit/snap_diff_test.rb	Tests cover screenshot path combinations, the VIPS tolerance floor, explicit tolerance precedence, and propagation of configured options to SnapDiff.compare.
Equal-image artifact cleanup test/unit/reporters/default_test.rb	The reporter test verifies that equal comparisons return nil and remove artifacts created by a previous differing comparison.

Viewport mismatch diagnostics

Layer / File(s)	Summary
Window-size mismatch reporting test/unit/capture/viewport_test.rb	Tests verify reporting of unknown actual sizes without Selenium and browser window dimensions with Selenium.

3.0 deletion compatibility

Layer / File(s)	Summary
Temporary deletion tree setup test/unit/deletion_3_0_test.rb	The test defines the legacy deletion surface, required source edits, and helpers that create and transform an isolated library tree.
Isolated entry-point validation test/unit/deletion_3_0_test.rb	Subprocess probes verify entry points, methods, constants, version data, load sources, and rejection of an untouched tree.

Estimated code review effort: 4 (Complex) | ~45 minutes

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/v3ready-coverage-gaps

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Screenshot diffs detected

Artifact	Link
HTML report (inline)	N/A
Full report with images	N/A
All artifacts	Browse all