From fbabb1cfbf646a6debe268d8f65033e3fdc34006 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Tue, 14 Apr 2026 12:14:23 +0200 Subject: [PATCH 1/3] feat: standardize screenshot consistency --- .../setup-ruby-and-dependencies/action.yml | 20 ++++- README.md | 8 +- docs/adr/0001-screenshot-prep-plugins.md | 47 +++++++++++ docs/adr/README.md | 5 ++ docs/ci-integration.md | 8 +- docs/configuration.md | 77 ++++++++++++++++++ docs/os-setup.md | 72 +++++++++++++++++ .../screenshot/diff/browser_helpers.rb | 81 +++++++++++++++++++ .../screenshot/diff/screenshot_matcher.rb | 3 + lib/capybara/screenshot/diff/screenshoter.rb | 36 +++++++++ lib/capybara_screenshot_diff.rb | 80 ++++++++++++++++++ lib/capybara_screenshot_diff/dsl.rb | 3 + test/test_helper.rb | 4 + test/unit/screenshot_test.rb | 19 +++++ test/unit/screenshoter_test.rb | 26 ++++++ 15 files changed, 483 insertions(+), 6 deletions(-) create mode 100644 docs/adr/0001-screenshot-prep-plugins.md create mode 100644 docs/adr/README.md create mode 100644 docs/os-setup.md diff --git a/.github/actions/setup-ruby-and-dependencies/action.yml b/.github/actions/setup-ruby-and-dependencies/action.yml index ed27ed77..848c5075 100644 --- a/.github/actions/setup-ruby-and-dependencies/action.yml +++ b/.github/actions/setup-ruby-and-dependencies/action.yml @@ -26,13 +26,27 @@ runs: if: ${{ inputs.cache-apt-packages == 'true' }} uses: jetthoughts/cache-apt-pkgs-action@fix/upgrade-actions-cache-v5 with: - packages: libvips libglib2.0-0 libglib2.0-dev libwebp-dev libvips42 libpng-dev + packages: libvips libglib2.0-0 libglib2.0-dev libwebp-dev libvips42 libpng-dev fonts-dejavu fonts-liberation fonts-ubuntu fonts-noto-color-emoji version: tests-v2 - name: Install vips (fallback) if: ${{ inputs.cache-apt-packages != 'true' }} - run: sudo apt-get -qq update && sudo apt-get -qq install -y libvips + run: sudo apt-get -qq update && sudo apt-get -qq install -y libvips fonts-dejavu fonts-liberation fonts-ubuntu fonts-noto-color-emoji shell: bash - - run: sudo sed -i 's/true/false/g' /etc/fonts/conf.d/10-yes-antialias.conf + - name: Configure font rendering + run: | + sudo tee /etc/fonts/local.conf >/dev/null <<'XML' + + + + + false + false + hintnone + none + + + XML + sudo fc-cache -f shell: bash diff --git a/README.md b/README.md index e7e71fa8..faaff0a6 100644 --- a/README.md +++ b/README.md @@ -187,7 +187,13 @@ Enable `Capybara::Screenshot.disable_animations = true` to freeze CSS animations
CI screenshots differ from local -Set `window_size` for consistent dimensions and use `perceptual_threshold: 2.0` to ignore anti-aliasing differences across environments. +Set `window_size` for consistent dimensions and use `perceptual_threshold: 2.0` to ignore anti-aliasing differences across environments. For cross-OS baselines, use the preset: + +```ruby +Capybara::Screenshot.enable_consistent_screenshots! +``` + +For advanced tuning and custom injections, see `docs/configuration.md`.
diff --git a/docs/adr/0001-screenshot-prep-plugins.md b/docs/adr/0001-screenshot-prep-plugins.md new file mode 100644 index 00000000..2a758c27 --- /dev/null +++ b/docs/adr/0001-screenshot-prep-plugins.md @@ -0,0 +1,47 @@ +# ADR 0001: Screenshot Preparation Plugins (Deferred) + +Date: 2026-04-14 +Status: Deferred + +## Context + +We added more pre-capture steps for stable screenshots, including: +- DOM normalization CSS +- Font readiness waits +- Animation disabling +- Caret hiding +- Custom CSS and JS injections + +The number of knobs and prep steps is growing. A plugin pipeline could make +ordering explicit and allow app-level extensions without monkey-patching. + +## Decision + +Defer a plugin system for now. + +We will keep the simple `configure_consistency` API plus existing flags as +aliases. This provides a single entry point and keeps complexity low. + +## Consequences + +Benefits: +- Minimal code and API surface +- Easy onboarding for new users + +Costs: +- Prep order is still encoded in `Screenshoter` +- Extensibility is limited to CSS/JS injections and flags + +## Revisit Criteria + +Re-open this decision when: +- We need more than one custom preparation step per app +- We add two or more new prep steps beyond CSS/JS injection +- Prep ordering or conditional logic becomes hard to reason about + +## Next Refactoring Ideas + +If revisited, implement a lightweight pipeline: +- `plugins` list with callables +- Context object with `inject_css`, `inject_js`, `wait_for_fonts`, and `session` +- Built-in plugins for existing steps, mapped from current flags diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 00000000..8096e6af --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,5 @@ +# Architecture Decision Records + +## ADR Index + +- [ADR 0001: Screenshot Preparation Plugins (Deferred)](0001-screenshot-prep-plugins.md) diff --git a/docs/ci-integration.md b/docs/ci-integration.md index 3f8f3ec5..6e870bc1 100644 --- a/docs/ci-integration.md +++ b/docs/ci-integration.md @@ -90,7 +90,8 @@ That's it. On failure, this will: ### 3. Ruby + libvips setup action -For consistent CI environments (libvips, font antialiasing disabled), use the setup action: +For consistent CI environments (libvips, standardized fonts, hinting disabled), +use the setup action: ```yaml - uses: snap-diff/snap_diff-capybara/.github/actions/setup-ruby-and-dependencies@master @@ -99,7 +100,10 @@ For consistent CI environments (libvips, font antialiasing disabled), use the se cache-apt-packages: true ``` -This installs Ruby, libvips (with apt caching), and disables font antialiasing for consistent rendering across CI runs. +This installs Ruby, libvips (with apt caching), installs the core font stack, +and disables font hinting for consistent rendering across CI runs. + +For local or Docker setups, see `docs/os-setup.md`. #### Inputs diff --git a/docs/configuration.md b/docs/configuration.md index e540ae08..50855446 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -135,6 +135,83 @@ If you want to skip the assertion for change in the screen shot, set Capybara::Screenshot::Diff.enabled = false ``` +### DOM normalization (recommended for cross-OS baselines) + +To reduce visual noise from OS-level font rendering, scrollbars, and UI chrome, +the default config injects a normalization stylesheet and waits for web fonts +before capture. You can override as needed: + +```ruby +Capybara::Screenshot.normalize_css = false +Capybara::Screenshot.wait_for_fonts = false +``` + +The built-in normalization stylesheet: +- disables animations/transitions +- standardizes font rendering +- hides carets and number spinners +- hides OS-specific scrollbars + +To supply custom CSS instead: + +```ruby +Capybara::Screenshot.normalize_css = true +Capybara::Screenshot.normalize_stylesheet = <<~CSS + /* your custom normalization rules */ +CSS +``` + +### One-line preset + +If you'd rather toggle everything in one call: + +```ruby +Capybara::Screenshot.enable_consistent_screenshots! +``` + +### Unified consistency config (recommended) + +For a single entry point with custom injections: + +```ruby +Capybara::Screenshot.configure_consistency(preset: :default) do |c| + c.blur_active_element = true + c.hide_caret = true + c.disable_animations = true + c.normalize_css = true + c.wait_for_fonts = true + c.css << "/* your custom css */" + c.js << "/* your custom js */" +end +``` + +Available presets: +- `:default` (enable normalization + font wait + disable animations + hide caret + blur) +- `:off` (disable all consistency shims) + +**Compatibility:** existing flags (`normalize_css`, `wait_for_fonts`, `disable_animations`, etc.) +remain supported as aliases. + +For OS-level setup (fonts + hinting), see `docs/os-setup.md`. + +### Cross-OS baseline preset (Ubuntu ↔ Alpine) + +If you compare baselines across `glibc` and `musl`, combine perceptual diffing +with a tighter tolerated diff area: + +```ruby +Capybara::Screenshot::Diff.configure do |screenshot, diff| + screenshot.window_size = [1280, 1024] + screenshot.disable_animations = true + screenshot.normalize_css = true + screenshot.wait_for_fonts = true + + diff.driver = :vips + diff.perceptual_threshold = 2.0 + diff.tolerance = 0.00005 # 0.005% of pixels +end +``` + Using an environment variable ```ruby diff --git a/docs/os-setup.md b/docs/os-setup.md new file mode 100644 index 00000000..0d790d87 --- /dev/null +++ b/docs/os-setup.md @@ -0,0 +1,72 @@ +# OS Setup for Consistent Screenshots + +Screenshot rendering varies across OS and C libraries. For reliable baselines +across Ubuntu (glibc) and Alpine (musl), standardize fonts and disable hinting. + +## Ubuntu (glibc) + +Install fonts: + +```bash +sudo apt-get update +sudo apt-get install -y \ + fonts-dejavu \ + fonts-liberation \ + fonts-ubuntu \ + fonts-noto-color-emoji +``` + +Disable font hinting and subpixel tweaks: + +```bash +sudo tee /etc/fonts/local.conf >/dev/null <<'XML' + + + + + false + false + hintnone + none + + +XML + +sudo fc-cache -f +``` + +## Alpine (musl) + +Install fonts: + +```bash +apk add --no-cache \ + ttf-dejavu \ + ttf-liberation \ + ttf-ubuntu-font-family \ + font-noto-emoji +``` + +Disable font hinting and subpixel tweaks: + +```bash +cat > /etc/fonts/local.conf <<'XML' + + + + + false + false + hintnone + none + + +XML + +fc-cache -f +``` + +## GitHub Actions (Ubuntu) + +If you use the provided setup action, the OS preparation is handled for you. +See `docs/ci-integration.md` for the GitHub Actions snippets. diff --git a/lib/capybara/screenshot/diff/browser_helpers.rb b/lib/capybara/screenshot/diff/browser_helpers.rb index 80ac7289..bd257603 100644 --- a/lib/capybara/screenshot/diff/browser_helpers.rb +++ b/lib/capybara/screenshot/diff/browser_helpers.rb @@ -73,6 +73,76 @@ def self.disable_animations session.execute_script(DISABLE_ANIMATIONS_SCRIPT) end + DEFAULT_NORMALIZE_CSS = <<~CSS + /* Kill animations and transitions */ + *, *::before, *::after { + animation-duration: 0s !important; + animation-delay: 0s !important; + animation-iteration-count: 1 !important; + transition-duration: 0s !important; + transition-delay: 0s !important; + scroll-behavior: auto !important; + } + + /* Standardize font rendering */ + * { + -webkit-font-smoothing: antialiased !important; + -moz-osx-font-smoothing: grayscale !important; + text-rendering: geometricPrecision !important; + } + + /* Neutralize inputs and dynamic artifacts */ + * { caret-color: transparent !important; } + input[type="number"]::-webkit-inner-spin-button, + input[type="number"]::-webkit-outer-spin-button { + -webkit-appearance: none !important; + } + + /* Hide OS-specific scrollbars */ + ::-webkit-scrollbar { display: none !important; } + * { scrollbar-width: none !important; -ms-overflow-style: none !important; } + CSS + + def self.normalize_css(css = nil) + css ||= DEFAULT_NORMALIZE_CSS + + session.execute_script(<<~JS, css) + (function(cssText) { + if (!document.getElementById('csdNormalizeStyle')) { + let style = document.createElement('style'); + style.setAttribute('id', 'csdNormalizeStyle'); + style.textContent = cssText; + document.head.appendChild(style); + } + })(arguments[0]); + JS + end + + def self.inject_custom_stylesheets(stylesheets) + Array(stylesheets).each_with_index do |css, index| + inject_stylesheet(css, "csdCustomStyle#{index}") + end + end + + def self.inject_custom_scripts(scripts) + Array(scripts).each do |script| + session.execute_script(script) + end + end + + def self.inject_stylesheet(css, element_id) + session.execute_script(<<~JS, css, element_id) + (function(cssText, styleId) { + if (!document.getElementById(styleId)) { + let style = document.createElement('style'); + style.setAttribute('id', styleId); + style.textContent = cssText; + document.head.appendChild(style); + } + })(arguments[0], arguments[1]); + JS + end + FIND_ACTIVE_ELEMENT_SCRIPT = <<~JS function activeElement(){ const ae = document.activeElement; @@ -113,6 +183,17 @@ def self.pending_image_to_load BrowserHelpers.session.evaluate_script(IMAGE_WAIT_SCRIPT) end + FONTS_READY_SCRIPT = <<~JS + (function() { + if (!document.fonts) return true; + return document.fonts.status === "loaded"; + })(); + JS + + def self.fonts_ready? + BrowserHelpers.session.evaluate_script(FONTS_READY_SCRIPT) + end + def self.current_capybara_driver_class session.driver.class end diff --git a/lib/capybara/screenshot/diff/screenshot_matcher.rb b/lib/capybara/screenshot/diff/screenshot_matcher.rb index 2b5309e2..97713ab4 100644 --- a/lib/capybara/screenshot/diff/screenshot_matcher.rb +++ b/lib/capybara/screenshot/diff/screenshot_matcher.rb @@ -102,6 +102,9 @@ def extract_capture_and_comparison_options!(driver_options = {}) # screenshot options capybara_screenshot_options: driver_options[:capybara_screenshot_options], crop: driver_options.delete(:crop), + normalize_css: driver_options.delete(:normalize_css), + normalize_stylesheet: driver_options.delete(:normalize_stylesheet), + wait_for_fonts: driver_options.delete(:wait_for_fonts), # delivery options screenshot_format: driver_options[:screenshot_format], # stability options diff --git a/lib/capybara/screenshot/diff/screenshoter.rb b/lib/capybara/screenshot/diff/screenshoter.rb index bfd48860..35dfc6ca 100644 --- a/lib/capybara/screenshot/diff/screenshoter.rb +++ b/lib/capybara/screenshot/diff/screenshoter.rb @@ -69,6 +69,11 @@ def notice_how_to_avoid_this def prepare_page_for_screenshot(timeout:) wait_images_loaded(timeout: timeout) if timeout + wait_for_fonts(timeout: timeout) if wait_for_fonts? + + BrowserHelpers.normalize_css(normalize_stylesheet) if normalize_css? + BrowserHelpers.inject_custom_stylesheets(Screenshot.custom_stylesheets) + BrowserHelpers.inject_custom_scripts(Screenshot.custom_scripts) blurred_input = BrowserHelpers.blur_from_focused_element if Screenshot.blur_active_element @@ -94,8 +99,39 @@ def wait_images_loaded(timeout:) end end + def wait_for_fonts(timeout:) + return unless timeout + + deadline_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) + timeout + loop do + break if BrowserHelpers.fonts_ready? + + if Process.clock_gettime(Process::CLOCK_MONOTONIC) > deadline_at + raise CapybaraScreenshotDiff::ExpectationNotMet.new("Fonts have not been loaded after #{timeout}s", caller) + end + + sleep 0.025 + end + end + private + def normalize_css? + return @capture_options[:normalize_css] unless @capture_options[:normalize_css].nil? + + Screenshot.normalize_css + end + + def normalize_stylesheet + @capture_options.key?(:normalize_stylesheet) ? @capture_options[:normalize_stylesheet] : Screenshot.normalize_stylesheet + end + + def wait_for_fonts? + return @capture_options[:wait_for_fonts] unless @capture_options[:wait_for_fonts].nil? + + Screenshot.wait_for_fonts + end + def save_and_process_screenshot(screenshot_path) tmpfile = Tempfile.new([screenshot_path.basename.to_s, PNG_EXTENSION]) BrowserHelpers.session.save_screenshot(tmpfile.path, **capybara_screenshot_options) diff --git a/lib/capybara_screenshot_diff.rb b/lib/capybara_screenshot_diff.rb index f275e32b..e08ddb69 100644 --- a/lib/capybara_screenshot_diff.rb +++ b/lib/capybara_screenshot_diff.rb @@ -31,6 +31,11 @@ module Screenshot mattr_accessor :enabled mattr_accessor(:hide_caret) { true } mattr_accessor :disable_animations + mattr_accessor(:normalize_css) { true } + mattr_accessor :normalize_stylesheet + mattr_accessor(:wait_for_fonts) { true } + mattr_accessor(:custom_stylesheets) { [] } + mattr_accessor(:custom_scripts) { [] } mattr_reader(:root) { (defined?(Rails) && defined?(Rails.root) && Rails.root) || Pathname(".").expand_path } mattr_accessor :stability_time_limit mattr_accessor :window_size @@ -40,6 +45,81 @@ module Screenshot mattr_accessor(:capybara_screenshot_options) { {} } class << self + class ConsistencyConfig + attr_accessor :css, :js, + :normalize_css, :wait_for_fonts, :disable_animations, + :hide_caret, :blur_active_element + + def initialize( + css:, + js:, + normalize_css:, + wait_for_fonts:, + disable_animations:, + hide_caret:, + blur_active_element: + ) + @css = css + @js = js + @normalize_css = normalize_css + @wait_for_fonts = wait_for_fonts + @disable_animations = disable_animations + @hide_caret = hide_caret + @blur_active_element = blur_active_element + end + + def apply_to_screenshot!(screenshot) + screenshot.normalize_css = normalize_css + screenshot.wait_for_fonts = wait_for_fonts + screenshot.disable_animations = disable_animations + screenshot.hide_caret = hide_caret + screenshot.blur_active_element = blur_active_element + end + end + + def enable_consistent_screenshots!( + normalize_css: true, + wait_for_fonts: true, + disable_animations: true, + hide_caret: true, + blur_active_element: true + ) + self.normalize_css = normalize_css + self.wait_for_fonts = wait_for_fonts + self.disable_animations = disable_animations + self.hide_caret = hide_caret + self.blur_active_element = blur_active_element + end + + def configure_consistency(preset: :default) + case preset + when :default + enable_consistent_screenshots! + when :off + enable_consistent_screenshots!( + normalize_css: false, + wait_for_fonts: false, + disable_animations: false, + hide_caret: false, + blur_active_element: false + ) + else + raise ArgumentError, "Unknown consistency preset: #{preset.inspect}" + end + + config = ConsistencyConfig.new( + css: custom_stylesheets, + js: custom_scripts, + normalize_css: normalize_css, + wait_for_fonts: wait_for_fonts, + disable_animations: disable_animations, + hide_caret: hide_caret, + blur_active_element: blur_active_element + ) + yield config if block_given? + config.apply_to_screenshot!(self) + end + def root=(path) @@root = Pathname(path).expand_path end diff --git a/lib/capybara_screenshot_diff/dsl.rb b/lib/capybara_screenshot_diff/dsl.rb index 96db07fe..59113b87 100644 --- a/lib/capybara_screenshot_diff/dsl.rb +++ b/lib/capybara_screenshot_diff/dsl.rb @@ -49,6 +49,9 @@ def screenshot_group(name) # @option options [Numeric] :shift_distance_limit Maximum allowed shift distance for pixels. # @option options [Numeric] :area_size_limit Maximum allowed difference area size in pixels. # @option options [Symbol] :driver (:auto) The image processing driver to use (:auto, :chunky_png, :vips). + # @option options [Boolean] :normalize_css Inject a CSS normalization layer before capturing. + # @option options [String] :normalize_stylesheet Custom CSS to inject when :normalize_css is enabled. + # @option options [Boolean] :wait_for_fonts Wait for document fonts to load before capturing. # @return [Boolean] True if the screenshot was successfully captured and processed. # @raise [CapybaraScreenshotDiff::ExpectationNotMet] If comparison fails and immediate validation is enabled. # @raise [CapybaraScreenshotDiff::UnstableImage] If the image comparison is unstable. diff --git a/test/test_helper.rb b/test/test_helper.rb index 0d9abbc0..826b89c0 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -58,6 +58,10 @@ class ActiveSupport::TestCase Capybara::Screenshot.blur_active_element = false Capybara::Screenshot.hide_caret = false Capybara::Screenshot.disable_animations = false + Capybara::Screenshot.normalize_css = false + Capybara::Screenshot.wait_for_fonts = false + Capybara::Screenshot.custom_stylesheets = [] + Capybara::Screenshot.custom_scripts = [] end teardown do diff --git a/test/unit/screenshot_test.rb b/test/unit/screenshot_test.rb index 718a76a0..06213045 100644 --- a/test/unit/screenshot_test.rb +++ b/test/unit/screenshot_test.rb @@ -24,5 +24,24 @@ class ScreenshotTest < ActiveSupport::TestCase ensure Capybara::Screenshot.root = @orig_root if @orig_root end + + test "configure_consistency presets defaults and allows custom injections" do + Capybara::Screenshot.custom_stylesheets = [] + Capybara::Screenshot.custom_scripts = [] + + Capybara::Screenshot.configure_consistency(preset: :off) do |c| + c.css << ".csd-test { color: red; }" + c.js << "window.__csd_test = true;" + c.hide_caret = true + end + + assert_equal false, Capybara::Screenshot.normalize_css + assert_equal false, Capybara::Screenshot.wait_for_fonts + assert_equal false, Capybara::Screenshot.disable_animations + assert_equal true, Capybara::Screenshot.hide_caret + assert_equal false, Capybara::Screenshot.blur_active_element + assert_includes Capybara::Screenshot.custom_stylesheets, ".csd-test { color: red; }" + assert_includes Capybara::Screenshot.custom_scripts, "window.__csd_test = true;" + end end end diff --git a/test/unit/screenshoter_test.rb b/test/unit/screenshoter_test.rb index 4fee149c..a8d45052 100644 --- a/test/unit/screenshoter_test.rb +++ b/test/unit/screenshoter_test.rb @@ -47,6 +47,32 @@ class ScreenshoterTest < ActiveSupport::TestCase assert_nil screenshoter.prepare_page_for_screenshot(timeout: nil) # does not raise an error end + + test "#prepare_page_for_screenshot injects normalization and custom hooks in order" do + Capybara::Screenshot.normalize_css = true + Capybara::Screenshot.custom_stylesheets = ["body { color: red; }"] + Capybara::Screenshot.custom_scripts = ["window.__csd = true;"] + Capybara::Screenshot.blur_active_element = false + Capybara::Screenshot.hide_caret = false + Capybara::Screenshot.disable_animations = false + + screenshoter = Screenshoter.new({wait: nil}, {driver: :chunky_png}) + calls = [] + + BrowserHelpers.stub(:normalize_css, ->(css = nil) { calls << [:normalize_css, css] }) do + BrowserHelpers.stub(:inject_custom_stylesheets, ->(styles) { calls << [:inject_custom_stylesheets, styles] }) do + BrowserHelpers.stub(:inject_custom_scripts, ->(scripts) { calls << [:inject_custom_scripts, scripts] }) do + screenshoter.prepare_page_for_screenshot(timeout: nil) + end + end + end + + assert_equal [ + [:normalize_css, nil], + [:inject_custom_stylesheets, ["body { color: red; }"]], + [:inject_custom_scripts, ["window.__csd = true;"]] + ], calls + end end end end From 63be6c8da0185513cb8e5e7b24cb8d6dcd3c299e Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Tue, 14 Apr 2026 12:17:20 +0200 Subject: [PATCH 2/3] fix: ignore git env in vcs checkout --- lib/capybara/screenshot/diff/vcs.rb | 9 +++++---- test/unit/vcs_test.rb | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/lib/capybara/screenshot/diff/vcs.rb b/lib/capybara/screenshot/diff/vcs.rb index 13dd7d8f..8bd6995d 100644 --- a/lib/capybara/screenshot/diff/vcs.rb +++ b/lib/capybara/screenshot/diff/vcs.rb @@ -9,7 +9,8 @@ module Diff module Vcs def self.checkout_vcs(root, screenshot_path, checkout_path) root_path = root.to_s - git_root, _, status = Open3.capture3("git", "-C", root_path, "rev-parse", "--show-toplevel") + git_env = { "GIT_DIR" => nil, "GIT_WORK_TREE" => nil, "GIT_INDEX_FILE" => nil } + git_root, _, status = Open3.capture3(git_env, "git", "-C", root_path, "rev-parse", "--show-toplevel") return false unless status.success? git_root = git_root.chomp @@ -17,13 +18,13 @@ def self.checkout_vcs(root, screenshot_path, checkout_path) if Screenshot.use_lfs tmp_path = "#{checkout_path}.tmp" - success = system("git", "-C", root_path, "show", "HEAD:#{vcs_file_path}", out: tmp_path, err: File::NULL) + success = system(git_env, "git", "-C", root_path, "show", "HEAD:#{vcs_file_path}", out: tmp_path, err: File::NULL) if success - system("git", "-C", root_path, "lfs", "smudge", in: tmp_path, out: checkout_path.to_s, err: File::NULL) + system(git_env, "git", "-C", root_path, "lfs", "smudge", in: tmp_path, out: checkout_path.to_s, err: File::NULL) end File.delete(tmp_path) if File.exist?(tmp_path) else - success = system("git", "-C", root_path, "show", "HEAD:#{vcs_file_path}", out: checkout_path.to_s, err: File::NULL) + success = system(git_env, "git", "-C", root_path, "show", "HEAD:#{vcs_file_path}", out: checkout_path.to_s, err: File::NULL) end unless success diff --git a/test/unit/vcs_test.rb b/test/unit/vcs_test.rb index f85b5159..20765370 100644 --- a/test/unit/vcs_test.rb +++ b/test/unit/vcs_test.rb @@ -32,6 +32,30 @@ class VcsTest < ActiveSupport::TestCase assert base_screenshot_path.exist? assert_equal screenshot_path.size, base_screenshot_path.size end + + test "#checkout_vcs ignores external git env overrides" do + screenshot_path = file_fixture("images/a.png") + base_screenshot_path = Pathname.new(@base_screenshot.path) + + with_env("GIT_DIR" => "/tmp/does-not-exist", "GIT_WORK_TREE" => "/tmp/does-not-exist") do + assert Vcs.checkout_vcs(@tmp_dir, screenshot_path, base_screenshot_path), + "checkout_vcs failed with overridden git env: root=#{@tmp_dir}" + end + + assert base_screenshot_path.exist? + assert_equal screenshot_path.size, base_screenshot_path.size + end + + private + + def with_env(vars) + previous = vars.transform_values { |_, _| nil } + vars.each_key { |key| previous[key] = ENV[key] } + vars.each { |key, value| ENV[key] = value } + yield + ensure + previous.each { |key, value| ENV[key] = value } + end end end end From e1fe15e4faba6d5091dd086dec1403ee6dc4bdcc Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Tue, 14 Apr 2026 12:17:47 +0200 Subject: [PATCH 3/3] chore: fix rubocop formatting --- lib/capybara/screenshot/diff/vcs.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/capybara/screenshot/diff/vcs.rb b/lib/capybara/screenshot/diff/vcs.rb index 8bd6995d..00a13fb8 100644 --- a/lib/capybara/screenshot/diff/vcs.rb +++ b/lib/capybara/screenshot/diff/vcs.rb @@ -9,7 +9,7 @@ module Diff module Vcs def self.checkout_vcs(root, screenshot_path, checkout_path) root_path = root.to_s - git_env = { "GIT_DIR" => nil, "GIT_WORK_TREE" => nil, "GIT_INDEX_FILE" => nil } + git_env = {"GIT_DIR" => nil, "GIT_WORK_TREE" => nil, "GIT_INDEX_FILE" => nil} git_root, _, status = Open3.capture3(git_env, "git", "-C", root_path, "rev-parse", "--show-toplevel") return false unless status.success?