From 54187eb306e5262f19eaf119188336f51dcf287b Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Sun, 23 Aug 2026 19:15:27 +0200 Subject: [PATCH] feat: deprecate chunky_png, shift_distance_limit and the driver abstraction (removed in 2.1) 2.0 is the transitional release: the contract is published before it is enforced. The legacy-namespace half of that promise already warns. The driver half -- everything 2.1 removes so that libvips becomes the only backend -- warned about nothing. A user on `driver: :auto` without libvips had no way to learn that 2.1 stops comparing for them. Six warnings, each once per process, each silenceable through the existing switches (`SnapDiff.silence_deprecations`, SNAP_DIFF_SILENCE_DEPRECATIONS): - chunky_png SELECTED, from Utils.find_driver_class_for -- the one funnel every selection surface ends up in (`driver:` per comparison, SnapDiff.config.driver, the legacy Diff.driver=). - `:auto` FALLING BACK to chunky_png because libvips is absent. Same funnel, its own message and key: these users never asked for chunky_png. - shift_distance_limit SET, from Config#shift_distance_limit= and from Comparison#initialize (non-nil only -- default_options carries the key on every comparison). One shared key, so it warns once whichever fires. - Drivers.loaded, the documented custom-driver registration point. - Drivers.available, i.e. driver detection. - `include SnapDiff::Driver` by a class that is not one of the gem's own. NOT warned on, and documented as silent: Drivers.for (the gem calls it for every comparison, so warning there would fire on vips-only setups that nothing here affects), detection itself (runs at load, before user code), and the eager LOADED_DRIVERS / AVAILABLE_DRIVERS constant aliases (nothing to hook). Each entry names the warning-capable equivalent instead. The machinery is SnapDiff::Removal -- same channel, same discipline and the same silencing switches as SnapDiff::Deprecation, in its own file: the legacy shims and their deprecation channel are part of what the deletion removes, while these call sites (utils, config, comparison, drivers) are core files that outlive them and cannot depend on a doomed file. SnapDiff.silence_deprecations moved there for the same reason -- it is the one switch that silences both halves -- which is why the canonical-suite gate no longer lists it as shim-only surface. Two internal seams keep the gem from warning at itself: Drivers.registry (the unannounced registry the gem reads) and reading the AVAILABLE_DRIVERS constant directly instead of .available. test_helper suppresses the channel the same way it already suppresses the migration notice: this suite runs its whole matrix on chunky_png by design. Evidence: 13 subprocess probes ("once per process" cannot be measured inside one long-lived suite), mutation-tested -- dropping the dedup goes 9 red, dropping the silence check 2 red, routing an internal read back through .loaded 5 red, unscoping the mixin hook 5 red, warning on the presence of shift_distance_limit rather than a value 4 red. rake test:unit 585/0, rake test 613/0/1, rake test:canonical 482/0/1, standardrb clean. --- docs/UPGRADING.md | 36 ++- docs/configuration.md | 11 +- docs/drivers.md | 39 +++ docs/snapdiff.md | 12 +- lib/snap_diff/comparison.rb | 8 + lib/snap_diff/config.rb | 18 +- lib/snap_diff/deprecation.rb | 26 +- lib/snap_diff/driver.rb | 18 ++ lib/snap_diff/drivers.rb | 43 ++- lib/snap_diff/legacy_shims.rb | 5 +- lib/snap_diff/removal.rb | 115 ++++++++ lib/snap_diff/utils.rb | 33 ++- .../legacy_namespace_deprecation_test.rb | 6 + test/test_helper.rb | 8 + ...canonical_suite_has_no_legacy_refs_test.rb | 8 +- test/unit/removed_in_2_1_deprecation_test.rb | 249 ++++++++++++++++++ 16 files changed, 599 insertions(+), 36 deletions(-) create mode 100644 lib/snap_diff/removal.rb create mode 100644 test/unit/removed_in_2_1_deprecation_test.rb diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 3aab628b..8b739311 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -159,7 +159,8 @@ This means you can migrate your codebase incrementally **now**, before opting in ### Deprecation Warnings -v2.0 emits two different things, and it is worth knowing which is which. +v2.0 emits three different things, and it is worth knowing which is which. The first two are +about the old namespaces; the third is about the driver features 2.1 removes. #### 1. The migration notice — one line per process @@ -199,6 +200,39 @@ ErrorWithFilteredBacktrace, ScreenshotAssertion, AssertionRegistry}`; canonical `snap_diff*` require**. Under the v1 entry points — what an unmigrated app actually uses — they are eagerly defined and silent, like everything in the next section. +#### 3. Removal warnings — the driver half, removed in 2.1 + +The warnings above are about *names*. These are about *features*: 2.1 makes **libvips the only +image backend** and deletes the rest of the driver machinery. 2.0 still supports all of it and +warns once per process per subject, through the same channel and the same silencing switches. + +| You will see it when you… | Removed in 2.1 | Do this instead | +|---|---|---| +| select the ChunkyPNG driver — `driver: :chunky_png`, `SnapDiff.config.driver = :chunky_png`, or the legacy `Capybara::Screenshot::Diff.driver =` | the `:chunky_png` driver | add `gem "ruby-vips"` (plus the libvips system package) and drop the option | +| run on `driver: :auto` **without `ruby-vips` installed** | the `:auto` fallback to ChunkyPNG | same — install libvips + `ruby-vips`. This is the case worth reading twice: nothing in your setup says `chunky_png`, so the warning is the only sign that 2.1 will break this process | +| set `shift_distance_limit` — globally or per screenshot | `shift_distance_limit` (ChunkyPNG-only) | `median_filter_window_size`, `tolerance`, or `color_distance_limit` — see [Configuration](configuration.md#allowed-shift-distance) | +| read `SnapDiff::Drivers.loaded` (the custom-driver registry) | the registry | nothing — custom drivers are removed, see below | +| read `SnapDiff::Drivers.available` | driver detection | require `ruby-vips` instead of branching on a detected list | +| `include SnapDiff::Driver` in your own driver class | the driver mixin | nothing — see below | + +``` +[snap_diff deprecation] `driver: :auto` selected chunky_png because libvips is not available in this process. The chunky_png driver is REMOVED in 2.1, when libvips (the `ruby-vips` gem) becomes required -- install it now, or this setup stops comparing on 2.1. See docs/drivers.md. Silence with `SnapDiff.silence_deprecations = true` or SNAP_DIFF_SILENCE_DEPRECATIONS=1. (shown once per process) (called from /app/test/test_helper.rb:12) +``` + +**Custom drivers have no migration path.** The whole abstraction goes: the `SnapDiff::Driver` +mixin, the `SnapDiff::Drivers.loaded` registry, `SnapDiff::Drivers.available` / +`SnapDiff::Utils.detect_available_drivers`, and selecting a driver by name. Nothing replaces +them, and this guide is not going to pretend otherwise — if you maintain a third-party driver, +say so on [#166](https://github.com/snap-diff/snap_diff-capybara/issues/166) before 2.1 ships. + +Three spots on the same chopping block stay silent: the legacy +`Capybara::Screenshot::Diff::LOADED_DRIVERS` / `::AVAILABLE_DRIVERS` aliases are plain constants +with nothing to hook (use `SnapDiff::Drivers.loaded` / `.available` to hear the warning); +`SnapDiff::Drivers.for` is not warned on at all — the gem itself calls it for every comparison, +so warning there would fire on setups that are not affected by anything on this list; and +detection (`SnapDiff::Drivers.detect_available` / `SnapDiff::Utils.detect_available_drivers`) +runs at load, before any user code. + #### Silent by design Some legacy names never warn individually, and that is deliberate — the migration notice above is diff --git a/docs/configuration.md b/docs/configuration.md index 1b245cd7..2406f5ed 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -122,7 +122,7 @@ Just `require 'snap_diff/integrations/minitest'` (legacy: `capybara_screenshot_d | Setting | When to use | |---------|-------------| | `perceptual_threshold` | Anti-aliasing false positives across OS/browser versions | -| `shift_distance_limit` | Content shifts by a few pixels (ChunkyPNG only) | +| `shift_distance_limit` | Content shifts by a few pixels (ChunkyPNG only — **removed in 2.1**) | | `area_size_limit` | Allow small diff regions below a pixel count | | `color_distance_limit` | Fine-tune raw RGB channel tolerance | | `median_filter_window_size` | Smooth noise before comparison (VIPS only) | @@ -313,6 +313,15 @@ Capybara::Screenshot::Diff.color_distance_limit = 42 ### Allowed shift distance +> **Removed in 2.1.** `shift_distance_limit` is implemented only by the ChunkyPNG driver, +> and 2.1 removes that driver — libvips becomes the only backend. Setting it anywhere +> (`SnapDiff.config.shift_distance_limit =`, the legacy +> `Capybara::Screenshot::Diff.shift_distance_limit =`, or `screenshot 'index', +> shift_distance_limit: 2`) warns once per process in 2.0. There is no vips equivalent: +> use `median_filter_window_size` (the faster answer to the same problem — see +> [Drivers](drivers.md#median-filter-size-vips-only)), `tolerance`, or +> `color_distance_limit`. + Sometimes you want to allow small movements in the images. For example, jquery-tablesorter renders the same table slightly differently sometimes. You can set set the shift distance threshold for the comparison using the `shift_distance_limit` option to the `screenshot` diff --git a/docs/drivers.md b/docs/drivers.md index 070dc0d8..f4faede0 100644 --- a/docs/drivers.md +++ b/docs/drivers.md @@ -6,6 +6,41 @@ > [Custom drivers](snapdiff.md#custom-drivers) for the `SnapDiff::Driver` mixin and how > registration in `SnapDiff::Drivers.loaded` works. +## Removed in 2.1: everything on this page except VIPS + +2.1 makes **libvips the only backend**. 2.0 is the transitional release — all of the +following still works, and warns once per process naming 2.1. Silence the warnings with +`SnapDiff.silence_deprecations = true` or `SNAP_DIFF_SILENCE_DEPRECATIONS=1`. + +| Removed in 2.1 | What to do in 2.0 | +|---|---| +| the `:chunky_png` driver | add `gem "ruby-vips"` to your Gemfile and drop `driver: :chunky_png` | +| `driver: :auto` (and the `:auto` default) | with one backend there is nothing to choose; install `ruby-vips` and the default just works | +| `shift_distance_limit` | ChunkyPNG-only. Use `median_filter_window_size`, `tolerance` or `color_distance_limit` — see [Configuration](configuration.md#allowed-shift-distance) | +| `SnapDiff::Driver` (the custom-driver mixin) | nothing — see below | +| `SnapDiff::Drivers.loaded` (the registry) | nothing — see below | +| `SnapDiff::Drivers.available` (driver detection) | require `ruby-vips` instead of branching on a detected list | + +Three related names on the same chopping block stay **silent**, and deliberately so: +`SnapDiff::Drivers.for` (the gem calls it for every comparison — warning there would fire on +setups that nothing in this table affects), `SnapDiff::Drivers.detect_available` / +`SnapDiff::Utils.detect_available_drivers` (run at load, before any user code), and the legacy +`Capybara::Screenshot::Diff::LOADED_DRIVERS` / `::AVAILABLE_DRIVERS` constant aliases (plain +constants, nothing to hook). Reach the same values through `.loaded` / `.available` and you +will hear about them. + +**libvips becomes a hard requirement.** Install it with your system package manager +(`brew install vips`, `apt-get install libvips`) and add `gem "ruby-vips"`. A 2.1 process +without it cannot compare images at all. + +**Custom drivers: there is no migration path.** The driver abstraction is removed whole — +the `SnapDiff::Driver` mixin, the `SnapDiff::Drivers.loaded` registry, and driver +selection by name. Third-party drivers stop working in 2.1 and nothing replaces them +(the decision was made deliberately: no measurable demand, and one backend is what keeps +the comparison engine honest). If you maintain one, say so on +[the issue tracker](https://github.com/snap-diff/snap_diff-capybara/issues) before 2.1 +ships — that is the only thing that can change this. + ## Perceptual color comparison (VIPS only) By default, color differences are measured using raw RGB channel distance. This can produce @@ -52,6 +87,10 @@ There are several options to setup active driver: `:auto`, `:chunky_png` and `:v * `:auto` - will try to load `:vips` if there is gem `ruby-vips`, in other cases will load `:chunky_png` * `:chunky_png` and `:vips` will load correspondent driver +> **2.1 keeps only `:vips`.** `:auto` and `:chunky_png` are removed; each warns once per +> process in 2.0. If `:auto` is quietly running you on ChunkyPNG today (no `ruby-vips` +> installed), the warning says so — that is the setup 2.1 breaks. + ## Enable VIPS image processing [Vips](https://www.rubydoc.info/gems/ruby-vips/Vips/Image) driver provides a faster comparison, diff --git a/docs/snapdiff.md b/docs/snapdiff.md index b99a550f..d431afe6 100644 --- a/docs/snapdiff.md +++ b/docs/snapdiff.md @@ -165,8 +165,8 @@ integration require; a few objects need their own require, noted below. | `SnapDiff::ExpectationNotMet` | A screenshot did not match its baseline | | `SnapDiff::UnstableImage` | No stable capture within `stability_time_limit` / `wait` | | `SnapDiff::WindowSizeMismatchError` | Browser window is not the configured `window_size` | -| `SnapDiff::Driver` | Mixin with the shared driver defaults (`require "snap_diff/driver"`) | -| `SnapDiff::Drivers` | Driver factory and registry — `.for`, `.loaded`, `.available` | +| `SnapDiff::Driver` | Mixin with the shared driver defaults (`require "snap_diff/driver"`) — **removed in 2.1** | +| `SnapDiff::Drivers` | Driver factory and registry — `.for`, `.loaded`, `.available` — **removed in 2.1** | | `SnapDiff::Reporting` | Process-global reporter lifecycle (`require "snap_diff/reporting"`) | | `SnapDiff::Reporters::HTML` | The interactive HTML report (`require "snap_diff/reporters/html"`) | | `SnapDiff::Reporters::Default` | Builds the annotated diff images and the failure message | @@ -264,6 +264,14 @@ SnapDiff::Reporting.finalize! ## Custom drivers +> **Removed in 2.1 — no replacement.** libvips becomes the only backend, and the driver +> abstraction goes with the choice: the `SnapDiff::Driver` mixin, the +> `SnapDiff::Drivers.loaded` registry, `SnapDiff::Drivers.available`, and selecting a driver +> by name. In 2.0 all of it still works and warns once per process (silence with +> `SnapDiff.silence_deprecations = true` or `SNAP_DIFF_SILENCE_DEPRECATIONS=1`). Nothing +> here migrates to a 2.1 shape — there is no 2.1 shape. If you maintain a driver, say so on +> [#166](https://github.com/snap-diff/snap_diff-capybara/issues/166) before 2.1 ships. + A driver is a plain object that does the image work. Include `SnapDiff::Driver` for the shared defaults, then implement the operations the comparison engine calls: diff --git a/lib/snap_diff/comparison.rb b/lib/snap_diff/comparison.rb index b0d60240..d4e64b2f 100644 --- a/lib/snap_diff/comparison.rb +++ b/lib/snap_diff/comparison.rb @@ -6,6 +6,7 @@ require "snap_diff/comparison_result" require "snap_diff/drivers" require "snap_diff/image_preprocessor" +require "snap_diff/removal" require "snap_diff/reporters/default" module SnapDiff @@ -53,6 +54,13 @@ def initialize(image_path, base_image_path, options = {}) ensure_files_exist! @driver_options = options.freeze + # The per-comparison half of the shift_distance_limit removal (the + # global half is Config#shift_distance_limit=). Presence is not enough: + # config.default_options carries the key on EVERY comparison, nil for + # everyone who never set it. + if options[:shift_distance_limit] + Removal.warn_once(:shift_distance_limit, Removal::SHIFT_DISTANCE_LIMIT_REMOVED) + end @driver = Drivers.for(@driver_options) @without_tolerable_options = (driver_options.keys & TOLERABLE_OPTIONS).empty? end diff --git a/lib/snap_diff/config.rb b/lib/snap_diff/config.rb index 54c46877..367e0d3c 100644 --- a/lib/snap_diff/config.rb +++ b/lib/snap_diff/config.rb @@ -12,6 +12,7 @@ # Referenced by Config#initialize (screenshoter/manager defaults), which # runs at the eager Config.new at the bottom of this file, so they must be # real, already-loaded classes first. Neither requires back here. +require "snap_diff/removal" require "snap_diff/screenshoter" require "snap_diff/snap_manager" @@ -78,8 +79,11 @@ class Config manager ].freeze - attr_accessor(*(SETTINGS - [:root])) - attr_reader :root + # shift_distance_limit is excluded from the generated writers and hand + # written below (it announces its 2.1 removal); generating it here too + # would print Ruby's "method redefined" warning on every load. + attr_accessor(*(SETTINGS - %i[root shift_distance_limit])) + attr_reader :root, :shift_distance_limit def initialize # Every setting gets its ivar up front (nil-defaulted ones included) @@ -112,6 +116,16 @@ def root=(path) @root = Pathname(path).expand_path end + # Overrides the generated accessor above to announce the 2.1 removal + # (chunky_png-only, and chunky_png goes too). The writer, not the reader: + # the reader runs on every comparison through #default_options, including + # for the overwhelming majority who never set this. #initialize seeds the + # ivar directly, so booting the gem stays silent. + def shift_distance_limit=(value) + Removal.warn_once(:shift_distance_limit, Removal::SHIFT_DISTANCE_LIMIT_REMOVED) unless value.nil? + @shift_distance_limit = value + end + # --- Derived config (ADR-008 step 7b) ------------------------------- # Read-only values computed from the storage above. They used to live # on the legacy modules; those now one-line forward here. diff --git a/lib/snap_diff/deprecation.rb b/lib/snap_diff/deprecation.rb index f1b92a2c..8cfea301 100644 --- a/lib/snap_diff/deprecation.rb +++ b/lib/snap_diff/deprecation.rb @@ -1,5 +1,11 @@ # frozen_string_literal: true +# SnapDiff.silence_deprecations? -- the one switch that silences BOTH halves +# of the story -- lives in snap_diff/removal.rb, not here: the other half +# (the driver features 2.1 removes) is announced from core files that outlive +# this one, and they cannot depend on a file the same deletion removes. +require "snap_diff/removal" + module SnapDiff # @api private # @@ -123,24 +129,4 @@ def origin_for(locations) end end end - - class << self - # @api private - attr_accessor :silence_deprecations - - # @api private - # - # @return [Boolean] true if deprecation warnings should be suppressed, - # either via the {silence_deprecations} accessor or the - # SNAP_DIFF_SILENCE_DEPRECATIONS env var (truthy = "1"/"true"). - def silence_deprecations? - !!silence_deprecations || truthy_env?(ENV["SNAP_DIFF_SILENCE_DEPRECATIONS"]) - end - - private - - def truthy_env?(value) - %w[1 true].include?(value.to_s.downcase) - end - end end diff --git a/lib/snap_diff/driver.rb b/lib/snap_diff/driver.rb index 34035fbb..7844b6f7 100644 --- a/lib/snap_diff/driver.rb +++ b/lib/snap_diff/driver.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "snap_diff/removal" + module SnapDiff # Shared default behavior for image-processing drivers. # @@ -10,6 +12,22 @@ module SnapDiff module Driver PNG_EXTENSION = ".png" + # Including this mixin is what makes a custom driver a driver, so it is + # where a custom-driver author can be told that 2.1 removes the whole + # abstraction. Scoped to drivers that are NOT the gem's own two: both + # bundled drivers include it themselves, and warning there would fire on + # every plain vips setup -- about code the user does not own. + def self.included(base) + return if base.name.to_s.start_with?("SnapDiff::") + + Removal.warn_once( + :driver_mixin, + "`include SnapDiff::Driver` (in #{base.name || base.inspect}) is REMOVED in 2.1: the " \ + "driver abstraction goes away and libvips becomes the only backend, so custom drivers " \ + "stop working. There is no replacement -- see docs/drivers.md." + ) + end + def same_dimension?(comparison) dimension(comparison.base_image) == dimension(comparison.new_image) end diff --git a/lib/snap_diff/drivers.rb b/lib/snap_diff/drivers.rb index 29de394b..52bfb4ad 100644 --- a/lib/snap_diff/drivers.rb +++ b/lib/snap_diff/drivers.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "snap_diff/removal" + module SnapDiff # utils.rb requires THIS file at its top. Requiring it back at load time # made Ruby shout "circular require considered harmful" under $VERBOSE -- @@ -19,14 +21,34 @@ def self.for(driver_options = {}) Utils.find_driver_class_for(driver_option).new end + # @api private + # + # The registry itself, unannounced: driver name => driver class, filled + # lazily by Utils.find_driver_class_for and mutated in place. The gem's + # own reads go through HERE rather than through .loaded, so the removal + # warning below stays a signal about USER code -- a gem that warns at + # itself teaches people to ignore its warnings. + def self.registry + @registry ||= {} + end + # Canonical driver-class cache (ADR-008 step 5b, ex # Capybara::Screenshot::Diff::LOADED_DRIVERS): driver name => driver - # class, filled lazily by Utils.find_driver_class_for. Mutated in - # place -- including by user registration through the legacy constant, - # which legacy_shims pins as an EAGER same-object alias of this hash - # (a lazy copy would silently drop such registrations). + # class. Mutated in place -- including by user registration through the + # legacy constant, which legacy_shims pins as an EAGER same-object alias + # of this hash (a lazy copy would silently drop such registrations). + # + # THE documented custom-driver registration point (docs/snapdiff.md), so + # a custom-driver author has to hear that 2.1 takes it away. def self.loaded - @loaded ||= {} + Removal.warn_once( + :drivers_loaded, + "`SnapDiff::Drivers.loaded` is REMOVED in 2.1 together with the rest of the driver " \ + "abstraction (`SnapDiff::Driver`, `SnapDiff::Drivers.available`, `driver: :auto`): " \ + "libvips becomes the only backend and custom drivers are no longer supported. " \ + "See docs/drivers.md." + ) + registry end # Which image drivers this process can actually load, in preference @@ -76,7 +98,18 @@ def self.detect_available # than caching, because the constant is the published stubbing point # (image_compare_test stubs it to [] to exercise the no-drivers error # path). + # + # Detection only exists because there is a choice of backend to detect; + # 2.1 removes the choice, so it warns. The gem's own callers read + # AVAILABLE_DRIVERS directly -- same value, same stubbing point, no + # warning at itself. def self.available + Removal.warn_once( + :drivers_available, + "`SnapDiff::Drivers.available` is REMOVED in 2.1: with libvips the only backend there " \ + "is nothing left to detect. Require the `ruby-vips` gem instead of branching on this " \ + "list. See docs/drivers.md." + ) AVAILABLE_DRIVERS end end diff --git a/lib/snap_diff/legacy_shims.rb b/lib/snap_diff/legacy_shims.rb index 1f4a010d..fe708489 100644 --- a/lib/snap_diff/legacy_shims.rb +++ b/lib/snap_diff/legacy_shims.rb @@ -262,7 +262,10 @@ def screenshot_area_abs module Diff # 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 + # .registry, not .loaded: this alias is assigned at load time by the + # gem itself, and .loaded announces its own 2.1 removal. Same object + # either way -- which is the whole point of the alias. + LOADED_DRIVERS = SnapDiff::Drivers.registry AVAILABLE_DRIVERS = SnapDiff::Drivers::AVAILABLE_DRIVERS Comparison = SnapDiff::Comparison::Images VERSION = SnapDiff::VERSION diff --git a/lib/snap_diff/removal.rb b/lib/snap_diff/removal.rb new file mode 100644 index 00000000..1b22d7a5 --- /dev/null +++ b/lib/snap_diff/removal.rb @@ -0,0 +1,115 @@ +# frozen_string_literal: true + +module SnapDiff + # @api private + # + # Announces what 2.1 REMOVES, from the 2.0 line that still supports it: + # the chunky_png driver, +shift_distance_limit+ (chunky-only, it dies with + # it) and the driver abstraction (+SnapDiff::Driver+, + # +SnapDiff::Drivers.loaded+ / +.available+, driver: :auto) -- + # libvips becomes the only backend. 2.0 is the transitional release: the + # contract is published before it is enforced. + # + # Same shape and same silencing switches as {SnapDiff::Deprecation}, which + # announces the other half (the v1 namespaces), but a file of its own: the + # legacy shims and their deprecation channel are themselves part of what is + # removed, while the call sites here -- utils, config, drivers -- are core + # files that outlive them, so they cannot depend on a doomed file. That is + # also why {SnapDiff.silence_deprecations} lives HERE rather than in + # deprecation.rb: it is the one switch that silences both halves. + # + # Deliberately not a warn-per-call channel: one line per subject per + # process is an actionable signal, N lines per comparison is noise people + # learn to filter out. + module Removal + # Everything under lib/ is "the gem"; the first caller frame outside it + # is the user code that touched the doomed API. + GEM_LIB_DIR = File.expand_path("..", __dir__) + File::SEPARATOR + + # Appended to every message, so the individual messages can stay about + # the thing being removed. + SILENCE_HINT = + "Silence with `SnapDiff.silence_deprecations = true` or " \ + "SNAP_DIFF_SILENCE_DEPRECATIONS=1. (shown once per process)" + + # The one message with two call sites -- the setting's writer (Config) + # and the per-comparison option (Comparison) -- so it lives here rather + # than in either of them. One subject, one warning, whichever fires. + SHIFT_DISTANCE_LIMIT_REMOVED = + "`shift_distance_limit` is REMOVED in 2.1: it is implemented only by the chunky_png " \ + "driver, which is removed with it. libvips has no shift-distance comparison -- drop the " \ + "option and tune `tolerance` / `color_distance_limit` instead. See docs/configuration.md." + + MUTEX = Mutex.new + @seen = {} + @suppressed = false + + class << self + # Emit +message+ once per +subject+ per process, via Kernel#warn (so + # anything hooking +Warning.warn+ sees it like any other Ruby warning). + # + # @param subject [Symbol] dedup key -- the doomed API, not the call site + # @param message [String] what is removed, when, and what to do instead + # @return [void] + def warn_once(subject, message) + return if @suppressed || SnapDiff.silence_deprecations? + + first_time = MUTEX.synchronize { @seen.key?(subject) ? false : (@seen[subject] = true) } + return unless first_time + + Kernel.warn(with_origin("[snap_diff deprecation] #{message} #{SILENCE_HINT}", caller_locations(1))) + end + + # @api private + # + # Silences these warnings for the rest of the process, without touching + # the v1-namespace ones. For hosts that exercise the doomed APIs BY + # DESIGN rather than depending on them -- this gem's own suite runs the + # whole comparison matrix on chunky_png and sets shift_distance_limit, + # and its test_helper raises on any deprecation output. + # @return [void] + def suppress! + MUTEX.synchronize { @suppressed = true } + end + + private + + def with_origin(message, locations) + origin = origin_for(locations) + origin ? "#{message} (called from #{origin})" : message + end + + # First frame outside the gem's lib dir, formatted "file:line"; nil + # when every frame is internal (or paths are unavailable). + def origin_for(locations) + (locations || []).each do |location| + path = location.absolute_path || location.path + next if path.nil? || path.start_with?(GEM_LIB_DIR) + + return "#{path}:#{location.lineno}" + end + nil + end + end + end + + class << self + # @api private + attr_accessor :silence_deprecations + + # @api private + # + # @return [Boolean] true if deprecation warnings should be suppressed, + # either via the {silence_deprecations} accessor or the + # SNAP_DIFF_SILENCE_DEPRECATIONS env var (truthy = "1"/"true"). + def silence_deprecations? + !!silence_deprecations || truthy_env?(ENV["SNAP_DIFF_SILENCE_DEPRECATIONS"]) + end + + private + + def truthy_env?(value) + %w[1 true].include?(value.to_s.downcase) + end + end +end diff --git a/lib/snap_diff/utils.rb b/lib/snap_diff/utils.rb index 41646ada..48f6829e 100644 --- a/lib/snap_diff/utils.rb +++ b/lib/snap_diff/utils.rb @@ -1,9 +1,28 @@ # frozen_string_literal: true require "snap_diff/drivers" +require "snap_diff/removal" module SnapDiff module Utils + # THE selection funnel. Every surface that picks a driver ends up here -- + # `driver: :chunky_png` per comparison, `SnapDiff.config.driver =`, the + # legacy `Diff.driver =` (a delegator onto the same storage), and `:auto` + # -- so this is the one place the chunky_png removal has to be announced + # from. Warned before the registry lookup, not inside it: the cache is a + # `||=`, so a hook there would fire only for the first comparison of a + # process that happened to miss. + CHUNKY_PNG_REMOVED = + "The chunky_png driver is REMOVED in 2.1, when libvips (the `ruby-vips` gem) becomes " \ + "required. Install ruby-vips and drop `driver: :chunky_png`. See docs/drivers.md." + + # The case that matters most: nobody asked for this driver, so the + # warning has to say why they are on it. + CHUNKY_PNG_AUTO_REMOVED = + "`driver: :auto` selected chunky_png because libvips is not available in this process. " \ + "The chunky_png driver is REMOVED in 2.1, when libvips (the `ruby-vips` gem) becomes " \ + "required -- install it now, or this setup stops comparing on 2.1. See docs/drivers.md." + # 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 @@ -13,9 +32,17 @@ def self.detect_available_drivers end def self.find_driver_class_for(driver) - driver = Drivers.available.first if driver == :auto + if driver == :auto + # Drivers::AVAILABLE_DRIVERS, not Drivers.available: same value and + # the same stubbing point, without the gem tripping .available's own + # removal warning on every comparison. + driver = Drivers::AVAILABLE_DRIVERS.first + Removal.warn_once(:chunky_png_auto, CHUNKY_PNG_AUTO_REMOVED) if driver == :chunky_png + elsif driver == :chunky_png + Removal.warn_once(:chunky_png, CHUNKY_PNG_REMOVED) + end - Drivers.loaded[driver] ||= + Drivers.registry[driver] ||= case driver when :chunky_png require "snap_diff/drivers/chunky_png_driver" @@ -24,7 +51,7 @@ def self.find_driver_class_for(driver) require "snap_diff/drivers/vips_driver" SnapDiff::Drivers::VipsDriver else - fail "Wrong adapter #{driver.inspect}. Available adapters: #{Drivers.available.inspect}" + fail "Wrong adapter #{driver.inspect}. Available adapters: #{Drivers::AVAILABLE_DRIVERS.inspect}" end end end diff --git a/test/legacy/legacy_namespace_deprecation_test.rb b/test/legacy/legacy_namespace_deprecation_test.rb index b8753695..bfcf7073 100644 --- a/test/legacy/legacy_namespace_deprecation_test.rb +++ b/test/legacy/legacy_namespace_deprecation_test.rb @@ -129,6 +129,12 @@ def capture_warnings test "loading each entry point and running a trivial comparison emits no deprecation output" do failures = ENTRY_POINT_PROBES.filter_map do |entry, kind| body = "require #{entry.inspect}\n" + # The probe itself names chunky_png (the one driver present on every + # box, so the comparison below is deterministic), and naming it is a + # user choice that warns about the 2.1 removal by design. Silence THAT + # channel only -- the legacy-namespace warnings this test is actually + # about stay live. + body << "SnapDiff::Removal.suppress!\n" body << "raise \"compare failed\" unless SnapDiff.compare(#{FIXTURE_IMAGE.inspect}, #{FIXTURE_IMAGE.inspect}, driver: :chunky_png).quick_equal?\n" if kind == :legacy body << "CapybaraScreenshotDiff.assertions_present?\n" diff --git a/test/test_helper.rb b/test/test_helper.rb index 5eccadb6..191afe33 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -31,6 +31,14 @@ # per-constant warnings must still reach the guard below and raise. SnapDiff::Deprecation.suppress_migration_notice! +# Same reasoning for the OTHER half of the 2.1 story: this suite runs the +# whole comparison matrix on chunky_png, sets shift_distance_limit, and reads +# the driver registry on purpose -- it exercises those APIs rather than +# depending on them. Suppressed here, asserted in a subprocess instead +# (test/unit/removed_in_2_1_deprecation_test.rb), because "once per process" +# cannot be measured inside one long-lived process anyway. +SnapDiff::Removal.suppress! + # v2 step 8: the suite exercises only canonical SnapDiff:: names, so any # legacy-shim deprecation warning during a test run is a bug in the # referencing test -- fail loud at the resolution site instead of letting diff --git a/test/unit/canonical_suite_has_no_legacy_refs_test.rb b/test/unit/canonical_suite_has_no_legacy_refs_test.rb index d87d10b6..11f01c1a 100644 --- a/test/unit/canonical_suite_has_no_legacy_refs_test.rb +++ b/test/unit/canonical_suite_has_no_legacy_refs_test.rb @@ -61,8 +61,14 @@ class CanonicalSuiteHasNoLegacyRefsTest < ActiveSupport::TestCase # are deleted with the v1 trees (ADR-008 -- SnapDiff.configure is the # single config entry point). This is the shape of the first of the three # incidents: a canonical gate demanding SnapDiff.start. + # + # SnapDiff.silence_deprecations is deliberately NOT here: it moved to + # snap_diff/removal.rb, a file the deletion keeps, because it is the one + # switch that also silences the 2.1 removal warnings (chunky_png, + # shift_distance_limit, the driver abstraction) -- which are announced from + # core files and have nothing to do with the v1 namespaces. LEGACY_SHIM_SURFACE = / - SnapDiff\.(start|silence_deprecations)\b + SnapDiff\.start\b |SnapDiff::Deprecation\b |suppress_migration_notice /x diff --git a/test/unit/removed_in_2_1_deprecation_test.rb b/test/unit/removed_in_2_1_deprecation_test.rb new file mode 100644 index 00000000..f10cf3c7 --- /dev/null +++ b/test/unit/removed_in_2_1_deprecation_test.rb @@ -0,0 +1,249 @@ +# frozen_string_literal: true + +require "test_helper" +require "open3" + +# THE 2.1 REMOVALS, ANNOUNCED IN 2.0. +# +# 2.0 is the transitional release: the contract is published before it is +# enforced, so everything 2.1 deletes has to warn HERE, naming 2.1, while it +# still works. The legacy-namespace half of that promise is covered by +# test/legacy/; this file covers the driver half, which 2.1 removes whole: +# the chunky_png driver, `shift_distance_limit` (chunky-only, it dies with +# it), and the driver abstraction itself (`SnapDiff::Driver`, +# `SnapDiff::Drivers.loaded` / `.available`, `driver: :auto`) -- libvips +# becomes the only backend. +# +# Every example runs in a SUBPROCESS. "Once per process" is the contract, and +# this suite is a single long-lived process that selects chunky_png in +# hundreds of tests (test_helper suppresses these warnings for exactly that +# reason) -- an in-process assertion could measure neither. +class RemovedIn21DeprecationTest < ActiveSupport::TestCase + PROJECT_ROOT = File.expand_path("../..", __dir__) + IMAGE_A = File.join(PROJECT_ROOT, "test/fixtures/images/a.png") + IMAGE_B = File.join(PROJECT_ROOT, "test/fixtures/images/b.png") + + # Blocks `require "vips"` so detection reports chunky_png only -- the + # `driver: :auto` fallback a user without libvips is silently on today. + # (Same technique as drivers_test's unavailable-leaf probe.) + NO_VIPS = <<~RUBY + module Kernel + alias_method :__real_require, :require + def require(name) + raise LoadError, "cannot load such file -- vips" if name == "vips" + __real_require(name) + end + end + RUBY + + # --- the chunky_png driver ------------------------------------------ + + test "selecting chunky_png per comparison warns once, naming 2.1" do + lines = probe(<<~RUBY) + require "snap_diff" + 3.times { #{compare(driver: :chunky_png)} } + RUBY + + assert_equal 1, lines.size, lines.join + assert_match(/chunky_png/, lines.first) + assert_match(/REMOVED in 2\.1/, lines.first) + assert_match(/vips/, lines.first) + end + + test "selecting chunky_png through SnapDiff.config.driver warns once" do + lines = probe(<<~RUBY) + require "snap_diff" + SnapDiff.config.driver = :chunky_png + 3.times { #{compare} } + RUBY + + assert_equal 1, lines.size, lines.join + assert_match(/chunky_png/, lines.first) + end + + # THE CASE THAT MATTERS MOST: these users never asked for chunky_png and + # have no idea they are on it, so the warning has to say why they are. + test "the :auto fallback to chunky_png warns and says libvips is missing" do + lines = probe(<<~RUBY) + #{NO_VIPS} + require "snap_diff" + 3.times { #{compare} } + RUBY + + assert_equal 1, lines.size, lines.join + assert_match(/auto/, lines.first) + assert_match(/libvips/, lines.first) + assert_match(/REMOVED in 2\.1/, lines.first) + end + + # --- shift_distance_limit (chunky-only, dies with it) ---------------- + + test "setting shift_distance_limit on the config warns once" do + lines = probe(<<~RUBY) + require "snap_diff" + 3.times { SnapDiff.config.shift_distance_limit = 5 } + RUBY + + assert_equal 1, lines.size, lines.join + assert_match(/shift_distance_limit/, lines.first) + assert_match(/REMOVED in 2\.1/, lines.first) + end + + # Counts the shift lines rather than every line: the driver is left to + # `:auto` so this runs on a box with or without libvips, and a box without + # it legitimately gets the chunky_png fallback warning as well. + test "passing shift_distance_limit per comparison warns once" do + lines = probe(<<~RUBY) + require "snap_diff" + 3.times { #{compare(shift_distance_limit: 5)} } + RUBY + + shift = lines.grep(/shift_distance_limit/) + assert_equal 1, shift.size, lines.join + assert_match(/REMOVED in 2\.1/, shift.first) + end + + # --- the driver abstraction ------------------------------------------ + + test "reading the custom-driver registry warns once" do + lines = probe(<<~RUBY) + require "snap_diff/drivers" + 3.times { SnapDiff::Drivers.loaded[:mine] = Class.new } + RUBY + + assert_equal 1, lines.size, lines.join + assert_match(/SnapDiff::Drivers\.loaded/, lines.first) + assert_match(/REMOVED in 2\.1/, lines.first) + end + + test "reading the detected-driver list warns once" do + lines = probe(<<~RUBY) + require "snap_diff/drivers" + 3.times { SnapDiff::Drivers.available } + RUBY + + assert_equal 1, lines.size, lines.join + assert_match(/SnapDiff::Drivers\.available/, lines.first) + end + + test "a custom driver including the Driver mixin warns once" do + lines = probe(<<~RUBY) + require "snap_diff/driver" + class MyDriver + include SnapDiff::Driver + end + class MyOtherDriver + include SnapDiff::Driver + end + RUBY + + assert_equal 1, lines.size, lines.join + assert_match(/include SnapDiff::Driver/, lines.first) + assert_match(/REMOVED in 2\.1/, lines.first) + end + + # --- what must stay silent ------------------------------------------- + + # The mutation that matters for everyone who is NOT affected: a plain vips + # setup, comparing images, must not gain a single line of stderr -- and the + # gem's own drivers include the mixin themselves, so an unscoped `included` + # hook would fire here. + test "a plain vips setup with no chunky or shift usage stays silent" do + skip "libvips not available on this box" unless SnapDiff::Drivers::AVAILABLE_DRIVERS.include?(:vips) + + out = probe_stderr(<<~RUBY) + require "snap_diff" + SnapDiff.config.driver = :vips + 3.times { #{compare} } + SnapDiff::Drivers::VipsDriver + RUBY + + assert_equal "", out, "a vips-only setup must not warn" + end + + # The other half of "the gem must not warn at itself": on a box with NO + # libvips, every internal load path runs through chunky_png. Loading the + # gem still has to be silent -- the warning belongs to the first + # comparison the user asks for, not to `require`. + test "loading the gem selects nothing and stays silent even without libvips" do + out = probe_stderr(<<~RUBY) + #{NO_VIPS} + require "snap_diff" + SnapDiff.config + RUBY + + assert_equal "", out, "requiring the gem must not select a driver" + end + + test "every warning fires exactly once per process, however many surfaces are touched" do + lines = probe(<<~RUBY) + require "snap_diff" + 3.times do + SnapDiff.config.shift_distance_limit = 5 + SnapDiff.config.driver = :chunky_png + #{compare} + SnapDiff::Drivers.loaded + SnapDiff::Drivers.available + Class.new { include SnapDiff::Driver } + end + RUBY + + assert_equal 5, lines.size, lines.join + assert_equal 5, lines.uniq.size, "duplicate warning text: #{lines.join}" + end + + # --- silencing -------------------------------------------------------- + + test "silenced by the SnapDiff.silence_deprecations accessor" do + out = probe_stderr(<<~RUBY) + require "snap_diff" + SnapDiff.silence_deprecations = true + SnapDiff.config.shift_distance_limit = 5 + #{compare(driver: :chunky_png)} + SnapDiff::Drivers.loaded + SnapDiff::Drivers.available + Class.new { include SnapDiff::Driver } + RUBY + + assert_equal "", out + end + + test "silenced by SNAP_DIFF_SILENCE_DEPRECATIONS" do + out = probe_stderr(<<~RUBY, "SNAP_DIFF_SILENCE_DEPRECATIONS" => "1") + require "snap_diff" + SnapDiff.config.shift_distance_limit = 5 + #{compare(driver: :chunky_png)} + SnapDiff::Drivers.loaded + SnapDiff::Drivers.available + Class.new { include SnapDiff::Driver } + RUBY + + assert_equal "", out + end + + private + + # `SnapDiff.compare(base, new, **options)` against two real fixtures -- + # the documented entry point, so the probes exercise driver selection the + # way an adopter reaches it rather than by poking at internals. + def compare(**options) + args = [IMAGE_A.inspect, IMAGE_B.inspect] + options.each { |key, value| args << "#{key}: #{value.inspect}" } + "SnapDiff.compare(#{args.join(", ")})" + end + + def probe(script, env = {}) + probe_stderr(script, env).lines.reject { |line| line.strip.empty? } + end + + # Runs +script+ in a fresh process with only lib/ on the load path and + # returns this gem's warnings from its stderr (other gems' warnings are + # none of this test's business). + def probe_stderr(script, env = {}) + _out, err, status = Open3.capture3( + env, RbConfig.ruby, "-Ilib", "-e", script, chdir: PROJECT_ROOT + ) + assert_predicate status, :success?, err + err.lines.grep(/\[snap_diff/).join + end +end