From 7a0bbc6f877b95b853ef0da405cc61ce594e0d51 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Sun, 23 Aug 2026 10:28:07 +0200 Subject: [PATCH 01/11] test: reverse gate -- no core file may depend on the v1 trees legacy_tree_is_alias_only_test.rb proves the v1 trees hold no logic. Nothing proved the mirror image, and it was false: 64 core->legacy edges (6 backward requires, AVAILABLE_DRIVERS, ~55 config reads through the legacy view) meant `git rm lib/capybara*` would break the gem. This gate reports every one as file:line and starts with them all allowlisted, so it lands green and the following commits can only shrink it. Red on master with an empty allowlist; mutation-checked by adding a Capybara::Screenshot::Diff reference to region.rb (gate named it). Comments are ignored (history, not dependency); strings are not. --- .../unit/core_tree_has_no_legacy_deps_test.rb | 212 ++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 test/unit/core_tree_has_no_legacy_deps_test.rb diff --git a/test/unit/core_tree_has_no_legacy_deps_test.rb b/test/unit/core_tree_has_no_legacy_deps_test.rb new file mode 100644 index 00000000..6149c8a1 --- /dev/null +++ b/test/unit/core_tree_has_no_legacy_deps_test.rb @@ -0,0 +1,212 @@ +# frozen_string_literal: true + +require "test_helper" + +# The REVERSE of legacy_tree_is_alias_only_test.rb, and the other half of +# what makes 3.0 a `git rm`. +# +# That test proves the v1 trees hold no logic. This one proves the canonical +# core does not reach BACK into them -- which is the half that actually +# breaks the gem if it is wrong: as long as any file under lib/snap_diff/ +# requires a `capybara/...` path or reads a `Capybara::Screenshot.*` / +# `CapybaraScreenshotDiff::*` constant, deleting lib/capybara* leaves a core +# that no longer loads. +# +# Scope: everything the 3.0 deletion KEEPS. Files that are themselves part of +# the deletion set (DELETED_IN_3_0 below) live under lib/snap_diff/ only +# because the generator for the v1 surface has to be code, and the v1 trees +# have to stay alias-only -- they are legacy by design and go with it. +# +# Comments are ignored: "ex +Capybara::Screenshot.active?+" is history, not a +# dependency. Strings are NOT ignored -- a user-facing message naming a +# legacy accessor is a legacy reference that survives the deletion and starts +# lying the day it happens. +class CoreTreeHasNoLegacyDepsTest < ActiveSupport::TestCase + LIB = Pathname.new(__dir__).join("../../lib").expand_path + + # Deleted alongside lib/capybara* in 3.0: these files exist to BUILD the + # v1 compatibility surface (const_missing shims, the legacy config + # accessor generator, the deprecation channel that announces both). + DELETED_IN_3_0 = %w[ + snap_diff/legacy_shims.rb + snap_diff/deprecation.rb + ].freeze + + CORE_FILES = ( + [LIB.join("snap_diff.rb")] + Dir[LIB.join("snap_diff/**/*.rb")].map { |p| Pathname.new(p) } + ).sort.reject { |file| DELETED_IN_3_0.include?(file.relative_path_from(LIB).to_s) }.freeze + + # A require of anything in the v1 trees: `capybara/screenshot/...`, + # `capybara_screenshot_diff...`, `capybara-screenshot-diff`. Plain + # `require "capybara"` / `"capybara/dsl"` is the base gem, not this gem's + # legacy tree, so it must not match. + LEGACY_REQUIRE = %r{\Arequire(_relative)?\s+["']capybara(/screenshot|_screenshot_diff|-screenshot-diff)} + + # A read or write of a v1 namespace constant. + LEGACY_CONSTANT = /(?legacy edge that existed the day the gate was + # written (64 of them), so the gate lands green and every later commit can + # only take entries away. The target is an empty hash. It must never grow: + # a new entry is a decision to keep a core->legacy edge across the 3.0 + # deletion, so it needs a written reason here AND an ADR-008 update -- + # never just a red build turned green. + ALLOWED = { + "snap_diff.rb" => [ + "require \"capybara/screenshot/diff/config_legacy\"", + "require \"capybara/screenshot/diff/image_compare\"", + "yield Capybara::Screenshot, Capybara::Screenshot::Diff" + ], + "snap_diff/browser_helpers.rb" => [ + "if ::Capybara::Screenshot.respond_to?(:window_size) && ::Capybara::Screenshot.window_size", + "resize_to(::Capybara::Screenshot.window_size)" + ], + "snap_diff/config.rb" => [ + "add_driver_path: [Capybara::Screenshot, :add_driver_path],", + "add_os_path: [Capybara::Screenshot, :add_os_path],", + "blur_active_element: [Capybara::Screenshot, :blur_active_element],", + "screenshot_enabled: [Capybara::Screenshot, :enabled],", + "hide_caret: [Capybara::Screenshot, :hide_caret],", + "disable_animations: [Capybara::Screenshot, :disable_animations],", + "root: [Capybara::Screenshot, :root],", + "stability_time_limit: [Capybara::Screenshot, :stability_time_limit],", + "window_size: [Capybara::Screenshot, :window_size],", + "save_path: [Capybara::Screenshot, :save_path],", + "use_lfs: [Capybara::Screenshot, :use_lfs],", + "screenshot_format: [Capybara::Screenshot, :screenshot_format],", + "capybara_screenshot_options: [Capybara::Screenshot, :capybara_screenshot_options],", + "delayed: [Capybara::Screenshot::Diff, :delayed],", + "area_size_limit: [Capybara::Screenshot::Diff, :area_size_limit],", + "fail_if_new: [Capybara::Screenshot::Diff, :fail_if_new],", + "pending_if_new: [Capybara::Screenshot::Diff, :pending_if_new],", + "fail_on_difference: [Capybara::Screenshot::Diff, :fail_on_difference],", + "color_distance_limit: [Capybara::Screenshot::Diff, :color_distance_limit],", + "enabled: [Capybara::Screenshot::Diff, :enabled],", + "shift_distance_limit: [Capybara::Screenshot::Diff, :shift_distance_limit],", + "skip_area: [Capybara::Screenshot::Diff, :skip_area],", + "driver: [Capybara::Screenshot::Diff, :driver],", + "tolerance: [Capybara::Screenshot::Diff, :tolerance],", + "perceptual_threshold: [Capybara::Screenshot::Diff, :perceptual_threshold],", + "screenshoter: [Capybara::Screenshot::Diff, :screenshoter],", + "manager: [Capybara::Screenshot::Diff, :manager]" + ], + "snap_diff/drivers.rb" => [ + "Capybara::Screenshot::Diff::AVAILABLE_DRIVERS" + ], + "snap_diff/dsl.rb" => [ + "require \"capybara/screenshot/diff/config_legacy\"", + "require \"capybara/screenshot/diff/drivers\"", + "require \"capybara/screenshot/diff/screenshot_matcher\"", + "return false unless Capybara::Screenshot.active?", + "delayed = options.fetch(:delayed, Capybara::Screenshot::Diff.delayed)", + "return false unless Capybara::Screenshot.active?" + ], + "snap_diff/integrations/cucumber.rb" => [ + "Capybara::Screenshot::Diff.delayed = false" + ], + "snap_diff/reporters/html.rb" => [ + "require \"capybara/screenshot/diff/config_legacy\"", + "root = Capybara::Screenshot.root || Pathname.pwd", + "root / Capybara::Screenshot.save_path / \"snap_diff_report.html\"" + ], + "snap_diff/screenshot_assertion.rb" => [ + "return unless ::Capybara::Screenshot::Diff.pending_if_new && session.new_screenshots_present?", + "return unless ::Capybara::Screenshot.active? && ::Capybara::Screenshot::Diff.fail_on_difference", + "return unless ::Capybara::Screenshot.active? && ::Capybara::Screenshot::Diff.fail_on_difference" + ], + "snap_diff/screenshot_matcher.rb" => [ + "require \"capybara_screenshot_diff/snap_manager\"", + "@driver_options = Capybara::Screenshot::Diff.default_options.merge(options)", + "Capture::Viewport.prepare!(Capybara::Screenshot.window_size)", + "Capture::Viewport.prepare!(Capybara::Screenshot.window_size)", + "if Capybara::Screenshot::Diff.fail_if_new && !@snapshot.base_path.exist?", + "To allow new screenshots: Capybara::Screenshot::Diff.fail_if_new = false", + "Capybara::Screenshot::Diff.screenshoter.new(capture_options, comparison_options)" + ], + "snap_diff/screenshot_namer.rb" => [ + "@screenshot_area ||= Capybara::Screenshot.screenshot_area" + ], + "snap_diff/screenshoter.rb" => [ + "blurred_input = BrowserHelpers.blur_from_focused_element if Capybara::Screenshot.blur_active_element", + "BrowserHelpers.hide_caret if Capybara::Screenshot.hide_caret", + "BrowserHelpers.disable_animations if Capybara::Screenshot.disable_animations", + "expected_image_width = Capybara::Screenshot.window_size[0]", + "Os::ON_MAC && BrowserHelpers.selenium? && Capybara::Screenshot.window_size" + ], + "snap_diff/snap_manager.rb" => [ + "manager_class = Capybara::Screenshot::Diff.manager", + "root = Pathname.new(Capybara::Screenshot.screenshot_area_abs)" + ], + "snap_diff/stable_screenshoter.rb" => [ + "@screenshoter = Capybara::Screenshot::Diff.screenshoter.new(capture_options.except(:stability_time_limit), @comparison_options)" + ], + "snap_diff/static.rb" => [ + "Capybara::Screenshot.root = root" + ], + "snap_diff/vcs.rb" => [ + "if Capybara::Screenshot.use_lfs" + ] + }.freeze + + test "no core file requires or references the v1 namespaces" do + refute_empty CORE_FILES, "core glob matched nothing -- the gate would pass vacuously" + + offenders = CORE_FILES.flat_map { |file| offences(file) } + + assert_empty offenders, <<~MSG + The canonical core still depends on the v1 compatibility trees. Repoint + these at their snap_diff/* equivalents (`SnapDiff.config.*`, + `SnapDiff::Drivers.*`, `require "snap_diff/..."`) -- until they are gone, + `git rm lib/capybara*` breaks the gem: + + #{offenders.join("\n")} + MSG + end + + test "the allowlist names only lines that still exist" do + stale = ALLOWED.flat_map do |path, lines| + present = significant_lines(LIB.join(path)).map(&:first) + (lines - present).map { |line| "#{path}: allowlisted line no longer present: `#{line}`" } + end + + assert_empty stale, <<~MSG + The allowlist is out of date -- these entries protect nothing and only + hide future regressions. Delete them: + + #{stale.join("\n")} + MSG + end + + private + + def offences(file) + rel = file.relative_path_from(LIB).to_s + allowed = ALLOWED.fetch(rel, []) + + significant_lines(file).filter_map do |line, number| + next if allowed.include?(line) + + reason = + if LEGACY_REQUIRE.match?(line) then "requires a v1 tree path" + elsif LEGACY_CONSTANT.match?(line) then "references a v1 namespace constant" + end + "#{rel}:#{number}: #{reason} -- `#{line}`" if reason + end + end + + # [stripped line, 1-based line number] for every line that is not blank and + # not a whole-line comment. + def significant_lines(file) + file.read.lines.each_with_index.filter_map do |line, index| + stripped = line.strip + [stripped, index + 1] unless stripped.empty? || stripped.start_with?("#") + end + end +end From 065c8f46d4b378d75992e8ab0f820db56dd1e1f2 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Sun, 23 Aug 2026 10:34:36 +0200 Subject: [PATCH 02/11] refactor: driver detection lives on SnapDiff::Drivers SnapDiff::Drivers.available is documented canonical API, but the list it read was defined only in config_legacy.rb, so ruby -Ilib -e 'require "snap_diff/drivers"; SnapDiff::Drivers.available' raised NameError: uninitialized constant Capybara::Screenshot::Diff. It now answers. Detection moved to Drivers.detect_available and runs at drivers.rb load (same load moment in every entry-point path); SnapDiff::Utils .detect_available_drivers one-lines into it, so the documented Utils name and its tests are unchanged. config_legacy keeps AVAILABLE_DRIVERS as an eager same-object alias -- test_helper still reads it at boot, and namespace_forwarding_test still pins assert_same. The stubbing point moves with the value: image_compare_test now stubs SnapDiff::Drivers::AVAILABLE_DRIVERS. Stubbing the legacy alias would only rebind the alias, which is the whole point of cutting the edge. utils.rb requires drivers and drivers now needs Utils at call time, so drivers.rb requires utils at the bottom -- either file can be required first. --- lib/capybara/screenshot/diff/config_legacy.rb | 9 ++-- lib/snap_diff/drivers.rb | 45 ++++++++++++++++--- lib/snap_diff/utils.rb | 19 +++----- .../unit/core_tree_has_no_legacy_deps_test.rb | 3 -- test/unit/drivers_test.rb | 45 +++++++++++++++++++ test/unit/image_compare_test.rb | 7 ++- 6 files changed, 99 insertions(+), 29 deletions(-) create mode 100644 test/unit/drivers_test.rb diff --git a/lib/capybara/screenshot/diff/config_legacy.rb b/lib/capybara/screenshot/diff/config_legacy.rb index 0fed2093..29155134 100644 --- a/lib/capybara/screenshot/diff/config_legacy.rb +++ b/lib/capybara/screenshot/diff/config_legacy.rb @@ -17,9 +17,10 @@ # default blocks used to run. snap_diff/config never requires back here, # so the graph stays acyclic. require "snap_diff/config" -# AVAILABLE_DRIVERS below is evaluated at class-body eval time, so Utils -# must be a real, already-loaded module before this module body runs. -require "snap_diff/utils" +# AVAILABLE_DRIVERS below aliases SnapDiff::Drivers::AVAILABLE_DRIVERS at +# class-body eval time, so Drivers must be a real, already-loaded module +# before this module body runs. +require "snap_diff/drivers" module Capybara module Screenshot @@ -39,7 +40,7 @@ def screenshot_area_abs # Module to track screenshot changes module Diff - AVAILABLE_DRIVERS = SnapDiff::Utils.detect_available_drivers.freeze + AVAILABLE_DRIVERS = SnapDiff::Drivers::AVAILABLE_DRIVERS # Configure screenshot and diff settings in one block. # diff --git a/lib/snap_diff/drivers.rb b/lib/snap_diff/drivers.rb index f1e02183..97e8738c 100644 --- a/lib/snap_diff/drivers.rb +++ b/lib/snap_diff/drivers.rb @@ -21,14 +21,45 @@ def self.loaded @loaded ||= {} end - # Canonical read API for the detected-drivers list. The value itself - # stays on Capybara::Screenshot::Diff::AVAILABLE_DRIVERS (assigned in - # config_legacy.rb at load time, exactly when detection historically - # ran); this reads it live rather than caching, because that constant - # is the published stubbing point (image_compare_test stubs it to [] - # to exercise the no-drivers error path). + # Which image drivers this process can actually load, in preference + # order. Tries the gem first and falls back to `require`, cleaning up + # the half-defined constant a failed native load leaves behind. + def self.detect_available + result = [] + begin + result << :vips if defined?(Vips) || require("vips") + rescue LoadError + # vips not present + Object.send(:remove_const, :Vips) if defined?(Vips) + end + begin + result << :chunky_png if defined?(ChunkyPNG) || require("chunky_png") + rescue LoadError + # chunky_png not present + Object.send(:remove_const, :ChunkyPNG) if defined?(ChunkyPNG) + end + result + end + + # Canonical home of the detected-drivers list (3.0 readiness: it used + # to live only on Capybara::Screenshot::Diff::AVAILABLE_DRIVERS, so + # `require "snap_diff/drivers"` alone left .available raising + # NameError). Detection runs HERE, at this file's load, and the legacy + # constant is now an eager same-object alias of this one. + AVAILABLE_DRIVERS = detect_available.freeze + + # Canonical read API for the list above. Reads the constant live rather + # than caching, because the constant is the published stubbing point + # (image_compare_test stubs it to [] to exercise the no-drivers error + # path). def self.available - Capybara::Screenshot::Diff::AVAILABLE_DRIVERS + AVAILABLE_DRIVERS end end end + +# Drivers.for calls Utils.find_driver_class_for, and utils.rb requires this +# file back -- so the require sits at the BOTTOM, after Drivers is fully +# defined. Either file can then be required first: whichever runs second +# finds a complete module, and neither touches the other at body-eval time. +require "snap_diff/utils" diff --git a/lib/snap_diff/utils.rb b/lib/snap_diff/utils.rb index 4124b7da..41646ada 100644 --- a/lib/snap_diff/utils.rb +++ b/lib/snap_diff/utils.rb @@ -4,21 +4,12 @@ module SnapDiff module Utils + # Detection itself lives on Drivers now (its canonical home -- so that + # `require "snap_diff/drivers"` standalone can answer .available); this + # keeps the documented Utils name working. One-way: Drivers never calls + # back here at load time, so requiring either file first is safe. def self.detect_available_drivers - result = [] - begin - result << :vips if defined?(Vips) || require("vips") - rescue LoadError - # vips not present - Object.send(:remove_const, :Vips) if defined?(Vips) - end - begin - result << :chunky_png if defined?(ChunkyPNG) || require("chunky_png") - rescue LoadError - # chunky_png not present - Object.send(:remove_const, :ChunkyPNG) if defined?(ChunkyPNG) - end - result + Drivers.detect_available end def self.find_driver_class_for(driver) diff --git a/test/unit/core_tree_has_no_legacy_deps_test.rb b/test/unit/core_tree_has_no_legacy_deps_test.rb index 6149c8a1..19509d7a 100644 --- a/test/unit/core_tree_has_no_legacy_deps_test.rb +++ b/test/unit/core_tree_has_no_legacy_deps_test.rb @@ -97,9 +97,6 @@ class CoreTreeHasNoLegacyDepsTest < ActiveSupport::TestCase "screenshoter: [Capybara::Screenshot::Diff, :screenshoter],", "manager: [Capybara::Screenshot::Diff, :manager]" ], - "snap_diff/drivers.rb" => [ - "Capybara::Screenshot::Diff::AVAILABLE_DRIVERS" - ], "snap_diff/dsl.rb" => [ "require \"capybara/screenshot/diff/config_legacy\"", "require \"capybara/screenshot/diff/drivers\"", diff --git a/test/unit/drivers_test.rb b/test/unit/drivers_test.rb new file mode 100644 index 00000000..c7531f92 --- /dev/null +++ b/test/unit/drivers_test.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require "test_helper" +require "open3" +require "minitest/stub_const" + +# SnapDiff::Drivers is the canonical driver registry (docs/snapdiff.md). +# Until 3.0 readiness work, `.available` read the value out of +# Capybara::Screenshot::Diff::AVAILABLE_DRIVERS, which is defined by +# config_legacy.rb -- so a documented canonical API only worked when the v1 +# tree happened to be loaded. +class DriversTest < ActiveSupport::TestCase + # The regression: `require "snap_diff/drivers"` alone used to raise + # `NameError: uninitialized constant Capybara::Screenshot::Diff` here. + # test_helper preloads the whole gem, so only a fresh process can catch it. + test ".available answers after requiring snap_diff/drivers and nothing else" do + script = <<~RUBY + require "snap_diff/drivers" + drivers = SnapDiff::Drivers.available + abort("not an Array: \#{drivers.inspect}") unless drivers.is_a?(Array) + # ...and it answered on its own, without config_legacy.rb being loaded. + # (Constants alone would not prove it: bundler/setup evaluates the + # gemspec, which loads the legacy version.rb and so defines + # Capybara::Screenshot::Diff in any subprocess.) + legacy = $LOADED_FEATURES.grep(/config_legacy\\.rb\\z/) + abort("config_legacy got loaded: \#{legacy.join(", ")}") unless legacy.empty? + RUBY + + out, status = Open3.capture2e(RbConfig.ruby, "-Ilib", "-e", script) + + assert status.success?, "expected standalone snap_diff/drivers to answer .available, got:\n#{out}" + end + + test ".available reads the constant live, so it stays stubbable" do + SnapDiff::Drivers.stub_const(:AVAILABLE_DRIVERS, []) do + assert_empty SnapDiff::Drivers.available + end + + assert_equal SnapDiff::Drivers::AVAILABLE_DRIVERS, SnapDiff::Drivers.available + end + + test "detection is reachable under both the canonical and the documented Utils name" do + assert_equal SnapDiff::Drivers.detect_available, SnapDiff::Utils.detect_available_drivers + end +end diff --git a/test/unit/image_compare_test.rb b/test/unit/image_compare_test.rb index d90826dc..cd4be258 100644 --- a/test/unit/image_compare_test.rb +++ b/test/unit/image_compare_test.rb @@ -79,7 +79,12 @@ class ImageCompareTest < ActiveSupport::TestCase end test "#initialize with :auto driver raises error when no drivers available" do - Capybara::Screenshot::Diff.stub_const(:AVAILABLE_DRIVERS, []) do + # Canonical stubbing point since the detected-drivers list moved to + # SnapDiff::Drivers (3.0 readiness). The legacy + # Capybara::Screenshot::Diff::AVAILABLE_DRIVERS is now an eager + # same-object ALIAS of this constant, so stubbing the old name only + # rebinds the alias and no longer reaches the core. + SnapDiff::Drivers.stub_const(:AVAILABLE_DRIVERS, []) do assert_raise(RuntimeError) do comparison = make_comparison(:b, driver: :auto) assert comparison.quick_equal? From 7d4a469bf48564bf939351fa2ba2462f9f9a9b75 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Sun, 23 Aug 2026 10:39:04 +0200 Subject: [PATCH 03/11] refactor: core config reads go to SnapDiff.config, not the legacy view Twenty-nine reads across twelve core files went through Capybara::Screenshot / Capybara::Screenshot::Diff -- the legacy VIEW of a storage that has lived in SnapDiff::Config since #230. Same storage, canonical name; behaviour is unchanged and the legacy delegators stay for users. Two follow-ons: - browser_helpers dropped its respond_to?(:window_size) guard: that existed because the mattr_accessor might not be installed yet, and Config always has the attribute. - the fail_if_new error message now tells users 'SnapDiff.config.fail_if_new = false' -- it named an accessor that 3.0 deletes. Tests that STUBBED the legacy accessors now stub SnapDiff.config: a delegator write still reaches the storage, but a stubbed delegator method does not, so those stubs were silently no-ops against the new reads. The legacy accessors keep their own coverage in snap_diff_config_test.rb (every MAPPING writer) and config_default_timing_test.rb. --- lib/snap_diff/browser_helpers.rb | 8 +-- lib/snap_diff/dsl.rb | 8 +-- lib/snap_diff/integrations/cucumber.rb | 2 +- lib/snap_diff/reporters/html.rb | 4 +- lib/snap_diff/screenshot_assertion.rb | 4 +- lib/snap_diff/screenshot_matcher.rb | 12 ++--- lib/snap_diff/screenshoter.rb | 10 ++-- lib/snap_diff/snap_manager.rb | 4 +- lib/snap_diff/stable_screenshoter.rb | 4 +- lib/snap_diff/static.rb | 2 +- lib/snap_diff/vcs.rb | 2 +- test/unit/attempts_reporter_test.rb | 2 +- .../unit/core_tree_has_no_legacy_deps_test.rb | 52 ++----------------- test/unit/diff_test.rb | 6 +-- test/unit/dsl_test.rb | 8 +-- test/unit/minitest_assertions_test.rb | 6 +-- test/unit/pending_screenshots_message_test.rb | 8 +-- test/unit/screenshot_matcher_test.rb | 4 +- test/unit/screenshoter_test.rb | 2 +- 19 files changed, 52 insertions(+), 96 deletions(-) diff --git a/lib/snap_diff/browser_helpers.rb b/lib/snap_diff/browser_helpers.rb index 68957261..ee129bb4 100644 --- a/lib/snap_diff/browser_helpers.rb +++ b/lib/snap_diff/browser_helpers.rb @@ -5,9 +5,11 @@ module SnapDiff module BrowserHelpers def self.resize_window_if_needed - if ::Capybara::Screenshot.respond_to?(:window_size) && ::Capybara::Screenshot.window_size - resize_to(::Capybara::Screenshot.window_size) - end + # The respond_to? guard this replaced existed because the legacy + # mattr_accessor might not be installed yet; Config always has the + # attribute, so only the value matters now. + window_size = SnapDiff.config.window_size + resize_to(window_size) if window_size end def self.resize_to(window_size) diff --git a/lib/snap_diff/dsl.rb b/lib/snap_diff/dsl.rb index 52545913..34b8013e 100644 --- a/lib/snap_diff/dsl.rb +++ b/lib/snap_diff/dsl.rb @@ -50,7 +50,7 @@ def screenshot_group(name) # @param name [String] The base name of the screenshot, used to generate the filename. # @param skip_stack_frames [Integer] The number of stack frames to skip when reporting errors. # @param options [Hash] Additional options for taking the screenshot and comparison. - # @option options [Boolean] :delayed (Capybara::Screenshot::Diff.delayed) + # @option options [Boolean] :delayed (SnapDiff.config.delayed) # Whether to validate the screenshot immediately or delay validation. # @option options [Array] :crop [left, top, right, bottom] Edge coordinates to crop the screenshot to. # @option options [Array>] :skip_area Array of [left, top, right, bottom] edge coordinates to ignore. @@ -68,7 +68,7 @@ def screenshot_group(name) # @raise [SnapDiff::UnstableImage] If the image comparison is unstable. # @raise [SnapDiff::WindowSizeMismatchError] If the window size doesn't match expectations. def assert_matches_screenshot(name, skip_stack_frames: 0, **options) - return false unless Capybara::Screenshot.active? + return false unless SnapDiff.config.active? # Get the full name with section and group information full_name = SnapDiff.session.screenshot_namer.full_name(name) @@ -82,7 +82,7 @@ def assert_matches_screenshot(name, skip_stack_frames: 0, **options) return false unless assertion # Determine if validation should be delayed or immediate - delayed = options.fetch(:delayed, Capybara::Screenshot::Diff.delayed) + delayed = options.fetch(:delayed, SnapDiff.config.delayed) if delayed SnapDiff.session.add_assertion(assertion) @@ -110,7 +110,7 @@ def screenshot(name, skip_stack_frames: 0, compare: true, **options) # @param options [Hash] Additional options for taking the screenshot. See {#assert_matches_screenshot}. # @return [Boolean] True if the screenshot was successfully captured. def capture_screenshot(name, **options) - return false unless Capybara::Screenshot.active? + return false unless SnapDiff.config.active? full_name = SnapDiff.session.screenshot_namer.full_name(name) SnapDiff::ScreenshotMatcher.new(full_name, options).capture diff --git a/lib/snap_diff/integrations/cucumber.rb b/lib/snap_diff/integrations/cucumber.rb index 990714b4..b64f279e 100644 --- a/lib/snap_diff/integrations/cucumber.rb +++ b/lib/snap_diff/integrations/cucumber.rb @@ -8,7 +8,7 @@ World(::SnapDiff::DSL) Before do - Capybara::Screenshot::Diff.delayed = false + SnapDiff.config.delayed = false SnapDiff::BrowserHelpers.resize_window_if_needed end diff --git a/lib/snap_diff/reporters/html.rb b/lib/snap_diff/reporters/html.rb index d5819564..951c5659 100644 --- a/lib/snap_diff/reporters/html.rb +++ b/lib/snap_diff/reporters/html.rb @@ -89,8 +89,8 @@ def self.template_path end def self.default_output_path - root = Capybara::Screenshot.root || Pathname.pwd - root / Capybara::Screenshot.save_path / "snap_diff_report.html" + root = SnapDiff.config.root || Pathname.pwd + root / SnapDiff.config.save_path / "snap_diff_report.html" end private diff --git a/lib/snap_diff/screenshot_assertion.rb b/lib/snap_diff/screenshot_assertion.rb index a0551ccc..574aa0ed 100644 --- a/lib/snap_diff/screenshot_assertion.rb +++ b/lib/snap_diff/screenshot_assertion.rb @@ -33,7 +33,7 @@ def self.reset # # @return [String, nil] the pending message, or nil when there is nothing to report def self.pending_screenshots_message - return unless ::Capybara::Screenshot::Diff.pending_if_new && session.new_screenshots_present? + return unless SnapDiff.config.pending_if_new && session.new_screenshots_present? "No baseline for: #{session.new_screenshots.join(", ")}. Commit the captured screenshots to record them." end @@ -96,7 +96,7 @@ def validate! # @return [Array, nil] Returns an array of error messages if there are screenshot differences, otherwise nil. # @note This method is typically called at the end of a test to assert all screenshots are as expected. def self.verify_screenshots!(screenshots) - return unless ::Capybara::Screenshot.active? && ::Capybara::Screenshot::Diff.fail_on_difference + return unless SnapDiff.config.active? && SnapDiff.config.fail_on_difference test_screenshot_errors = screenshots.map do |assertion| assertion.validate diff --git a/lib/snap_diff/screenshot_matcher.rb b/lib/snap_diff/screenshot_matcher.rb index 536bc1f1..5679406f 100644 --- a/lib/snap_diff/screenshot_matcher.rb +++ b/lib/snap_diff/screenshot_matcher.rb @@ -14,14 +14,14 @@ class ScreenshotMatcher def initialize(screenshot_full_name, options = {}) @screenshot_full_name = screenshot_full_name - @driver_options = Capybara::Screenshot::Diff.default_options.merge(options) + @driver_options = SnapDiff.config.default_options.merge(options) @screenshot_format = @driver_options[:screenshot_format] @snapshot = SnapDiff::SnapManager.snapshot(screenshot_full_name, @screenshot_format) end def build_screenshot_assertion(skip_stack_frames: 0) - Capture::Viewport.prepare!(Capybara::Screenshot.window_size) + Capture::Viewport.prepare!(SnapDiff.config.window_size) prepare_screenshot_options check_base_screenshot @@ -41,7 +41,7 @@ def build_screenshot_assertion(skip_stack_frames: 0) # Captures a screenshot without comparing it to a baseline. def capture - Capture::Viewport.prepare!(Capybara::Screenshot.window_size) + Capture::Viewport.prepare!(SnapDiff.config.window_size) prepare_screenshot_options capture_options, comparison_options = extract_capture_and_comparison_options(driver_options) @@ -67,11 +67,11 @@ def prepare_screenshot_options def check_base_screenshot @snapshot.checkout_base_screenshot - if Capybara::Screenshot::Diff.fail_if_new && !@snapshot.base_path.exist? + if SnapDiff.config.fail_if_new && !@snapshot.base_path.exist? raise SnapDiff::ExpectationNotMet.new(<<~ERROR.chomp, caller) No existing screenshot found for #{@snapshot.base_path}! To record baselines: RECORD_SCREENSHOTS=1 bundle exec rake test - To allow new screenshots: Capybara::Screenshot::Diff.fail_if_new = false + To allow new screenshots: SnapDiff.config.fail_if_new = false ERROR end end @@ -80,7 +80,7 @@ def capture_screenshot(capture_options, comparison_options) screenshoter = if capture_options[:stability_time_limit] StableScreenshoter.new(capture_options, comparison_options) else - Capybara::Screenshot::Diff.screenshoter.new(capture_options, comparison_options) + SnapDiff.config.screenshoter.new(capture_options, comparison_options) end screenshoter.take_comparison_screenshot(@snapshot) end diff --git a/lib/snap_diff/screenshoter.rb b/lib/snap_diff/screenshoter.rb index 391096af..13cc16bc 100644 --- a/lib/snap_diff/screenshoter.rb +++ b/lib/snap_diff/screenshoter.rb @@ -69,10 +69,10 @@ def notice_how_to_avoid_this def prepare_page_for_screenshot(timeout:) wait_images_loaded(timeout: timeout) if timeout - blurred_input = BrowserHelpers.blur_from_focused_element if Capybara::Screenshot.blur_active_element + blurred_input = BrowserHelpers.blur_from_focused_element if SnapDiff.config.blur_active_element - BrowserHelpers.hide_caret if Capybara::Screenshot.hide_caret - BrowserHelpers.disable_animations if Capybara::Screenshot.disable_animations + BrowserHelpers.hide_caret if SnapDiff.config.hide_caret + BrowserHelpers.disable_animations if SnapDiff.config.disable_animations blurred_input end @@ -111,7 +111,7 @@ def capture_screenshot_at(snapshot) end def resize_if_needed(saved_image) - expected_image_width = Capybara::Screenshot.window_size[0] + expected_image_width = SnapDiff.config.window_size[0] return saved_image if driver.width_for(saved_image) < expected_image_width * 2 notice_how_to_avoid_this @@ -121,7 +121,7 @@ def resize_if_needed(saved_image) end def selenium_with_retina_screen? - Os::ON_MAC && BrowserHelpers.selenium? && Capybara::Screenshot.window_size + Os::ON_MAC && BrowserHelpers.selenium? && SnapDiff.config.window_size end end end diff --git a/lib/snap_diff/snap_manager.rb b/lib/snap_diff/snap_manager.rb index d2aa64f5..3611fc88 100644 --- a/lib/snap_diff/snap_manager.rb +++ b/lib/snap_diff/snap_manager.rb @@ -110,8 +110,8 @@ def self.root # test/fixtures/app/doc/screenshots/ that VCS rollback just restored. # Do not reorder those teardowns without revisiting this. def self.instance - manager_class = Capybara::Screenshot::Diff.manager - root = Pathname.new(Capybara::Screenshot.screenshot_area_abs) + manager_class = SnapDiff.config.manager + root = Pathname.new(SnapDiff.config.screenshot_area_abs) current = Thread.current[:snap_diff_manager] unless current&.instance_of?(manager_class) && current.root == root diff --git a/lib/snap_diff/stable_screenshoter.rb b/lib/snap_diff/stable_screenshoter.rb index ed50fe6c..06f7e196 100644 --- a/lib/snap_diff/stable_screenshoter.rb +++ b/lib/snap_diff/stable_screenshoter.rb @@ -12,7 +12,7 @@ class StableScreenshoter # `:stability_time_limit` and `:wait` in capture options and ensures that `:stability_time_limit` is less than or equal to `:wait`. # # @param capture_options [Hash] The options for capturing screenshots, must include `:stability_time_limit` and `:wait`. - # @param comparison_options [Hash] The options for comparing screenshots, defaults to `{}`. Same signature as {Capybara::Screenshot::Screenshoter#initialize}. + # @param comparison_options [Hash] The options for comparing screenshots, defaults to `{}`. Same signature as {SnapDiff::Screenshoter#initialize}. # @raise [ArgumentError] If `:wait` or `:stability_time_limit` are not provided, or if `:stability_time_limit` is greater than `:wait`. def initialize(capture_options, comparison_options = {}) @stability_time_limit, @wait = capture_options.fetch_values(*STABILITY_OPTIONS) @@ -23,7 +23,7 @@ def initialize(capture_options, comparison_options = {}) @comparison_options = comparison_options - @screenshoter = Capybara::Screenshot::Diff.screenshoter.new(capture_options.except(:stability_time_limit), @comparison_options) + @screenshoter = SnapDiff.config.screenshoter.new(capture_options.except(:stability_time_limit), @comparison_options) end # Takes a comparison screenshot ensuring page stability diff --git a/lib/snap_diff/static.rb b/lib/snap_diff/static.rb index 65628814..18769d97 100644 --- a/lib/snap_diff/static.rb +++ b/lib/snap_diff/static.rb @@ -6,6 +6,6 @@ module SnapDiff def self.serve(directory, root: Dir.pwd) Capybara.app = Rack::Files.new(directory) - Capybara::Screenshot.root = root + SnapDiff.config.root = root end end diff --git a/lib/snap_diff/vcs.rb b/lib/snap_diff/vcs.rb index 4c2df0de..49454be7 100644 --- a/lib/snap_diff/vcs.rb +++ b/lib/snap_diff/vcs.rb @@ -13,7 +13,7 @@ def self.checkout_vcs(root, screenshot_path, checkout_path) git_root = git_root.chomp vcs_file_path = Pathname.new(screenshot_path).expand_path.relative_path_from(Pathname.new(git_root)).to_s - if Capybara::Screenshot.use_lfs + if SnapDiff.config.use_lfs tmp_path = "#{checkout_path}.tmp" success = system("git", "-C", root_path, "show", "HEAD:#{vcs_file_path}", out: tmp_path, err: File::NULL) if success diff --git a/test/unit/attempts_reporter_test.rb b/test/unit/attempts_reporter_test.rb index 54e4c954..1f8d02ff 100644 --- a/test/unit/attempts_reporter_test.rb +++ b/test/unit/attempts_reporter_test.rb @@ -59,7 +59,7 @@ def take_screenshot(screenshot_path) snap = @manager.snapshot("unstable_end_to_end") error = nil - Capybara::Screenshot::Diff.stub(:screenshoter, alternating_screenshoter) do + SnapDiff.config.stub(:screenshoter, alternating_screenshoter) do error = assert_raises(CapybaraScreenshotDiff::UnstableImage) do StableScreenshoter .new({stability_time_limit: 0.05, wait: 0.2}, {driver: :chunky_png}) diff --git a/test/unit/core_tree_has_no_legacy_deps_test.rb b/test/unit/core_tree_has_no_legacy_deps_test.rb index 19509d7a..d1dcdd32 100644 --- a/test/unit/core_tree_has_no_legacy_deps_test.rb +++ b/test/unit/core_tree_has_no_legacy_deps_test.rb @@ -64,10 +64,6 @@ class CoreTreeHasNoLegacyDepsTest < ActiveSupport::TestCase "require \"capybara/screenshot/diff/image_compare\"", "yield Capybara::Screenshot, Capybara::Screenshot::Diff" ], - "snap_diff/browser_helpers.rb" => [ - "if ::Capybara::Screenshot.respond_to?(:window_size) && ::Capybara::Screenshot.window_size", - "resize_to(::Capybara::Screenshot.window_size)" - ], "snap_diff/config.rb" => [ "add_driver_path: [Capybara::Screenshot, :add_driver_path],", "add_os_path: [Capybara::Screenshot, :add_os_path],", @@ -100,55 +96,13 @@ class CoreTreeHasNoLegacyDepsTest < ActiveSupport::TestCase "snap_diff/dsl.rb" => [ "require \"capybara/screenshot/diff/config_legacy\"", "require \"capybara/screenshot/diff/drivers\"", - "require \"capybara/screenshot/diff/screenshot_matcher\"", - "return false unless Capybara::Screenshot.active?", - "delayed = options.fetch(:delayed, Capybara::Screenshot::Diff.delayed)", - "return false unless Capybara::Screenshot.active?" - ], - "snap_diff/integrations/cucumber.rb" => [ - "Capybara::Screenshot::Diff.delayed = false" + "require \"capybara/screenshot/diff/screenshot_matcher\"" ], "snap_diff/reporters/html.rb" => [ - "require \"capybara/screenshot/diff/config_legacy\"", - "root = Capybara::Screenshot.root || Pathname.pwd", - "root / Capybara::Screenshot.save_path / \"snap_diff_report.html\"" - ], - "snap_diff/screenshot_assertion.rb" => [ - "return unless ::Capybara::Screenshot::Diff.pending_if_new && session.new_screenshots_present?", - "return unless ::Capybara::Screenshot.active? && ::Capybara::Screenshot::Diff.fail_on_difference", - "return unless ::Capybara::Screenshot.active? && ::Capybara::Screenshot::Diff.fail_on_difference" + "require \"capybara/screenshot/diff/config_legacy\"" ], "snap_diff/screenshot_matcher.rb" => [ - "require \"capybara_screenshot_diff/snap_manager\"", - "@driver_options = Capybara::Screenshot::Diff.default_options.merge(options)", - "Capture::Viewport.prepare!(Capybara::Screenshot.window_size)", - "Capture::Viewport.prepare!(Capybara::Screenshot.window_size)", - "if Capybara::Screenshot::Diff.fail_if_new && !@snapshot.base_path.exist?", - "To allow new screenshots: Capybara::Screenshot::Diff.fail_if_new = false", - "Capybara::Screenshot::Diff.screenshoter.new(capture_options, comparison_options)" - ], - "snap_diff/screenshot_namer.rb" => [ - "@screenshot_area ||= Capybara::Screenshot.screenshot_area" - ], - "snap_diff/screenshoter.rb" => [ - "blurred_input = BrowserHelpers.blur_from_focused_element if Capybara::Screenshot.blur_active_element", - "BrowserHelpers.hide_caret if Capybara::Screenshot.hide_caret", - "BrowserHelpers.disable_animations if Capybara::Screenshot.disable_animations", - "expected_image_width = Capybara::Screenshot.window_size[0]", - "Os::ON_MAC && BrowserHelpers.selenium? && Capybara::Screenshot.window_size" - ], - "snap_diff/snap_manager.rb" => [ - "manager_class = Capybara::Screenshot::Diff.manager", - "root = Pathname.new(Capybara::Screenshot.screenshot_area_abs)" - ], - "snap_diff/stable_screenshoter.rb" => [ - "@screenshoter = Capybara::Screenshot::Diff.screenshoter.new(capture_options.except(:stability_time_limit), @comparison_options)" - ], - "snap_diff/static.rb" => [ - "Capybara::Screenshot.root = root" - ], - "snap_diff/vcs.rb" => [ - "if Capybara::Screenshot.use_lfs" + "require \"capybara_screenshot_diff/snap_manager\"" ] }.freeze diff --git a/test/unit/diff_test.rb b/test/unit/diff_test.rb index 50e21caa..1c3c0344 100644 --- a/test/unit/diff_test.rb +++ b/test/unit/diff_test.rb @@ -71,7 +71,7 @@ class DiffTest < ActiveSupport::TestCase end test "does not fail when fail_on_difference is false and screenshots differ" do - Capybara::Screenshot::Diff.stub(:fail_on_difference, false) do + SnapDiff.config.stub(:fail_on_difference, false) do test_case = SampleMiniTestCase.new(:_test_sample_screenshot_error) test_case.run assert_equal 0, test_case.failures.size @@ -217,7 +217,7 @@ class ScreenshotFormatTest < ActiveSupport::TestCase set_test_images(snap, :a, :a) - Capybara::Screenshot.stub(:screenshot_format, "webp") do + SnapDiff.config.stub(:screenshot_format, "webp") do screenshot "a", driver: :vips assert_stored_screenshot("a.webp") @@ -228,7 +228,7 @@ class ScreenshotFormatTest < ActiveSupport::TestCase snap = SnapDiff::SnapManager.snapshot("a", "png") set_test_images(snap, :a, :a) - Capybara::Screenshot.stub(:screenshot_format, "webp") do + SnapDiff.config.stub(:screenshot_format, "webp") do screenshot "a", screenshot_format: "png" assert_stored_screenshot("a.png") diff --git a/test/unit/dsl_test.rb b/test/unit/dsl_test.rb index a7c418d5..cdbcf37a 100644 --- a/test/unit/dsl_test.rb +++ b/test/unit/dsl_test.rb @@ -24,7 +24,7 @@ def after_teardown test "#screenshot raises error when screenshot is missing and fail_if_new is true" do SnapDiff::Vcs.stub(:checkout_vcs, false) do - Capybara::Screenshot::Diff.stub(:fail_if_new, true) do + SnapDiff.config.stub(:fail_if_new, true) do assert_raises CapybaraScreenshotDiff::ExpectationNotMet, match: /No existing screenshot found for/ do screenshot "not_existing_screenshot-name" end @@ -117,7 +117,7 @@ def assert_no_screenshot_jobs_scheduled test "#screenshot with delayed: false raises error when images differ" do SnapDiff::Vcs.stub(:checkout_vcs, true) do - Capybara::Screenshot::Diff.stub(:delayed, false) do + SnapDiff.config.stub(:delayed, false) do assert_raises(CapybaraScreenshotDiff::ExpectationNotMet) do snap = create_snapshot_for(:c, :a) screenshot(snap.full_name, delayed: false) @@ -128,7 +128,7 @@ def assert_no_screenshot_jobs_scheduled test "#screenshot with delayed: false succeeds when images match" do SnapDiff::Vcs.stub(:checkout_vcs, true) do - Capybara::Screenshot::Diff.stub(:delayed, false) do + SnapDiff.config.stub(:delayed, false) do snap = create_snapshot_for(:a) assert_nothing_raised { screenshot(snap.full_name, delayed: false) } end @@ -216,7 +216,7 @@ def take_comparison_screenshot(snapshot) end end - Capybara::Screenshot::Diff.stub(:screenshoter, naive_screenshoter) do + SnapDiff.config.stub(:screenshoter, naive_screenshoter) do capture_screenshot("nested/dir/example") assert_predicate SnapDiff::SnapManager.snapshot("nested/dir/example").path, :exist? diff --git a/test/unit/minitest_assertions_test.rb b/test/unit/minitest_assertions_test.rb index 1c12cca8..7f4a62af 100644 --- a/test/unit/minitest_assertions_test.rb +++ b/test/unit/minitest_assertions_test.rb @@ -24,7 +24,7 @@ def run_inner_test(teardown: nil, &block) test "#before_teardown skips the test when pending_if_new is enabled and a screenshot has no baseline" do SnapDiff::Vcs.stub(:checkout_vcs, false) do - Capybara::Screenshot::Diff.stub(:pending_if_new, true) do + SnapDiff.config.stub(:pending_if_new, true) do result = run_inner_test { screenshot("a") } assert_predicate result, :skipped? @@ -50,7 +50,7 @@ def run_inner_test(teardown: nil, &block) test "#before_teardown does not mask a real teardown error behind a pending skip" do SnapDiff::Vcs.stub(:checkout_vcs, false) do - Capybara::Screenshot::Diff.stub(:pending_if_new, true) do + SnapDiff.config.stub(:pending_if_new, true) do result = run_inner_test(teardown: proc { super() raise "boom from teardown" @@ -64,7 +64,7 @@ def run_inner_test(teardown: nil, &block) test "#before_teardown does not skip the test when pending_if_new is disabled" do SnapDiff::Vcs.stub(:checkout_vcs, false) do - Capybara::Screenshot::Diff.stub(:pending_if_new, false) do + SnapDiff.config.stub(:pending_if_new, false) do result = run_inner_test { screenshot("a") } assert_predicate result, :passed? diff --git a/test/unit/pending_screenshots_message_test.rb b/test/unit/pending_screenshots_message_test.rb index fc956033..90d4175f 100644 --- a/test/unit/pending_screenshots_message_test.rb +++ b/test/unit/pending_screenshots_message_test.rb @@ -9,7 +9,7 @@ class PendingScreenshotsMessageTest < ActiveSupport::TestCase end test "returns nil when pending_if_new is disabled" do - Capybara::Screenshot::Diff.stub(:pending_if_new, false) do + SnapDiff.config.stub(:pending_if_new, false) do CapybaraScreenshotDiff.record_new_screenshot("a") assert_nil CapybaraScreenshotDiff.pending_screenshots_message @@ -17,13 +17,13 @@ class PendingScreenshotsMessageTest < ActiveSupport::TestCase end test "returns nil when pending_if_new is enabled but no new screenshots were recorded" do - Capybara::Screenshot::Diff.stub(:pending_if_new, true) do + SnapDiff.config.stub(:pending_if_new, true) do assert_nil CapybaraScreenshotDiff.pending_screenshots_message end end test "returns the baseline message listing recorded screenshot names" do - Capybara::Screenshot::Diff.stub(:pending_if_new, true) do + SnapDiff.config.stub(:pending_if_new, true) do CapybaraScreenshotDiff.record_new_screenshot("a") CapybaraScreenshotDiff.record_new_screenshot("b") @@ -35,7 +35,7 @@ class PendingScreenshotsMessageTest < ActiveSupport::TestCase end test "reads from the calling thread's own registry, not other threads'" do - Capybara::Screenshot::Diff.stub(:pending_if_new, true) do + SnapDiff.config.stub(:pending_if_new, true) do other_thread_result = Thread.new { CapybaraScreenshotDiff.record_new_screenshot("other-thread") CapybaraScreenshotDiff.pending_screenshots_message diff --git a/test/unit/screenshot_matcher_test.rb b/test/unit/screenshot_matcher_test.rb index 437c164f..bd60c579 100644 --- a/test/unit/screenshot_matcher_test.rb +++ b/test/unit/screenshot_matcher_test.rb @@ -69,7 +69,7 @@ def take_comparison_screenshot(snapshot) calls = [] SnapDiff::Vcs.stub(:checkout_vcs, true) do - Capybara::Screenshot::Diff.stub(:screenshoter, recording_screenshoter(calls)) do + SnapDiff.config.stub(:screenshoter, recording_screenshoter(calls)) do snap = create_snapshot_for(:a, :c) SnapDiff::ScreenshotMatcher.new(snap.full_name, tolerance: 0.03, wait: 5).build_screenshot_assertion @@ -110,7 +110,7 @@ def take_comparison_screenshot(snapshot) calls = [] SnapDiff::Vcs.stub(:checkout_vcs, true) do - Capybara::Screenshot::Diff.stub(:screenshoter, recording_screenshoter(calls)) do + SnapDiff.config.stub(:screenshoter, recording_screenshoter(calls)) do snap = create_snapshot_for(:a, :c) SnapDiff::ScreenshotMatcher.new(snap.full_name).build_screenshot_assertion end diff --git a/test/unit/screenshoter_test.rb b/test/unit/screenshoter_test.rb index 3d4fed38..0f156f99 100644 --- a/test/unit/screenshoter_test.rb +++ b/test/unit/screenshoter_test.rb @@ -53,7 +53,7 @@ class ScreenshoterTest < ActiveSupport::TestCase screenshoter = SnapDiff::Screenshoter.new({}, {driver: :vips}) retina_image = Vips::Image.black(2560, 1600) # 2x window size, non-square - resized = Screenshot.stub(:window_size, [1280, 1024]) do + resized = SnapDiff.config.stub(:window_size, [1280, 1024]) do screenshoter.send(:resize_if_needed, retina_image) end From 3cd2c18530df9ec20355cc568a87c5d5095e7390 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Sun, 23 Aug 2026 10:42:04 +0200 Subject: [PATCH 04/11] refactor: core requires point at snap_diff/*, not the v1 forwarders dsl.rb, reporters/html.rb and screenshot_matcher.rb pulled their dependencies through capybara/screenshot/diff/* and capybara_screenshot_diff/* forwarders -- files whose only content is a require of the snap_diff/* unit the core actually wanted. Point at the units directly. snap_diff.rb's own two backward requires are the last ones left and go with the v1-surface consolidation. --- lib/snap_diff/dsl.rb | 13 +++++++------ lib/snap_diff/reporters/html.rb | 2 +- lib/snap_diff/screenshot_matcher.rb | 2 +- test/unit/core_tree_has_no_legacy_deps_test.rb | 11 ----------- 4 files changed, 9 insertions(+), 19 deletions(-) diff --git a/lib/snap_diff/dsl.rb b/lib/snap_diff/dsl.rb index 34b8013e..2695c8c7 100644 --- a/lib/snap_diff/dsl.rb +++ b/lib/snap_diff/dsl.rb @@ -8,16 +8,17 @@ require "snap_diff" # Must NOT require "capybara_screenshot_diff": that would cycle back here via -# this file's old-path forwarder. Every Capybara::Screenshot reference below -# sits inside a method body and resolves lazily at call time, so no eager -# dependency on the umbrella is needed. +# this file's old-path forwarder. Nothing from the v1 trees is required here +# at all (3.0 readiness): the three requires below used to point at their +# capybara/screenshot/diff/* forwarders, which made this unit depend on the +# compatibility tree it is meant to replace. # DSL includes Capybara::DSL directly below, so it needs the base gem # loaded regardless of what pulled this file in. require "capybara/dsl" -require "capybara/screenshot/diff/config_legacy" -require "capybara/screenshot/diff/drivers" +require "snap_diff/config" +require "snap_diff/drivers" require "snap_diff/comparison" -require "capybara/screenshot/diff/screenshot_matcher" +require "snap_diff/screenshot_matcher" require_relative "screenshot_namer" require_relative "screenshot_assertion" diff --git a/lib/snap_diff/reporters/html.rb b/lib/snap_diff/reporters/html.rb index 951c5659..b24c4e9e 100644 --- a/lib/snap_diff/reporters/html.rb +++ b/lib/snap_diff/reporters/html.rb @@ -8,7 +8,7 @@ # The auto-registration block at the bottom of this file registers with # SnapDiff::Reporting. require "snap_diff/reporting" -require "capybara/screenshot/diff/config_legacy" +require "snap_diff/config" module SnapDiff module Reporters diff --git a/lib/snap_diff/screenshot_matcher.rb b/lib/snap_diff/screenshot_matcher.rb index 5679406f..2dcf0054 100644 --- a/lib/snap_diff/screenshot_matcher.rb +++ b/lib/snap_diff/screenshot_matcher.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "capybara_screenshot_diff/snap_manager" +require "snap_diff/snap_manager" require_relative "screenshoter" require_relative "stable_screenshoter" require_relative "browser_helpers" diff --git a/test/unit/core_tree_has_no_legacy_deps_test.rb b/test/unit/core_tree_has_no_legacy_deps_test.rb index d1dcdd32..593c4c57 100644 --- a/test/unit/core_tree_has_no_legacy_deps_test.rb +++ b/test/unit/core_tree_has_no_legacy_deps_test.rb @@ -92,17 +92,6 @@ class CoreTreeHasNoLegacyDepsTest < ActiveSupport::TestCase "perceptual_threshold: [Capybara::Screenshot::Diff, :perceptual_threshold],", "screenshoter: [Capybara::Screenshot::Diff, :screenshoter],", "manager: [Capybara::Screenshot::Diff, :manager]" - ], - "snap_diff/dsl.rb" => [ - "require \"capybara/screenshot/diff/config_legacy\"", - "require \"capybara/screenshot/diff/drivers\"", - "require \"capybara/screenshot/diff/screenshot_matcher\"" - ], - "snap_diff/reporters/html.rb" => [ - "require \"capybara/screenshot/diff/config_legacy\"" - ], - "snap_diff/screenshot_matcher.rb" => [ - "require \"capybara_screenshot_diff/snap_manager\"" ] }.freeze From 9f310cab44136eb6fe8a6a7897802cce6dfd6cd0 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Sun, 23 Aug 2026 10:51:39 +0200 Subject: [PATCH 05/11] refactor: one file holds the v1 surface, and the core names none of it Three legacy edges were left in the canonical core: - lib/snap_diff/config.rb DEFINED the Capybara::Screenshot::Diff module skeleton so Config::MAPPING could name it, and generated the legacy mattr_accessors. After a 3.0 `git rm` that would have survived: the core would still define a phantom v1 namespace, so adopters' `defined?(Capybara::Screenshot::Diff)` checks would keep passing. - lib/snap_diff.rb required config_legacy.rb and image_compare.rb -- the only reason being that those forwarders happened to install the v1 surface for bare `require "snap_diff"` processes. - SnapDiff.start yields the two legacy holders, so it cannot outlive them. All three move to lib/snap_diff/legacy_shims.rb, which now holds the whole v1 surface as code: const_missing forwarders, CONFIG_MAPPING and its generator, the derived forwarders that were in config_legacy.rb (Screenshot.active?, Diff.configure/.compare/.default_options), SnapDiff.start, and the eager AVAILABLE_DRIVERS / Comparison aliases. config_legacy.rb and image_compare.rb are now requires only. Config keeps the setting list as Config::SETTINGS and names nothing legacy; a new test pins CONFIG_MAPPING.keys == SETTINGS so the split cannot drift into storage with no accessor (or the reverse). Behaviour is unchanged, including the awkward part: bare `require "snap_diff"` still answers Capybara::Screenshot.active?, Diff.default_options, Diff::AVAILABLE_DRIVERS and Diff::Comparison, exactly as it did when it reached through the v1 forwarders (verified before/after in a fresh process). The reverse gate's allowlist is now EMPTY. --- docs/architecture.md | 8 +- lib/capybara/screenshot/diff/config_legacy.rb | 71 ++------ lib/capybara/screenshot/diff/image_compare.rb | 18 +- lib/snap_diff.rb | 34 ++-- lib/snap_diff/config.rb | 138 +++++++--------- lib/snap_diff/legacy_shims.rb | 155 +++++++++++++++++- test/unit/config_default_timing_test.rb | 2 +- .../unit/core_tree_has_no_legacy_deps_test.rb | 48 +----- test/unit/snap_diff_config_test.rb | 23 ++- 9 files changed, 267 insertions(+), 230 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 5bdada16..dc864310 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -125,9 +125,9 @@ Drivers abstract image processing operations. Shared default behavior lives in t | `merge` | Composite images | Not applicable | | `highlight_mask` | Conditional color overlay | Not applicable | -**Auto-detection:** `Utils.detect_available_drivers` tries to load `:vips` first (via `ruby-vips` gem), then `:chunky_png`. The `:auto` driver mode picks the first available. +**Auto-detection:** `SnapDiff::Drivers.detect_available` tries to load `:vips` first (via `ruby-vips` gem), then `:chunky_png`. The `:auto` driver mode picks the first available. `Utils.detect_available_drivers` is the older name and one-lines into it. -**Registry (ADR-008 step 5b):** `SnapDiff::Drivers.loaded` is the canonical driver-class cache — a `name => class` hash filled lazily by `Utils.find_driver_class_for`, and the registration point for custom drivers (the legacy `Capybara::Screenshot::Diff::LOADED_DRIVERS` is an eager same-object alias, so registrations through either land in the same hash). `SnapDiff::Drivers.available` is the canonical read API for the detected list; the value itself still lives on `Capybara::Screenshot::Diff::AVAILABLE_DRIVERS`, which stays the published stubbing point. `SnapDiff::Drivers.for` resolves an options hash to a driver instance. See [Custom drivers](snapdiff.md#custom-drivers). +**Registry (ADR-008 step 5b):** `SnapDiff::Drivers.loaded` is the canonical driver-class cache — a `name => class` hash filled lazily by `Utils.find_driver_class_for`, and the registration point for custom drivers (the legacy `Capybara::Screenshot::Diff::LOADED_DRIVERS` is an eager same-object alias, so registrations through either land in the same hash). `SnapDiff::Drivers.available` is the canonical read API for the detected list, and since the 3.0-readiness pass the value lives with it, as `SnapDiff::Drivers::AVAILABLE_DRIVERS` — that constant is now the published stubbing point, and the legacy `Capybara::Screenshot::Diff::AVAILABLE_DRIVERS` is an eager same-object alias of it. `SnapDiff::Drivers.for` resolves an options hash to a driver instance. See [Custom drivers](snapdiff.md#custom-drivers). ### 6. Difference Region Detection @@ -221,7 +221,9 @@ Test begins Since ADR-008 step 1 the storage ownership is inverted from the original v2 consolidation: **`SnapDiff::Config` (`lib/snap_diff/config.rb`) IS the storage** — one eagerly-created instance, reachable as `SnapDiff.config`, holding every setting as a plain `attr_accessor`. It is the leaf of the config require graph and requires nothing that leads back to either entry point. -The legacy `Capybara::Screenshot.*` / `Capybara::Screenshot::Diff.*` accessors are thin delegators generated from `Config::MAPPING` (both singleton and instance methods, matching what `mattr_accessor` used to define) that forward to that one object. One storage, two views — a write through either surface is visible through the other structurally, not by synchronization. `lib/capybara/screenshot/diff/config_legacy.rb` remains at the old path, but it now installs the delegating surface rather than owning the state. +The legacy `Capybara::Screenshot.*` / `Capybara::Screenshot::Diff.*` accessors are thin delegators generated from `SnapDiff::LegacyShims::CONFIG_MAPPING` (both singleton and instance methods, matching what `mattr_accessor` used to define) that forward to that one object. One storage, two views — a write through either surface is visible through the other structurally, not by synchronization. + +Since the 3.0-readiness pass, `lib/snap_diff/legacy_shims.rb` is the single file that holds the v1 surface as code: the `const_missing` forwarders, `CONFIG_MAPPING` and its generator, the derived forwarders (`Screenshot.active?`, `Diff.configure`, `Diff.default_options`, …) and `SnapDiff.start`. `Config` itself names nothing from the v1 namespaces — it declares its settings in `Config::SETTINGS`, and `LegacyShims::CONFIG_MAPPING` says which legacy holder each one is exposed on (an invariant pinned by `snap_diff_config_test.rb`). `lib/capybara/screenshot/diff/config_legacy.rb` remains at the old path as a pair of requires. The two legacy views are organized into two namespaces: diff --git a/lib/capybara/screenshot/diff/config_legacy.rb b/lib/capybara/screenshot/diff/config_legacy.rb index 29155134..0da6e61a 100644 --- a/lib/capybara/screenshot/diff/config_legacy.rb +++ b/lib/capybara/screenshot/diff/config_legacy.rb @@ -2,67 +2,20 @@ # Legacy Capybara::Screenshot / Capybara::Screenshot::Diff config surface. # -# Since ADR-008 step 1 the storage lives in SnapDiff::Config -- the require -# leaf of the config graph (see its own header) -- and since step 7b the -# DERIVED values (active?, screenshot_area, default_options) live there -# too. snap_diff/config.rb also generates the old accessor names as thin -# delegators from SnapDiff::Config::MAPPING, so nothing but forwarders is -# left here. The v1 surface (Capybara::Screenshot.window_size = ..., -# Diff.configure { ... }, Diff.compare) keeps working unchanged: one -# storage, two views. +# Nothing but requires is left here. The storage is SnapDiff::Config +# (ADR-008 step 1, the require leaf of the config graph); the derived values +# (active?, screenshot_area, default_options) live there too since step 7b; +# and the old accessor names, Diff.configure/.compare, SnapDiff.start and +# the AVAILABLE_DRIVERS alias are generated by snap_diff/legacy_shims -- the +# one file that holds the v1 surface as code, so that the canonical core +# needs nothing from this tree and 3.0 can delete both together. The v1 +# surface (Capybara::Screenshot.window_size = ..., Diff.configure { ... }, +# Diff.compare) keeps working unchanged: one storage, two views. # # Load order: requiring snap_diff/config first also eagerly evaluates the # require-time defaults (ENV["CI"] for fail_if_new, Rails.root/pwd for # root) at this same load moment, exactly when the old mattr_accessor -# default blocks used to run. snap_diff/config never requires back here, -# so the graph stays acyclic. +# default blocks used to run. Neither file requires back here, so the graph +# stays acyclic. require "snap_diff/config" -# AVAILABLE_DRIVERS below aliases SnapDiff::Drivers::AVAILABLE_DRIVERS at -# class-body eval time, so Drivers must be a real, already-loaded module -# before this module body runs. -require "snap_diff/drivers" - -module Capybara - module Screenshot - class << self - def active? - SnapDiff.config.active? - end - - def screenshot_area - SnapDiff.config.screenshot_area - end - - def screenshot_area_abs - SnapDiff.config.screenshot_area_abs - end - end - - # Module to track screenshot changes - module Diff - AVAILABLE_DRIVERS = SnapDiff::Drivers::AVAILABLE_DRIVERS - - # Configure screenshot and diff settings in one block. - # - # Capybara::Screenshot::Diff.configure do |screenshot, diff| - # screenshot.window_size = [1280, 1024] - # screenshot.stability_time_limit = 1 - # diff.driver = :vips - # diff.tolerance = 0.0005 - # end - # The bare `yield` (rather than an explicit &block) keeps this - # method's published arity byte-identical to what it always had. - def self.configure - SnapDiff.start { |screenshot, diff| yield screenshot, diff } - end - - def self.compare(baseline_path, current_path, **options) - SnapDiff.compare(baseline_path, current_path, **options) - end - - def self.default_options - SnapDiff.config.default_options - end - end - end -end +require "snap_diff/legacy_shims" diff --git a/lib/capybara/screenshot/diff/image_compare.rb b/lib/capybara/screenshot/diff/image_compare.rb index 52649cf7..19863e3e 100644 --- a/lib/capybara/screenshot/diff/image_compare.rb +++ b/lib/capybara/screenshot/diff/image_compare.rb @@ -8,15 +8,9 @@ require "snap_diff/comparison" require "snap_diff/legacy_shims" -# Deliberately EAGER and silent (v2 step 6 exception): Comparison is a -# documented user-facing struct, so adopters feature-detect it with -# defined?/const_defined? -- neither of which triggers const_missing, so a -# lazy shim reported it permanently absent. See snap_diff/legacy_shims.rb -# for the full exception list. -module Capybara - module Screenshot - module Diff - Comparison = SnapDiff::Comparison::Images - end - end -end +# Capybara::Screenshot::Diff::Comparison (the images-holder struct) is a +# documented user-facing name that adopters feature-detect with +# defined?/const_defined?, so it is assigned EAGERLY rather than shimmed -- +# const_defined? never triggers const_missing. That assignment now lives in +# snap_diff/legacy_shims (required above), with the rest of the v1 surface, +# so `require "snap_diff"` alone provides it too. diff --git a/lib/snap_diff.rb b/lib/snap_diff.rb index 3c126f79..1bc84e7b 100644 --- a/lib/snap_diff.rb +++ b/lib/snap_diff.rb @@ -29,20 +29,25 @@ def self.assert_single_gem!(loaded_specs = Gem.loaded_specs) # This lean entry must never load the umbrella "capybara_screenshot_diff" # -- snap_diff_test.rb's "bare require never loads the umbrella" guard -# enforces it -- so nothing required below may reach back here. +# enforces it -- so nothing required below may reach back here. None of +# these requires reaches into lib/capybara* at all, so the canonical entry +# point is exactly what 3.0 keeps. # -# The image_compare forwarder (not "snap_diff/comparison" directly) is -# deliberate: it installs snap_diff/legacy_shims, so the old -# Capybara::Screenshot::Diff constants stay resolvable, with deprecation -# warnings, in processes that only ever require "snap_diff". # "capybara/dsl" is needed directly (not just transitively) so # `Capybara.default_max_wait_time` in Config#default_options resolves even # when "snap_diff" is required standalone (SnapDiffTest's # "standalone-loadable in a fresh process" regression test). +# +# snap_diff/legacy_shims is deliberate and is the ONE line here that 3.0 +# drops: it carries the whole v1 surface (const_missing forwarders, the old +# mattr_accessors, SnapDiff.start), so a process that only ever requires +# "snap_diff" still resolves the old Capybara::Screenshot::Diff names -- +# with deprecation warnings -- exactly as it did when this file reached +# through the capybara/screenshot/diff/* forwarders to get them. require "capybara/dsl" -require "capybara/screenshot/diff/config_legacy" -require "capybara/screenshot/diff/image_compare" require "snap_diff/config" +require "snap_diff/comparison" +require "snap_diff/legacy_shims" require "snap_diff/version" # SnapDiff.session/.reset/.pending_screenshots_message are part of the # documented core surface (docs/snapdiff.md object map lists them with no @@ -63,18 +68,9 @@ def self.compare(baseline_path, current_path, **options) Comparison.new(current_path, baseline_path, config.default_options.merge(options)) end - # v1-style configuration: yields the two legacy accessor holders - # (+Capybara::Screenshot+, +Capybara::Screenshot::Diff+). Canonical home; - # +Capybara::Screenshot::Diff.configure+ forwards here, and both names - # stay identical in call shape. - # - # SnapDiff.start do |screenshot, diff| - # screenshot.window_size = [1280, 1024] - # diff.tolerance = 0.0005 - # end - def self.start - yield Capybara::Screenshot, Capybara::Screenshot::Diff - end + # SnapDiff.start -- the v1-shaped two-holder config block -- is defined in + # snap_diff/legacy_shims (required above), because the holders it yields + # are the v1 surface and it cannot outlive them. # Forward-looking configuration: yields the single consolidated # {SnapDiff::Config} object instead of the two old holders. Same diff --git a/lib/snap_diff/config.rb b/lib/snap_diff/config.rb index 7f9617ba..54c46877 100644 --- a/lib/snap_diff/config.rb +++ b/lib/snap_diff/config.rb @@ -4,17 +4,10 @@ # This file is the LEAF of the config require graph (ADR-008 step 1): # config_legacy.rb requires it, so it must never require config_legacy nor -# anything that leads back to either entry point. The MAPPING below needs -# the legacy module constants to exist at class-body eval time, so the empty -# skeleton is predefined here (same technique as legacy_shims.rb); -# config_legacy.rb reopens these modules and installs the delegating -# accessors from MAPPING. -module Capybara - module Screenshot - module Diff - end - end -end +# anything that leads back to either entry point. It also names nothing from +# the v1 trees at all (3.0 readiness): which legacy accessor each setting is +# exposed as is snap_diff/legacy_shims' business, and that file is deleted +# together with lib/capybara* -- see LegacyShims::CONFIG_MAPPING. # Referenced by Config#initialize (screenshoter/manager defaults), which # runs at the eager Config.new at the bottom of this file, so they must be @@ -30,9 +23,9 @@ module SnapDiff # Storage ownership (ADR-008 step 1, inverted from the original v2 # consolidation): Config IS the single storage. The legacy accessors on # +Capybara::Screenshot+ / +Capybara::Screenshot::Diff+ are thin - # delegators installed by config_legacy.rb from {MAPPING} -- one storage, - # two views, so a write through either surface is visible through the - # other structurally, not by synchronization. + # delegators generated by snap_diff/legacy_shims from its CONFIG_MAPPING + # -- one storage, two views, so a write through either surface is visible + # through the other structurally, not by synchronization. # # Default timing contract (pinned by config_default_timing_test.rb): # every default below is evaluated ONCE, in #initialize, which runs at @@ -44,57 +37,57 @@ module SnapDiff # not storage at all: it stays a method-body read of # +Capybara.default_max_wait_time+ in +#default_options+. class Config - # config attr name => [legacy module, legacy accessor name]. + # Every setting this object stores, in the order the two legacy holders + # used to declare them. # - # The two names differ only for +screenshot_enabled+: - # +Capybara::Screenshot.enabled+ and +Capybara::Screenshot::Diff.enabled+ - # are independent settings (see +Capybara::Screenshot.active?+, which - # reads both) that happen to share a bare name in their own modules. A - # flat Config can't expose two attributes both called +enabled+, so the - # Screenshot-side one is renamed here; Diff's keeps the bare +enabled+ - # name since it's the one most existing configuration touches directly. - MAPPING = { - # Capybara::Screenshot - add_driver_path: [Capybara::Screenshot, :add_driver_path], - add_os_path: [Capybara::Screenshot, :add_os_path], - blur_active_element: [Capybara::Screenshot, :blur_active_element], - screenshot_enabled: [Capybara::Screenshot, :enabled], - hide_caret: [Capybara::Screenshot, :hide_caret], - disable_animations: [Capybara::Screenshot, :disable_animations], - root: [Capybara::Screenshot, :root], - stability_time_limit: [Capybara::Screenshot, :stability_time_limit], - window_size: [Capybara::Screenshot, :window_size], - save_path: [Capybara::Screenshot, :save_path], - use_lfs: [Capybara::Screenshot, :use_lfs], - screenshot_format: [Capybara::Screenshot, :screenshot_format], - capybara_screenshot_options: [Capybara::Screenshot, :capybara_screenshot_options], - # Capybara::Screenshot::Diff - delayed: [Capybara::Screenshot::Diff, :delayed], - area_size_limit: [Capybara::Screenshot::Diff, :area_size_limit], - fail_if_new: [Capybara::Screenshot::Diff, :fail_if_new], - pending_if_new: [Capybara::Screenshot::Diff, :pending_if_new], - fail_on_difference: [Capybara::Screenshot::Diff, :fail_on_difference], - color_distance_limit: [Capybara::Screenshot::Diff, :color_distance_limit], - enabled: [Capybara::Screenshot::Diff, :enabled], - shift_distance_limit: [Capybara::Screenshot::Diff, :shift_distance_limit], - skip_area: [Capybara::Screenshot::Diff, :skip_area], - driver: [Capybara::Screenshot::Diff, :driver], - tolerance: [Capybara::Screenshot::Diff, :tolerance], - perceptual_threshold: [Capybara::Screenshot::Diff, :perceptual_threshold], - screenshoter: [Capybara::Screenshot::Diff, :screenshoter], - manager: [Capybara::Screenshot::Diff, :manager] - }.freeze - - attr_accessor(*(MAPPING.keys - [:root])) + # +screenshot_enabled+ is the one name that differs from its legacy + # spelling: +Capybara::Screenshot.enabled+ and + # +Capybara::Screenshot::Diff.enabled+ are independent settings (see + # {#active?}, which reads both) that happened to share a bare name in + # their own modules. A flat Config can't expose two attributes both + # called +enabled+, so the Screenshot-side one is renamed here; Diff's + # keeps the bare +enabled+ name since it's the one most existing + # configuration touches directly. + SETTINGS = %i[ + add_driver_path + add_os_path + blur_active_element + screenshot_enabled + hide_caret + disable_animations + root + stability_time_limit + window_size + save_path + use_lfs + screenshot_format + capybara_screenshot_options + delayed + area_size_limit + fail_if_new + pending_if_new + fail_on_difference + color_distance_limit + enabled + shift_distance_limit + skip_area + driver + tolerance + perceptual_threshold + screenshoter + manager + ].freeze + + attr_accessor(*(SETTINGS - [:root])) attr_reader :root def initialize - # Every mapped setting gets its ivar up front (nil-defaulted ones - # included) so the full set always exists -- test_helper's per-test - # isolation snapshots/restores config by instance variable, and an - # ivar that only appears on first write would escape that snapshot - # and leak between tests. - MAPPING.each_key { |key| instance_variable_set(:"@#{key}", nil) } + # Every setting gets its ivar up front (nil-defaulted ones included) + # so the full set always exists -- test_helper's per-test isolation + # snapshots/restores config by instance variable, and an ivar that + # only appears on first write would escape that snapshot and leak + # between tests. + SETTINGS.each { |key| instance_variable_set(:"@#{key}", nil) } # Capybara::Screenshot side. @blur_active_element = true @hide_caret = true @@ -124,7 +117,7 @@ def root=(path) # on the legacy modules; those now one-line forward here. # ex +Capybara::Screenshot.active?+. The two +enabled+ settings are - # independent (see {MAPPING}): the Screenshot-side one wins whenever it + # independent (see {SETTINGS}): the Screenshot-side one wins whenever it # was set at all, and only a nil there falls through to the Diff-side # one. def active? @@ -176,25 +169,4 @@ def default_options def self.config @config end - - # Installs the old mattr_accessor surface onto the legacy modules, - # delegating to the single storage above. mattr_accessor used to define - # both singleton and instance accessors (the instance ones are what - # `include Capybara::Screenshot::Diff` picks up), so both are installed. - # root keeps its historical asymmetry -- readable everywhere, writable - # only at module level (it was mattr_reader plus a custom module-level - # writer) -- with the Pathname coercion living in Config#root=. - # - # Generated here rather than in config_legacy.rb (ADR-008 step 7b) for - # the same reason legacy_shims.rb generates the legacy constants here: - # the generator is code, and the v1 trees must stay alias-only so 3.0 is - # a `git rm`. Same technique, same side of the fence. - Config::MAPPING.each do |name, (mod, mattr)| - [mod, mod.singleton_class].each do |target| - target.define_method(mattr) { SnapDiff.config.public_send(name) } - next if name == :root && target == mod - - target.define_method(:"#{mattr}=") { |value| SnapDiff.config.public_send(:"#{name}=", value) } - end - end end diff --git a/lib/snap_diff/legacy_shims.rb b/lib/snap_diff/legacy_shims.rb index 0341ae6d..ab69e72b 100644 --- a/lib/snap_diff/legacy_shims.rb +++ b/lib/snap_diff/legacy_shims.rb @@ -1,8 +1,27 @@ # frozen_string_literal: true +require "snap_diff/comparison" +require "snap_diff/config" require "snap_diff/deprecation" require "snap_diff/drivers" +# THE v1 compatibility surface, in one file -- and the whole of it that is +# code. lib/capybara* is alias-only by contract +# (legacy_tree_is_alias_only_test.rb) and the canonical core names nothing +# from it (core_tree_has_no_legacy_deps_test.rb), so this file plus those +# trees is exactly what 3.0 deletes. +# +# Three things live here: +# 1. the const_missing forwarders for the pre-v2 namespaces (below); +# 2. CONFIG_MAPPING -- the old mattr_accessor surface, generated as thin +# delegators onto SnapDiff.config (which owns the storage); +# 3. the derived/config forwarders that used to sit in config_legacy.rb +# (Screenshot.active?, Diff.configure, SnapDiff.start, ...). +# +# 2 and 3 moved here so `require "snap_diff"` can keep offering the full v1 +# surface -- as it always has -- without the core requiring anything from +# lib/capybara/. +# # const_missing-based forwarders for the pre-v2 namespaces. Every old-name # lookup below resolves -- lazily -- to the exact # same object as its SnapDiff:: replacement (identity pinned by @@ -34,9 +53,19 @@ # canonical SnapDiff::Drivers.loaded -- a lazy warn-once shim could not # keep a mutable alias, and warning on a supported registration surface # would be noise. Assigned eagerly below. -# - Diff::AVAILABLE_DRIVERS: stays a real constant defined by -# config_legacy.rb (detection runs at that load moment); -# SnapDiff::Drivers.available is the canonical reader. +# - Diff::AVAILABLE_DRIVERS: stays a real constant, aliased in +# config_legacy.rb from the canonical SnapDiff::Drivers::AVAILABLE_DRIVERS +# (same object; SnapDiff::Drivers.available is the canonical reader). + +# The v1 namespaces, predefined empty so CONFIG_MAPPING can name them at +# class-body eval time. Everything below reopens them. +module Capybara + module Screenshot + module Diff + end + end +end + module SnapDiff # @api private module LegacyShims @@ -54,14 +83,132 @@ def self.install(namespace, old_prefix, mapping) Object.const_get(target) end end + + # config attr name => [legacy module, legacy accessor name]. + # The keys are exactly SnapDiff::Config::SETTINGS; this hash only says + # which of the two legacy holders each one used to hang off, and under + # what name (only +screenshot_enabled+ differs -- see Config::SETTINGS). + CONFIG_MAPPING = { + # Capybara::Screenshot + add_driver_path: [Capybara::Screenshot, :add_driver_path], + add_os_path: [Capybara::Screenshot, :add_os_path], + blur_active_element: [Capybara::Screenshot, :blur_active_element], + screenshot_enabled: [Capybara::Screenshot, :enabled], + hide_caret: [Capybara::Screenshot, :hide_caret], + disable_animations: [Capybara::Screenshot, :disable_animations], + root: [Capybara::Screenshot, :root], + stability_time_limit: [Capybara::Screenshot, :stability_time_limit], + window_size: [Capybara::Screenshot, :window_size], + save_path: [Capybara::Screenshot, :save_path], + use_lfs: [Capybara::Screenshot, :use_lfs], + screenshot_format: [Capybara::Screenshot, :screenshot_format], + capybara_screenshot_options: [Capybara::Screenshot, :capybara_screenshot_options], + # Capybara::Screenshot::Diff + delayed: [Capybara::Screenshot::Diff, :delayed], + area_size_limit: [Capybara::Screenshot::Diff, :area_size_limit], + fail_if_new: [Capybara::Screenshot::Diff, :fail_if_new], + pending_if_new: [Capybara::Screenshot::Diff, :pending_if_new], + fail_on_difference: [Capybara::Screenshot::Diff, :fail_on_difference], + color_distance_limit: [Capybara::Screenshot::Diff, :color_distance_limit], + enabled: [Capybara::Screenshot::Diff, :enabled], + shift_distance_limit: [Capybara::Screenshot::Diff, :shift_distance_limit], + skip_area: [Capybara::Screenshot::Diff, :skip_area], + driver: [Capybara::Screenshot::Diff, :driver], + tolerance: [Capybara::Screenshot::Diff, :tolerance], + perceptual_threshold: [Capybara::Screenshot::Diff, :perceptual_threshold], + screenshoter: [Capybara::Screenshot::Diff, :screenshoter], + manager: [Capybara::Screenshot::Diff, :manager] + }.freeze + + # Installs the old mattr_accessor surface onto the legacy modules, + # delegating to the single storage in SnapDiff.config. mattr_accessor + # used to define both singleton and instance accessors (the instance + # ones are what `include Capybara::Screenshot::Diff` picks up), so both + # are installed. root keeps its historical asymmetry -- readable + # everywhere, writable only at module level (it was mattr_reader plus a + # custom module-level writer) -- with the Pathname coercion living in + # Config#root=. + def self.install_config_accessors + CONFIG_MAPPING.each do |name, (mod, mattr)| + [mod, mod.singleton_class].each do |target| + target.define_method(mattr) { SnapDiff.config.public_send(name) } + next if name == :root && target == mod + + target.define_method(:"#{mattr}=") { |value| SnapDiff.config.public_send(:"#{name}=", value) } + end + end + end + end +end + +SnapDiff::LegacyShims.install_config_accessors + +module SnapDiff + # v1-style configuration: yields the two legacy accessor holders + # (+Capybara::Screenshot+, +Capybara::Screenshot::Diff+) exactly as + # +Capybara::Screenshot::Diff.configure+ always has -- and, since ADR-008 + # step 7b, this is where that yield actually happens; Diff.configure + # forwards here. Both names stay identical in call shape. + # + # SnapDiff.start do |screenshot, diff| + # screenshot.window_size = [1280, 1024] + # diff.tolerance = 0.0005 + # end + # + # Defined in this file, not snap_diff.rb, because the two holders it + # yields ARE the v1 surface: it cannot outlive them. The consolidated + # shape, SnapDiff.configure, is the canonical one and lives in the core. + def self.start + yield Capybara::Screenshot, Capybara::Screenshot::Diff end end module Capybara module Screenshot + # Derived config, ex config_legacy.rb: one-line forwarders onto the + # canonical implementations in SnapDiff::Config (ADR-008 step 7b). + class << self + def active? + SnapDiff.config.active? + end + + def screenshot_area + SnapDiff.config.screenshot_area + end + + def screenshot_area_abs + SnapDiff.config.screenshot_area_abs + end + end + module Diff - # EAGER same-object alias of the canonical driver cache (see header). + # EAGER same-object aliases of canonical values (see header for why + # each one is eager rather than a warn-once const_missing shim). LOADED_DRIVERS = SnapDiff::Drivers.loaded + AVAILABLE_DRIVERS = SnapDiff::Drivers::AVAILABLE_DRIVERS + Comparison = SnapDiff::Comparison::Images + + # Configure screenshot and diff settings in one block. + # + # Capybara::Screenshot::Diff.configure do |screenshot, diff| + # screenshot.window_size = [1280, 1024] + # screenshot.stability_time_limit = 1 + # diff.driver = :vips + # diff.tolerance = 0.0005 + # end + # The bare `yield` (rather than an explicit &block) keeps this + # method's published arity byte-identical to what it always had. + def self.configure + SnapDiff.start { |screenshot, diff| yield screenshot, diff } + end + + def self.compare(baseline_path, current_path, **options) + SnapDiff.compare(baseline_path, current_path, **options) + end + + def self.default_options + SnapDiff.config.default_options + end end end end diff --git a/test/unit/config_default_timing_test.rb b/test/unit/config_default_timing_test.rb index 3bb376e0..53cd535c 100644 --- a/test/unit/config_default_timing_test.rb +++ b/test/unit/config_default_timing_test.rb @@ -63,7 +63,7 @@ def check_both(name, expected, mod, mattr) Dir.chdir(Dir.tmpdir) # 1) Every mapped setting reads the same through both surfaces. - SnapDiff::Config::MAPPING.each do |name, (mod, mattr)| + SnapDiff::LegacyShims::CONFIG_MAPPING.each do |name, (mod, mattr)| check("SnapDiff.config.#{name} vs #{mod}.#{mattr}", mod.public_send(mattr), SnapDiff.config.public_send(name)) end diff --git a/test/unit/core_tree_has_no_legacy_deps_test.rb b/test/unit/core_tree_has_no_legacy_deps_test.rb index 593c4c57..7a039efd 100644 --- a/test/unit/core_tree_has_no_legacy_deps_test.rb +++ b/test/unit/core_tree_has_no_legacy_deps_test.rb @@ -52,48 +52,12 @@ class CoreTreeHasNoLegacyDepsTest < ActiveSupport::TestCase # file-level, on purpose: a file with one blessed line must still fail on # the second. # - # Seeded with every core->legacy edge that existed the day the gate was - # written (64 of them), so the gate lands green and every later commit can - # only take entries away. The target is an empty hash. It must never grow: - # a new entry is a decision to keep a core->legacy edge across the 3.0 - # deletion, so it needs a written reason here AND an ADR-008 update -- - # never just a red build turned green. - ALLOWED = { - "snap_diff.rb" => [ - "require \"capybara/screenshot/diff/config_legacy\"", - "require \"capybara/screenshot/diff/image_compare\"", - "yield Capybara::Screenshot, Capybara::Screenshot::Diff" - ], - "snap_diff/config.rb" => [ - "add_driver_path: [Capybara::Screenshot, :add_driver_path],", - "add_os_path: [Capybara::Screenshot, :add_os_path],", - "blur_active_element: [Capybara::Screenshot, :blur_active_element],", - "screenshot_enabled: [Capybara::Screenshot, :enabled],", - "hide_caret: [Capybara::Screenshot, :hide_caret],", - "disable_animations: [Capybara::Screenshot, :disable_animations],", - "root: [Capybara::Screenshot, :root],", - "stability_time_limit: [Capybara::Screenshot, :stability_time_limit],", - "window_size: [Capybara::Screenshot, :window_size],", - "save_path: [Capybara::Screenshot, :save_path],", - "use_lfs: [Capybara::Screenshot, :use_lfs],", - "screenshot_format: [Capybara::Screenshot, :screenshot_format],", - "capybara_screenshot_options: [Capybara::Screenshot, :capybara_screenshot_options],", - "delayed: [Capybara::Screenshot::Diff, :delayed],", - "area_size_limit: [Capybara::Screenshot::Diff, :area_size_limit],", - "fail_if_new: [Capybara::Screenshot::Diff, :fail_if_new],", - "pending_if_new: [Capybara::Screenshot::Diff, :pending_if_new],", - "fail_on_difference: [Capybara::Screenshot::Diff, :fail_on_difference],", - "color_distance_limit: [Capybara::Screenshot::Diff, :color_distance_limit],", - "enabled: [Capybara::Screenshot::Diff, :enabled],", - "shift_distance_limit: [Capybara::Screenshot::Diff, :shift_distance_limit],", - "skip_area: [Capybara::Screenshot::Diff, :skip_area],", - "driver: [Capybara::Screenshot::Diff, :driver],", - "tolerance: [Capybara::Screenshot::Diff, :tolerance],", - "perceptual_threshold: [Capybara::Screenshot::Diff, :perceptual_threshold],", - "screenshoter: [Capybara::Screenshot::Diff, :screenshoter],", - "manager: [Capybara::Screenshot::Diff, :manager]" - ] - }.freeze + # EMPTY. It was seeded with all 64 core->legacy edges that existed the day + # the gate was written, and every one of them is gone. Keep it empty: an + # entry is a decision to keep a core->legacy edge across the 3.0 deletion, + # so it needs a written reason here AND an ADR-008 update -- never just a + # red build turned green. + ALLOWED = {}.freeze test "no core file requires or references the v1 namespaces" do refute_empty CORE_FILES, "core glob matched nothing -- the gate would pass vacuously" diff --git a/test/unit/snap_diff_config_test.rb b/test/unit/snap_diff_config_test.rb index e1d2dfc8..5663d26f 100644 --- a/test/unit/snap_diff_config_test.rb +++ b/test/unit/snap_diff_config_test.rb @@ -23,14 +23,14 @@ def config # setting -- a future `mattr_accessor :foo` (active_support's ext is # one require away) or hand-rolled writer would create unmapped # storage invisible to SnapDiff.config; - # (b) SnapDiff.config stores exactly one ivar per MAPPING key -- catches + # (b) SnapDiff.config stores exactly one ivar per declared setting -- catches # both an unmapped ivar sneaking into Config and a mapped setting # whose ivar is missing (which would also silently escape # test_helper's per-test ivar snapshot/restore). NON_CONFIG_WRITERS = [].freeze # currently no non-config writer API on the legacy modules - test "every legacy singleton writer is covered by SnapDiff::Config::MAPPING" do - covered = SnapDiff::Config::MAPPING.values + test "every legacy singleton writer is covered by SnapDiff::LegacyShims::CONFIG_MAPPING" do + covered = SnapDiff::LegacyShims::CONFIG_MAPPING.values [Capybara::Screenshot, Capybara::Screenshot::Diff].each do |mod| writers = mod.singleton_class.public_instance_methods(false).grep(/=\z/) - NON_CONFIG_WRITERS @@ -41,20 +41,29 @@ def config assert_includes covered, [mod, mattr], "#{mod}.#{writer} is a config writer with no SnapDiff::Config mapping. " \ - "Add an entry to Config::MAPPING (rename the key if `#{mattr}` collides " \ + "Add an entry to LegacyShims::CONFIG_MAPPING (rename the key if `#{mattr}` collides " \ "with an existing one, as `enabled` does), or add it to NON_CONFIG_WRITERS " \ "if it is deliberately not a config setting." end end end - test "SnapDiff.config stores exactly one ivar per MAPPING key" do - assert_equal SnapDiff::Config::MAPPING.keys.map { |k| :"@#{k}" }.sort, + # The two halves of the split are only safe while they agree: Config + # declares the settings and knows nothing about the legacy holders, + # LegacyShims says which holder each one is exposed on. A setting in one + # and not the other is either storage with no v1 accessor or a v1 + # accessor delegating to a setting that does not exist. + test "LegacyShims::CONFIG_MAPPING covers exactly Config::SETTINGS" do + assert_equal SnapDiff::Config::SETTINGS, SnapDiff::LegacyShims::CONFIG_MAPPING.keys + end + + test "SnapDiff.config stores exactly one ivar per declared setting" do + assert_equal SnapDiff::Config::SETTINGS.map { |k| :"@#{k}" }.sort, SnapDiff.config.instance_variables.sort end test "every mapped setting is readable via config and equal to its mattr_accessor's value" do - SnapDiff::Config::MAPPING.each do |name, (mod, mattr)| + SnapDiff::LegacyShims::CONFIG_MAPPING.each do |name, (mod, mattr)| expected = mod.public_send(mattr) actual = config.public_send(name) From d76d8e7319f32cee0c934ea7eb9aab200fb16629 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Sun, 23 Aug 2026 10:53:40 +0200 Subject: [PATCH 06/11] test: alias-only gate demands a real single-expression forwarder The rule was `body.include?("SnapDiff")` plus a following `end`, which accepted anything that merely mentioned the canonical namespace: def x SnapDiff.config.a ? b : c # a conditional -- real behaviour end def x; SnapDiff.config.a; File.write(...); end # two statements Both are now rejected: a semicolon is never alias-shaped (it is how several statements, or a whole def, hide inside one "line"), and a forwarder body must match the whole of FORWARDER_BODY -- one method-call chain rooted at SnapDiff, at most one argument list and one block. Mutation-checked with exactly those two shapes plus a genuine forwarder, which still passes. --- test/unit/legacy_tree_is_alias_only_test.rb | 46 +++++++++++++++------ 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/test/unit/legacy_tree_is_alias_only_test.rb b/test/unit/legacy_tree_is_alias_only_test.rb index 2838828c..a459ab7f 100644 --- a/test/unit/legacy_tree_is_alias_only_test.rb +++ b/test/unit/legacy_tree_is_alias_only_test.rb @@ -23,19 +23,35 @@ class LegacyTreeIsAliasOnlyTest < ActiveSupport::TestCase # the v1 trees holds logic any more. config_legacy.rb was the last entry; # step 7b moved its derived config (.active? precedence, .screenshot_area # path assembly, .default_options incl. the vips tolerance literal) into - # SnapDiff::Config, and the Config::MAPPING accessor generator into - # snap_diff/config.rb alongside it, leaving only one-line forwarders. + # SnapDiff::Config, and the 3.0-readiness pass moved the remaining + # forwarders and the legacy accessor generator into + # snap_diff/legacy_shims.rb -- the one file that holds the v1 surface as + # code, and that 3.0 deletes together with these trees. There is not a + # single `def` left here. # # Keep it empty. Adding an entry back is a decision to keep behaviour on # the v1 side of the 3.0 deletion, so it needs a written reason here AND # an ADR-008 update -- never just to turn a red build green. - # - # (Diff::AVAILABLE_DRIVERS still lives in config_legacy.rb, but as a bare - # constant assignment it is alias-shaped and needs no exemption. It stays - # there on purpose -- see #227: test_helper reads it at boot and - # image_compare_test stubs it as the published no-drivers hook.) ALLOWED_WITH_CODE = {}.freeze + # A `def` in these trees is only acceptable as a THREE-line forwarder -- + # signature, ONE delegating expression, `end` -- and this is that + # expression: a single method-call chain rooted at SnapDiff, with at most + # one argument list and one block. + # + # The rule used to be `body.include?("SnapDiff")`, which waved through + # real behaviour hiding behind a mention of the canonical namespace: + # `SnapDiff.config.a ? b : c` is a conditional, and (with the `;` hole + # closed below) `SnapDiff.config.a; something_else` was two statements. + # Anchoring the whole body rejects both -- neither can be consumed by the + # chain, so neither matches. + FORWARDER_BODY = /\A + SnapDiff(::[A-Z]\w*)* # SnapDiff, SnapDiff::Reporters, ... + (\.[a-z_]\w*[?!]?)+ # .config.active?, .compare, ... + (\(.*\))? # at most one argument list + (\s*\{.*\})? # at most one block (yield-through forwarders) + \z/x + # Shapes that are pure compatibility plumbing rather than behaviour. ALIAS_SHAPES = /\A( require(_relative)?\s | @@ -78,8 +94,8 @@ def allowed?(file) end # Walks the file's significant lines. A `def` is only acceptable when its - # whole body is a single line delegating into SnapDiff; anything else must - # match ALIAS_SHAPES. + # whole body is one FORWARDER_BODY expression; anything else must match + # ALIAS_SHAPES. def offences(file) rel = file.relative_path_from(LIB) lines = significant_lines(file) @@ -89,10 +105,16 @@ def offences(file) while index < lines.length line = lines[index] - if line.start_with?("def ") + if line.include?(";") + # A semicolon is how several statements -- or an entire + # `def x; body; end` -- hide inside one "line", which would then be + # judged as a single line. Never alias-shaped, whatever it says. + found << "#{rel}: `#{line}` puts more than one statement on a line" + index += 1 + elsif line.start_with?("def ") body, terminator = lines[index + 1], lines[index + 2] - unless body.to_s.include?("SnapDiff") && terminator == "end" - found << "#{rel}: `#{line}` is not a one-line forwarder into SnapDiff" + unless FORWARDER_BODY.match?(body.to_s) && terminator == "end" + found << "#{rel}: `#{line}` is not a single-expression forwarder into SnapDiff" end index += 3 else From 37f63b5bb0373e272ffd09fc8a557d3ef9089f33 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:01:43 +0200 Subject: [PATCH 07/11] build: gemspec resolves the version from snap_diff/version The gemspec required capybara/screenshot/diff/version and read Capybara::Screenshot::Diff::VERSION -- a build-time dependency on a file 3.0 deletes, so `gem build` would have failed the moment it did. Same value (the legacy constant is an alias of SnapDiff::VERSION), one fewer 3.0 blocker. The legacy constant stays for adopters who read it. Found by the 3.0 dry run; fixed here because it is one line and in scope. --- capybara-screenshot-diff.gemspec | 4 ++-- lib/capybara/screenshot/diff/version.rb | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/capybara-screenshot-diff.gemspec b/capybara-screenshot-diff.gemspec index 9cf92082..fea3dbc2 100644 --- a/capybara-screenshot-diff.gemspec +++ b/capybara-screenshot-diff.gemspec @@ -2,11 +2,11 @@ lib = File.expand_path("lib", __dir__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) -require "capybara/screenshot/diff/version" +require "snap_diff/version" Gem::Specification.new do |spec| spec.name = "capybara-screenshot-diff" - spec.version = Capybara::Screenshot::Diff::VERSION + spec.version = SnapDiff::VERSION spec.authors = ["Uwe Kubosch"] spec.email = ["uwe@kubosch.no"] spec.summary = "Track your GUI changes with diff assertions" diff --git a/lib/capybara/screenshot/diff/version.rb b/lib/capybara/screenshot/diff/version.rb index 01022d9f..e6f75ee7 100644 --- a/lib/capybara/screenshot/diff/version.rb +++ b/lib/capybara/screenshot/diff/version.rb @@ -2,10 +2,12 @@ require "snap_diff/version" -# Deliberately EAGER and silent (v2 step 6 exception): the gemspec resolves -# Capybara::Screenshot::Diff::VERSION at build time, so a lazy warning shim -# would make every `gem build` warn. See snap_diff/legacy_shims.rb for the -# full exception list. +# Deliberately EAGER and silent (v2 step 6 exception): this is a documented +# name that adopters read directly, and const_defined? never triggers +# const_missing. The gemspec used to resolve it at build time (which a lazy +# warning shim would have made warn on every `gem build`); it reads +# SnapDiff::VERSION now, so this file is pure compatibility. See +# snap_diff/legacy_shims.rb for the full exception list. module Capybara module Screenshot module Diff From ac2ae6835b031ffbd9baa63232215248e0915ba4 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:30:58 +0200 Subject: [PATCH 08/11] fix: legacy VERSION vanished from six entry points Capybara::Screenshot::Diff::VERSION raised NameError -- and defined? returned nil -- under "snap_diff", snap_diff/dsl, both integrations, snap_diff/static and, worst, the LEGACY capybara_screenshot_diff/dsl. Cause: the constant was assigned by capybara/screenshot/diff/version.rb, and the 3.0-readiness pass stopped the core requiring that forwarder. legacy_shims deliberately omits VERSION from its const_missing map -- it is one of the documented eager exceptions -- so nothing filled the gap and adopters' `defined?` feature detection silently went false. Assign it in legacy_shims next to the other eager aliases: that file is required by every entry point, canonical and legacy, so it is the only place the eager exceptions can actually be eager. version.rb drops the assignment (a second one is a duplicate-constant warning, not a safety net) and becomes a require. The suite could not have caught it: EAGER_USER_FACING was only probed for the four legacy entry points, and VERSION was not in the list at all. New EAGER_EVERYWHERE probe covers all 15 entry points including capybara_screenshot_diff/dsl, which is in none of the existing lists -- exactly why it was the legacy entry that lost VERSION unnoticed. Red without the alias, naming all six. Also corrects two now-false claims in the legacy_shims header: it said AVAILABLE_DRIVERS was aliased in config_legacy.rb (it is aliased in this file) and that VERSION/Comparison were defined by their own forwarder files (nothing requires those anymore). --- lib/capybara/screenshot/diff/version.rb | 23 ++++++--------- lib/snap_diff/legacy_shims.rb | 27 ++++++++++++------ test/unit/support_load_probe_test.rb | 38 +++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 24 deletions(-) diff --git a/lib/capybara/screenshot/diff/version.rb b/lib/capybara/screenshot/diff/version.rb index e6f75ee7..0d87948d 100644 --- a/lib/capybara/screenshot/diff/version.rb +++ b/lib/capybara/screenshot/diff/version.rb @@ -1,17 +1,10 @@ # frozen_string_literal: true -require "snap_diff/version" - -# Deliberately EAGER and silent (v2 step 6 exception): this is a documented -# name that adopters read directly, and const_defined? never triggers -# const_missing. The gemspec used to resolve it at build time (which a lazy -# warning shim would have made warn on every `gem build`); it reads -# SnapDiff::VERSION now, so this file is pure compatibility. See -# snap_diff/legacy_shims.rb for the full exception list. -module Capybara - module Screenshot - module Diff - VERSION = SnapDiff::VERSION - end - end -end +# Capybara::Screenshot::Diff::VERSION is a documented name adopters read +# directly, so it is assigned EAGERLY rather than shimmed -- const_defined? +# never triggers const_missing. That assignment lives in +# snap_diff/legacy_shims (required below) with the rest of the v1 surface, +# because this file is no longer on any entry point's require path: the core +# reads SnapDiff::VERSION, and so does the gemspec. Assigning it here too +# would be a duplicate-constant warning, not a second safety net. +require "snap_diff/legacy_shims" diff --git a/lib/snap_diff/legacy_shims.rb b/lib/snap_diff/legacy_shims.rb index ab69e72b..0cafa5a2 100644 --- a/lib/snap_diff/legacy_shims.rb +++ b/lib/snap_diff/legacy_shims.rb @@ -4,6 +4,7 @@ require "snap_diff/config" require "snap_diff/deprecation" require "snap_diff/drivers" +require "snap_diff/version" # THE v1 compatibility surface, in one file -- and the whole of it that is # code. lib/capybara* is alias-only by contract @@ -37,13 +38,19 @@ # entry-point constants probed with Object.const_defined? by # support_load_probe_test.rb -- const_defined? never triggers # const_missing, so a lazy shim would break that contract. -# - Capybara::Screenshot::Diff::VERSION: the gemspec resolves it at build -# time; a lazy shim would make every `gem build` warn. -# - Capybara::Screenshot::Diff::Reporters::Default (a documented subclassing -# extension point) and Capybara::Screenshot::Diff::Comparison (the images +# - Capybara::Screenshot::Diff::VERSION, and ::Comparison (the images # struct): documented user-facing names that adopters feature-detect with -# defined?/const_defined?. Assigned eagerly by their own forwarder files -# (reporters/default.rb, image_compare.rb). +# defined?/const_defined?. Both are assigned eagerly BELOW rather than by +# their own forwarder files (version.rb, image_compare.rb): the core +# stopped requiring those forwarders in the 3.0-readiness pass, so nothing +# loaded them under a canonical entry point and both names silently +# vanished from six of them. This file is required by every entry point, +# canonical and legacy, so it is the only place the eager exceptions can +# actually be eager. +# - Capybara::Screenshot::Diff::Reporters::Default (a documented subclassing +# extension point): same reasoning, but its forwarder +# (reporters/default.rb) is still loaded on every path that defines +# ::Reporters at all, so the assignment stays there. # - Drivers::ChunkyPNGDriver / Drivers::VipsDriver: real constants on the # shared SnapDiff::Drivers module (the Drivers alias is same-object by # contract), so const_missing can never fire for the leaf names; @@ -53,9 +60,10 @@ # canonical SnapDiff::Drivers.loaded -- a lazy warn-once shim could not # keep a mutable alias, and warning on a supported registration surface # would be noise. Assigned eagerly below. -# - Diff::AVAILABLE_DRIVERS: stays a real constant, aliased in -# config_legacy.rb from the canonical SnapDiff::Drivers::AVAILABLE_DRIVERS -# (same object; SnapDiff::Drivers.available is the canonical reader). +# - Diff::AVAILABLE_DRIVERS: stays a real constant, aliased BELOW from the +# canonical SnapDiff::Drivers::AVAILABLE_DRIVERS (same object; +# SnapDiff::Drivers.available is the canonical reader, and its constant is +# the stubbing point -- stubbing this alias only rebinds the alias). # The v1 namespaces, predefined empty so CONFIG_MAPPING can name them at # class-body eval time. Everything below reopens them. @@ -187,6 +195,7 @@ module Diff LOADED_DRIVERS = SnapDiff::Drivers.loaded AVAILABLE_DRIVERS = SnapDiff::Drivers::AVAILABLE_DRIVERS Comparison = SnapDiff::Comparison::Images + VERSION = SnapDiff::VERSION # Configure screenshot and diff settings in one block. # diff --git a/test/unit/support_load_probe_test.rb b/test/unit/support_load_probe_test.rb index 83914de7..48e83d99 100644 --- a/test/unit/support_load_probe_test.rb +++ b/test/unit/support_load_probe_test.rb @@ -65,6 +65,19 @@ class SupportLoadProbeTest < ActiveSupport::TestCase Capybara::Screenshot::Diff::Comparison ].freeze + # The subset of EAGER_USER_FACING that must resolve under EVERY entry + # point, canonical ones included -- these are read directly (a version + # string, a struct), not just feature-detected, so a canonical-only app + # still hits them. The test above only covers the four legacy entries, + # which is how VERSION silently disappeared from six entry points when the + # core stopped requiring capybara/screenshot/diff/version.rb: nothing + # loaded the forwarder that assigned it, and const_missing does not fire + # for a constant legacy_shims deliberately leaves out of its map. + EAGER_EVERYWHERE = %w[ + Capybara::Screenshot::Diff::VERSION + Capybara::Screenshot::Diff::Comparison + ].freeze + ENTRY_POINTS = { "capybara_screenshot_diff" => %w[ CapybaraScreenshotDiff::DSL Capybara::Screenshot::Os Capybara::Screenshot::Diff @@ -198,6 +211,31 @@ class SupportLoadProbeTest < ActiveSupport::TestCase MSG end + # Every documented entry point, canonical and legacy. capybara_screenshot_diff/dsl + # is listed only here: it is not in ENTRY_POINTS or LEGACY_ENTRY_POINTS, which + # is exactly why it was the legacy entry that lost VERSION unnoticed. + ALL_ENTRY_POINTS = ( + CANONICAL_ENTRY_POINTS.keys + LEGACY_ENTRY_POINTS + %w[capybara_screenshot_diff/dsl] + ).uniq.freeze + + test "the eager user-facing constants resolve under every entry point" do + failures = ALL_ENTRY_POINTS.filter_map do |entry| + probe(entry, <<~RUBY) + require #{entry.inspect} + missing = #{EAGER_EVERYWHERE.inspect}.reject { |c| Object.const_defined?(c) } + abort("missing: \#{missing.join(", ")}") unless missing.empty? + RUBY + end + + assert_empty failures, <<~MSG + Entry point(s) no longer resolve constants that are supposed to be eager + everywhere. `defined?` returns nil for these and const_missing does not + fire, so adopter feature detection fails silently: + + #{failures.join("\n")} + MSG + end + private # cucumber's World/Before/After/AfterAll only exist inside its runtime, so From 84b525ece25bcb27cf2648bde36ba4b1ee8c4f8e Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:31:05 +0200 Subject: [PATCH 09/11] test: reverse gate catches require_relative escapes LEGACY_REQUIRE anchored straight on the opening quote, so require_relative "../capybara/screenshot/diff/version" passed the gate while genuinely loading the v1 file (confirmed via $LOADED_FEATURES). Every core file sits one directory below lib/, so that is a one-line escape, not a hypothetical. Allow an optional (\.{1,2}/)* -- mutation-checked with exactly that line. Two smaller fixes while here: the header claimed comments are ignored when only WHOLE-LINE ones are (trailing notes on a live code line are scanned, which is the behaviour worth keeping -- the header now says so), and the stale-allowlist check now reports a deleted allowlisted file instead of raising Errno::ENOENT on it. --- .../unit/core_tree_has_no_legacy_deps_test.rb | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/test/unit/core_tree_has_no_legacy_deps_test.rb b/test/unit/core_tree_has_no_legacy_deps_test.rb index 7a039efd..e68e6b34 100644 --- a/test/unit/core_tree_has_no_legacy_deps_test.rb +++ b/test/unit/core_tree_has_no_legacy_deps_test.rb @@ -17,10 +17,13 @@ # because the generator for the v1 surface has to be code, and the v1 trees # have to stay alias-only -- they are legacy by design and go with it. # -# Comments are ignored: "ex +Capybara::Screenshot.active?+" is history, not a -# dependency. Strings are NOT ignored -- a user-facing message naming a -# legacy accessor is a legacy reference that survives the deletion and starts -# lying the day it happens. +# WHOLE-LINE comments are ignored: "ex +Capybara::Screenshot.active?+" on its +# own line is history, not a dependency. Everything else on a code line +# counts, strings and trailing comments included -- a user-facing message +# naming a legacy accessor is a legacy reference that survives the deletion +# and starts lying the day it happens, and a trailing note on a live line is +# close enough to the code to be worth keeping honest. Move such a note to +# its own line if the gate objects. class CoreTreeHasNoLegacyDepsTest < ActiveSupport::TestCase LIB = Pathname.new(__dir__).join("../../lib").expand_path @@ -40,7 +43,12 @@ class CoreTreeHasNoLegacyDepsTest < ActiveSupport::TestCase # `capybara_screenshot_diff...`, `capybara-screenshot-diff`. Plain # `require "capybara"` / `"capybara/dsl"` is the base gem, not this gem's # legacy tree, so it must not match. - LEGACY_REQUIRE = %r{\Arequire(_relative)?\s+["']capybara(/screenshot|_screenshot_diff|-screenshot-diff)} + # + # The `(\.{1,2}/)*` is load-bearing: every core file sits one directory + # below lib/, so `require_relative "../capybara/screenshot/diff/version"` + # reaches the v1 tree and really loads it. Anchoring straight on the quote + # let that through. + LEGACY_REQUIRE = %r{\Arequire(_relative)?\s+["'](\.{1,2}/)*capybara(/screenshot|_screenshot_diff|-screenshot-diff)} # A read or write of a v1 namespace constant. LEGACY_CONSTANT = /(? Date: Sun, 23 Aug 2026 11:31:14 +0200 Subject: [PATCH 10/11] test: forwarder rule takes simple pass-through args only The tightened rule rejected ternaries and semicolons but left `(.*)` and `{ .* }` unbounded, so both of these still passed as "forwarders": SnapDiff.config.x(File.exist?("/etc/passwd") ? raise("boom") : ENV.fetch("HOME")) SnapDiff.config.tap { |c| File.write("/tmp/pwned", c.inspect); exit 1 } The block case also dodged the semicolon check, because the walk steps over a def's body line without re-examining it -- that scan now runs over every line up front. An argument list is now names, commas, splats and keyword colons, or Ruby's `...` forwarding. No parens, so no nested call; no `.`, so no bare receiver call; no `?`/quote/`=`, so no conditional, literal or assignment. The block form is gone entirely: nothing in these trees has a def left, and an unbounded block is exactly the hole above. Mutation-checked with both shapes above (rejected, the block twice) plus three genuine forwarders that must not false-positive: a bare chain, an argument pass-through, and CapybaraScreenshotDiff.serve(...) -- which the first draft of this rule did reject. --- test/unit/legacy_tree_is_alias_only_test.rb | 56 +++++++++++++-------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/test/unit/legacy_tree_is_alias_only_test.rb b/test/unit/legacy_tree_is_alias_only_test.rb index a459ab7f..29d3f741 100644 --- a/test/unit/legacy_tree_is_alias_only_test.rb +++ b/test/unit/legacy_tree_is_alias_only_test.rb @@ -36,20 +36,32 @@ class LegacyTreeIsAliasOnlyTest < ActiveSupport::TestCase # A `def` in these trees is only acceptable as a THREE-line forwarder -- # signature, ONE delegating expression, `end` -- and this is that - # expression: a single method-call chain rooted at SnapDiff, with at most - # one argument list and one block. + # expression: a single method-call chain rooted at SnapDiff, passing its + # arguments straight through. # - # The rule used to be `body.include?("SnapDiff")`, which waved through - # real behaviour hiding behind a mention of the canonical namespace: - # `SnapDiff.config.a ? b : c` is a conditional, and (with the `;` hole - # closed below) `SnapDiff.config.a; something_else` was two statements. - # Anchoring the whole body rejects both -- neither can be consumed by the - # chain, so neither matches. + # SIMPLE_ARGS is where the strictness lives. Names, commas, `*`/`**`/`&` + # and keyword colons -- and nothing else. No parentheses, so a nested call + # cannot appear; no `.`, so neither can a bare receiver call; no `?`, `"` + # or `=`, so no conditional, literal or assignment. Two escapes this + # closes, both of which ran arbitrary code past the previous rule: + # + # SnapDiff.config.x(File.exist?("/etc/passwd") ? raise("boom") : ENV.fetch("HOME")) + # SnapDiff.config.tap { |c| File.write("/tmp/pwned", c.inspect); exit 1 } + # + # The second also slipped past the semicolon check, because the walk below + # steps over a def's body line -- fixed there. + # + # No block form at all: nothing in these trees has a `def` left, and an + # unbounded `{ ... }` is exactly the hole above. A yield-through forwarder + # that genuinely needs one is a decision to re-open here, deliberately. + # `...` is Ruby's argument forwarding -- the purest forwarder there is + # (CapybaraScreenshotDiff.serve uses it), so it is spelled out rather than + # let in by loosening the character set. + SIMPLE_ARGS = /\.\.\.|[\w\s,:*&]*/ FORWARDER_BODY = /\A - SnapDiff(::[A-Z]\w*)* # SnapDiff, SnapDiff::Reporters, ... - (\.[a-z_]\w*[?!]?)+ # .config.active?, .compare, ... - (\(.*\))? # at most one argument list - (\s*\{.*\})? # at most one block (yield-through forwarders) + SnapDiff(::[A-Z]\w*)* # SnapDiff, SnapDiff::Reporting, ... + (\.[a-z_]\w*[?!]?)+ # .config.active?, .compare, ... + (\((?:#{SIMPLE_ARGS})\))? # at most one argument list, pass-through only \z/x # Shapes that are pure compatibility plumbing rather than behaviour. @@ -99,19 +111,23 @@ def allowed?(file) def offences(file) rel = file.relative_path_from(LIB) lines = significant_lines(file) - found = [] + + # A semicolon is how several statements -- or an entire + # `def x; body; end` -- hide inside one "line", which would then be + # judged as a single line. Never alias-shaped, whatever it says. + # + # Scanned over EVERY line up front, not inside the walk below: the walk + # steps past a def's body line without re-examining it, so a semicolon + # there went unseen. + found = lines.filter_map do |line| + "#{rel}: `#{line}` puts more than one statement on a line" if line.include?(";") + end index = 0 while index < lines.length line = lines[index] - if line.include?(";") - # A semicolon is how several statements -- or an entire - # `def x; body; end` -- hide inside one "line", which would then be - # judged as a single line. Never alias-shaped, whatever it says. - found << "#{rel}: `#{line}` puts more than one statement on a line" - index += 1 - elsif line.start_with?("def ") + if line.start_with?("def ") body, terminator = lines[index + 1], lines[index + 2] unless FORWARDER_BODY.match?(body.to_s) && terminator == "end" found << "#{rel}: `#{line}` is not a single-expression forwarder into SnapDiff" From c50d1973ec63f34291d6590368adbaae77b4caa4 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Sun, 23 Aug 2026 11:31:20 +0200 Subject: [PATCH 11/11] docs: flag the two beta moves that fail silently Both are read-identical and only break on write, so nothing warns: - stubbing Capybara::Screenshot::Diff::AVAILABLE_DRIVERS now only rebinds an alias -- the gem reads SnapDiff::Drivers::AVAILABLE_DRIVERS, so a downstream test stubbing it to [] stops exercising the no-drivers path and passes for the wrong reason; - SnapDiff::Config::MAPPING is gone mid-beta, split into Config::SETTINGS and the @api-private LegacyShims::CONFIG_MAPPING. --- docs/UPGRADING.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index feab764e..0a9a4ffd 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -199,6 +199,19 @@ Two deliberate consequences of the lazy shim design — both flagged for feedbac 2. **Reopening `module Capybara::Screenshot::Diff::Drivers` shadows the shim.** The historical custom-driver monkey-patch pattern defines a fresh, empty `Drivers` module instead of reaching the real one. Define custom drivers under `SnapDiff::Drivers` instead — and note `BaseDriver` is gone as a superclass: `class MyDriver < BaseDriver` becomes `include SnapDiff::Driver` (it's a mixin now). +#### Two moves that fail *silently* if you miss them + +**Stubbing the detected-drivers list.** The value moved to `SnapDiff::Drivers::AVAILABLE_DRIVERS`, and `Capybara::Screenshot::Diff::AVAILABLE_DRIVERS` is now an eager alias of it. *Reading* either is identical, but **stubbing the legacy name only rebinds the alias** — the gem keeps reading the canonical constant, so a test that stubs it to `[]` no longer exercises the no-drivers path and just passes for the wrong reason: + +```ruby +# before +Capybara::Screenshot::Diff.stub_const(:AVAILABLE_DRIVERS, []) { ... } +# now +SnapDiff::Drivers.stub_const(:AVAILABLE_DRIVERS, []) { ... } +``` + +**`SnapDiff::Config::MAPPING` is gone.** It split in two: `SnapDiff::Config::SETTINGS` (the setting names, no legacy knowledge) and `SnapDiff::LegacyShims::CONFIG_MAPPING` (which legacy holder each name hangs off). If you referenced `MAPPING` — iterating settings in a test helper, say — use `SETTINGS`; `CONFIG_MAPPING` is `@api private` and disappears in 3.0 with the rest of the v1 surface. + --- ### FAQ