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/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 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 0fed2093..0da6e61a 100644 --- a/lib/capybara/screenshot/diff/config_legacy.rb +++ b/lib/capybara/screenshot/diff/config_legacy.rb @@ -2,66 +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 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" - -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::Utils.detect_available_drivers.freeze - - # 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/capybara/screenshot/diff/version.rb b/lib/capybara/screenshot/diff/version.rb index 01022d9f..0d87948d 100644 --- a/lib/capybara/screenshot/diff/version.rb +++ b/lib/capybara/screenshot/diff/version.rb @@ -1,15 +1,10 @@ # frozen_string_literal: true -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. -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.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/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/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/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/dsl.rb b/lib/snap_diff/dsl.rb index 52545913..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" @@ -50,7 +51,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 +69,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 +83,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 +111,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/legacy_shims.rb b/lib/snap_diff/legacy_shims.rb index 0341ae6d..0cafa5a2 100644 --- a/lib/snap_diff/legacy_shims.rb +++ b/lib/snap_diff/legacy_shims.rb @@ -1,8 +1,28 @@ # frozen_string_literal: true +require "snap_diff/comparison" +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 +# (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 @@ -18,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; @@ -34,9 +60,20 @@ # 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 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. +module Capybara + module Screenshot + module Diff + end + end +end + module SnapDiff # @api private module LegacyShims @@ -54,14 +91,133 @@ 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 + VERSION = SnapDiff::VERSION + + # 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/lib/snap_diff/reporters/html.rb b/lib/snap_diff/reporters/html.rb index d5819564..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 @@ -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..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" @@ -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/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/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/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 new file mode 100644 index 00000000..e68e6b34 --- /dev/null +++ b/test/unit/core_tree_has_no_legacy_deps_test.rb @@ -0,0 +1,129 @@ +# 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. +# +# 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 + + # 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. + # + # 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 = /(?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" + + 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| + file = LIB.join(path) + # A deleted file is the most stale an entry can get; report it rather + # than letting Pathname#read blow up with Errno::ENOENT. + next ["#{path}: allowlisted file no longer exists"] unless file.exist? + + present = significant_lines(file).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 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/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/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/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? diff --git a/test/unit/legacy_tree_is_alias_only_test.rb b/test/unit/legacy_tree_is_alias_only_test.rb index 2838828c..29d3f741 100644 --- a/test/unit/legacy_tree_is_alias_only_test.rb +++ b/test/unit/legacy_tree_is_alias_only_test.rb @@ -23,19 +23,47 @@ 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, passing its + # arguments straight through. + # + # 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::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. ALIAS_SHAPES = /\A( require(_relative)?\s | @@ -78,12 +106,22 @@ 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) - 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 @@ -91,8 +129,8 @@ def offences(file) if 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 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 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) 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