From b48483369de55620afd729945ae8916c4808d9e8 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Tue, 30 Jul 2024 22:41:29 +0200
Subject: [PATCH 01/23] refact: cleanup redundant code and make explicit names

---
 lib/capybara/screenshot/diff/screenshot_matcher.rb | 7 ++++---
 lib/capybara/screenshot/diff/screenshoter.rb       | 3 +--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/capybara/screenshot/diff/screenshot_matcher.rb b/lib/capybara/screenshot/diff/screenshot_matcher.rb
index ef075937..913fd973 100644
--- a/lib/capybara/screenshot/diff/screenshot_matcher.rb
+++ b/lib/capybara/screenshot/diff/screenshot_matcher.rb
@@ -55,7 +55,8 @@ def build_screenshot_matches_job
           }
 
           # Load new screenshot from Browser
-          take_comparison_screenshot(capture_options, driver_options, screenshot_path)
+          comparison_options = driver_options
+          take_comparison_screenshot(capture_options, comparison_options, screenshot_path)
 
           # Pre-computation: No need to compare without base screenshot
           return unless base_screenshot_path.exist?
@@ -81,8 +82,8 @@ def create_output_directory_for(screenshot_path)
         # Try to get screenshot from browser.
         # On `stability_time_limit` it checks that page stop updating by comparison several screenshot attempts
         # On reaching `wait` limit then it has been failed. On failing we annotate screenshot attempts to help to debug
-        def take_comparison_screenshot(capture_options, driver_options, screenshot_path)
-          screenshoter = build_screenshoter_for(capture_options, driver_options)
+        def take_comparison_screenshot(capture_options, comparison_options, screenshot_path)
+          screenshoter = build_screenshoter_for(capture_options, comparison_options)
           screenshoter.take_comparison_screenshot(screenshot_path)
         end
 
diff --git a/lib/capybara/screenshot/diff/screenshoter.rb b/lib/capybara/screenshot/diff/screenshoter.rb
index 0042f9ff..826bf4da 100644
--- a/lib/capybara/screenshot/diff/screenshoter.rb
+++ b/lib/capybara/screenshot/diff/screenshoter.rb
@@ -6,11 +6,10 @@
 module Capybara
   module Screenshot
     class Screenshoter
-      attr_reader :capture_options, :comparison_options, :driver
+      attr_reader :capture_options, :driver
 
       def initialize(capture_options, driver)
         @capture_options = capture_options
-        @comparison_options = comparison_options
         @driver = driver
       end
 

From 2da97eed7425e4604f50d7064e56eb248d62ebb3 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 00:10:07 +0200
Subject: [PATCH 02/23] refact: extracted snap manager

---
 .../screenshot/diff/screenshot_matcher.rb     |  28 ++--
 lib/capybara/screenshot/diff/screenshoter.rb  |   9 +-
 lib/capybara/screenshot/diff/vcs.rb           |   8 +-
 lib/capybara_screenshot_diff.rb               |   4 +-
 lib/capybara_screenshot_diff/dsl.rb           |   1 +
 lib/capybara_screenshot_diff/snap_manager.rb  | 122 ++++++++++++++++++
 test/integration/browser_screenshot_test.rb   |   2 +-
 test/support/stub_test_methods.rb             |   4 +-
 test/test_helper.rb                           |   4 +-
 9 files changed, 150 insertions(+), 32 deletions(-)
 create mode 100644 lib/capybara_screenshot_diff/snap_manager.rb

diff --git a/lib/capybara/screenshot/diff/screenshot_matcher.rb b/lib/capybara/screenshot/diff/screenshot_matcher.rb
index 913fd973..80dc5507 100644
--- a/lib/capybara/screenshot/diff/screenshot_matcher.rb
+++ b/lib/capybara/screenshot/diff/screenshot_matcher.rb
@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require "capybara_screenshot_diff/snap_manager"
 require_relative "screenshoter"
 require_relative "stable_screenshoter"
 require_relative "browser_helpers"
@@ -10,15 +11,22 @@ module Capybara
   module Screenshot
     module Diff
       class ScreenshotMatcher
-        attr_reader :screenshot_full_name, :driver_options, :screenshot_path, :base_screenshot_path, :screenshot_format
+        attr_reader :screenshot_full_name, :driver_options, :screenshot_format
 
         def initialize(screenshot_full_name, options = {})
           @screenshot_full_name = screenshot_full_name
           @driver_options = Diff.default_options.merge(options)
 
           @screenshot_format = @driver_options[:screenshot_format]
-          @screenshot_path = Screenshot.screenshot_area_abs / Pathname.new(screenshot_full_name).sub_ext(".#{screenshot_format}")
-          @base_screenshot_path = ScreenshotMatcher.base_image_path_from(@screenshot_path)
+          @snapshot = CapybaraScreenshotDiff::SnapManager.snapshot(screenshot_full_name, @screenshot_format)
+        end
+
+        def screenshot_path
+          @snapshot.screenshot_path
+        end
+
+        def base_screenshot_path
+          @snapshot.base_screenshot_path
         end
 
         def build_screenshot_matches_job
@@ -32,12 +40,8 @@ def build_screenshot_matches_job
           # TODO: Move this into screenshot stage, in order to re-evaluate coordinates after page updates
           # Allow nil or single or multiple areas
           driver_options[:skip_area] = area_calculator.calculate_skip_area
-
           driver_options[:driver] = Drivers.for(driver_options[:driver])
 
-          # Load base screenshot from VCS
-          create_output_directory_for(screenshot_path) unless screenshot_path.exist?
-
           checkout_base_screenshot
 
           # When fail_if_new is true no need to create screenshot if base screenshot is missing
@@ -65,18 +69,10 @@ def build_screenshot_matches_job
           [screenshot_full_name, ImageCompare.new(screenshot_path, base_screenshot_path, driver_options)]
         end
 
-        def self.base_image_path_from(screenshot_path)
-          screenshot_path.sub_ext(".base#{screenshot_path.extname}")
-        end
-
         private
 
         def checkout_base_screenshot
-          Vcs.checkout_vcs(screenshot_path, base_screenshot_path)
-        end
-
-        def create_output_directory_for(screenshot_path)
-          screenshot_path.dirname.mkpath
+          CapybaraScreenshotDiff::SnapManager.checkout_base_screenshot(screenshot_path)
         end
 
         # Try to get screenshot from browser.
diff --git a/lib/capybara/screenshot/diff/screenshoter.rb b/lib/capybara/screenshot/diff/screenshoter.rb
index 826bf4da..ccf0812e 100644
--- a/lib/capybara/screenshot/diff/screenshoter.rb
+++ b/lib/capybara/screenshot/diff/screenshoter.rb
@@ -30,12 +30,11 @@ def capybara_screenshot_options
       end
 
       def self.attempts_screenshot_paths(base_file)
-        extname = Pathname.new(base_file).extname
-        Dir["#{base_file.to_s.chomp(extname)}.attempt_*#{extname}"].sort
+        CapybaraScreenshotDiff::SnapManager.attempts_screenshot_paths(base_file)
       end
 
       def self.cleanup_attempts_screenshots(base_file)
-        FileUtils.rm_rf attempts_screenshot_paths(base_file)
+        CapybaraScreenshotDiff::SnapManager.cleanup_attempts_screenshots(base_file)
       end
 
       # Try to get screenshot from browser.
@@ -48,7 +47,7 @@ def take_comparison_screenshot(screenshot_path)
       end
 
       def self.gen_next_attempt_path(screenshot_path, iteration)
-        screenshot_path.sub_ext(format(".attempt_%02i#{screenshot_path.extname}", iteration))
+        CapybaraScreenshotDiff::SnapManager.gen_next_attempt_path(screenshot_path, iteration)
       end
 
       PNG_EXTENSION = ".png"
@@ -133,7 +132,7 @@ def take_and_process_screenshot(new_screenshot_path, screenshot_path)
       end
 
       def move_screenshot_to(new_screenshot_path, screenshot_path)
-        FileUtils.mv(new_screenshot_path, screenshot_path, force: true)
+        CapybaraScreenshotDiff::SnapManager.move_screenshot_to(new_screenshot_path, screenshot_path)
       end
 
       def resize_if_needed(saved_image)
diff --git a/lib/capybara/screenshot/diff/vcs.rb b/lib/capybara/screenshot/diff/vcs.rb
index f8672724..846cea65 100644
--- a/lib/capybara/screenshot/diff/vcs.rb
+++ b/lib/capybara/screenshot/diff/vcs.rb
@@ -31,8 +31,8 @@ def self.restore_git_revision(screenshot_path, checkout_path)
           end
         end
 
-        def self.checkout_vcs(screenshot_path, checkout_path)
-          if svn?
+        def self.checkout_vcs(screenshot_path, checkout_path, root = Screenshot.screenshot_area_abs)
+          if svn?(root)
             restore_svn_revision(screenshot_path, checkout_path)
           else
             restore_git_revision(screenshot_path, checkout_path)
@@ -61,8 +61,8 @@ def self.restore_svn_revision(screenshot_path, checkout_path)
           false
         end
 
-        def self.svn?
-          (Screenshot.screenshot_area_abs / ".svn").exist?
+        def self.svn?(root)
+          (root / ".svn").exist?
         end
       end
     end
diff --git a/lib/capybara_screenshot_diff.rb b/lib/capybara_screenshot_diff.rb
index 5aa1915d..21443bdb 100644
--- a/lib/capybara_screenshot_diff.rb
+++ b/lib/capybara_screenshot_diff.rb
@@ -4,9 +4,9 @@
 require "capybara/screenshot/diff/version"
 require "capybara/screenshot/diff/utils"
 require "capybara/screenshot/diff/image_compare"
+require "capybara_screenshot_diff/snap_manager"
 require "capybara/screenshot/diff/test_methods"
 require "capybara/screenshot/diff/screenshoter"
-
 require "capybara/screenshot/diff/reporters/default"
 
 module CapybaraScreenshotDiff
@@ -63,6 +63,8 @@ module Diff
       mattr_accessor :tolerance
 
       mattr_accessor(:screenshoter) { Screenshoter }
+      mattr_accessor(:manager) { CapybaraScreenshotDiff::SnapManager }
+
 
       AVAILABLE_DRIVERS = Utils.detect_available_drivers.freeze
 
diff --git a/lib/capybara_screenshot_diff/dsl.rb b/lib/capybara_screenshot_diff/dsl.rb
index 8a58739f..188463d2 100644
--- a/lib/capybara_screenshot_diff/dsl.rb
+++ b/lib/capybara_screenshot_diff/dsl.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "capybara_screenshot_diff"
+require "capybara/screenshot/diff/test_methods"
 
 module CapybaraScreenshotDiff
   module DSL
diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
new file mode 100644
index 00000000..30485d64
--- /dev/null
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -0,0 +1,122 @@
+# frozen_string_literal: true
+
+require "capybara/screenshot/diff/vcs"
+require "active_support/core_ext/module/attribute_accessors"
+
+module CapybaraScreenshotDiff
+  class SnapManager
+
+    class Snap
+      attr_reader :screenshot_full_name, :screenshot_format, :screenshot_path, :base_screenshot_path
+
+      def initialize(screenshot_full_name, screenshot_format)
+        @screenshot_full_name = screenshot_full_name
+        @screenshot_format = screenshot_format
+        @screenshot_path = SnapManager.image_path_for(screenshot_full_name, screenshot_format)
+        @base_screenshot_path = SnapManager.base_image_path_from(@screenshot_path)
+      end
+
+      def delete!
+        @screenshot_path.delete if @screenshot_path.exist?
+        @base_screenshot_path.delete if @base_screenshot_path.exist?
+      end
+    end
+
+    attr_reader :root
+
+    def initialize(root)
+      @root = root
+    end
+
+    def snap_for(screenshot_full_name, screenshot_format)
+      Snap.new(screenshot_full_name, screenshot_format)
+    end
+
+    def image_path_for(screenshot_full_name, screenshot_format)
+      @root / Pathname.new(screenshot_full_name).sub_ext(".#{screenshot_format}")
+    end
+
+    def checkout_base_screenshot(screenshot_path)
+      create_output_directory_for(screenshot_path) unless screenshot_path.exist?
+
+      Capybara::Screenshot::Diff::Vcs.checkout_vcs(screenshot_path, base_image_path_from(screenshot_path), root)
+    end
+
+    def create_output_directory_for(screenshot_path)
+      screenshot_path.dirname.mkpath
+    end
+
+    def delete_output_directory_for
+
+    end
+
+    def base_image_path_from(screenshot_path)
+      screenshot_path.sub_ext(".base#{screenshot_path.extname}")
+    end
+
+    def cleanup_attempts_screenshots(base_file)
+      FileUtils.rm_rf attempts_screenshot_paths(base_file)
+    end
+
+    def attempts_screenshot_paths(base_file)
+      extname = Pathname.new(base_file).extname
+      Dir["#{base_file.to_s.chomp(extname)}.attempt_*#{extname}"].sort
+    end
+
+    def gen_next_attempt_path(screenshot_path, iteration)
+      screenshot_path.sub_ext(format(".attempt_%02i#{screenshot_path.extname}", iteration))
+    end
+
+    def move_screenshot_to(new_screenshot_path, screenshot_path)
+      FileUtils.mv(new_screenshot_path, screenshot_path, force: true)
+    end
+
+    def screenshots
+      root.children.map { |f| f.basename.to_s }
+    end
+
+    def self.screenshots
+      manager.screenshots
+    end
+
+    def self.snapshot(screenshot_full_name, screenshot_format = "png")
+      manager.snap_for(screenshot_full_name, screenshot_format)
+    end
+
+    def self.image_path_for(screenshot_full_name, screenshot_format)
+      manager.image_path_for(screenshot_full_name, screenshot_format)
+    end
+
+    def self.checkout_base_screenshot(screenshot_path)
+      manager.checkout_base_screenshot(screenshot_path)
+    end
+
+    def self.create_output_directory_for(screenshot_path)
+      manager.create_output_directory_for(screenshot_path)
+    end
+
+    def self.base_image_path_from(screenshot_path)
+      manager.base_image_path_from(screenshot_path)
+    end
+
+    def self.cleanup_attempts_screenshots(base_file)
+      manager.cleanup_attempts_screenshots(base_file)
+    end
+
+    def self.attempts_screenshot_paths(base_file)
+      manager.attempts_screenshot_paths(base_file)
+    end
+
+    def self.gen_next_attempt_path(screenshot_path, iteration)
+      manager.gen_next_attempt_path(screenshot_path, iteration)
+    end
+
+    def self.move_screenshot_to(new_screenshot_path, screenshot_path)
+      manager.move_screenshot_to(new_screenshot_path, screenshot_path)
+    end
+
+    def self.manager
+      Capybara::Screenshot::Diff.manager.new(Capybara::Screenshot.screenshot_area_abs)
+    end
+  end
+end
diff --git a/test/integration/browser_screenshot_test.rb b/test/integration/browser_screenshot_test.rb
index e0685e28..5db08c0d 100644
--- a/test/integration/browser_screenshot_test.rb
+++ b/test/integration/browser_screenshot_test.rb
@@ -187,7 +187,7 @@ def test_screenshot_selected_element
         end
       end
     ensure
-      FileUtils.rm_rf(Capybara::Screenshot.screenshot_area_abs / "index-with-anim.png")
+      CapybaraScreenshotDiff::SnapManager.snapshot("index-with-anim").delete!
     end
 
     def test_await_all_images_are_loaded
diff --git a/test/support/stub_test_methods.rb b/test/support/stub_test_methods.rb
index ffe065ea..a979c3c0 100644
--- a/test/support/stub_test_methods.rb
+++ b/test/support/stub_test_methods.rb
@@ -24,7 +24,7 @@ def make_comparison(fixture_base_image, fixture_new_image, destination: nil, **o
 
           set_test_images(destination, fixture_base_image, fixture_new_image)
 
-          ImageCompare.new(destination, ScreenshotMatcher.base_image_path_from(destination), **options)
+          ImageCompare.new(destination, CapybaraScreenshotDiff::SnapManager.base_image_path_from(destination), **options)
         end
 
         def set_test_images(destination, original_base_image, original_new_image, ext: "png")
@@ -36,7 +36,7 @@ def set_test_images(destination, original_base_image, original_new_image, ext: "
           FileUtils.cp(TEST_IMAGES_DIR / "#{original_new_image}.#{ext}", destination)
           FileUtils.cp(
             TEST_IMAGES_DIR / "#{original_base_image}.#{ext}",
-            ScreenshotMatcher.base_image_path_from(destination)
+            CapybaraScreenshotDiff::SnapManager.base_image_path_from(destination)
           )
         end
 
diff --git a/test/test_helper.rb b/test/test_helper.rb
index f61fdfee..95496abe 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -46,10 +46,8 @@ def assert_same_images(expected_image_name, image_path)
   end
 
   def assert_stored_screenshot(filename)
-    screenshots = Capybara::Screenshot.screenshot_area_abs.children.map { |f| f.basename.to_s }
-
     assert_includes(
-      screenshots,
+      CapybaraScreenshotDiff::SnapManager.screenshots,
       filename,
       "Screenshot #{filename} not found in #{Capybara::Screenshot.screenshot_area_abs}"
     )

From 4aa8247410e2dd095d14c0c2527a6e436b34563f Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 01:11:21 +0200
Subject: [PATCH 03/23] refact: cleanup

---
 .../screenshot/diff/screenshot_matcher.rb     |  2 +-
 lib/capybara/screenshot/diff/vcs.rb           |  2 +-
 lib/capybara_screenshot_diff/snap_manager.rb  | 59 +++++++++++--------
 .../screenshot/diff/test_methods_test.rb      |  8 ++-
 test/capybara/screenshot/diff_test.rb         |  2 +-
 test/capybara/screenshot/screenshot_test.rb   |  2 +-
 test/support/stub_test_methods.rb             |  2 +-
 test/test_helper.rb                           |  2 +-
 8 files changed, 45 insertions(+), 34 deletions(-)

diff --git a/lib/capybara/screenshot/diff/screenshot_matcher.rb b/lib/capybara/screenshot/diff/screenshot_matcher.rb
index 80dc5507..b56604b5 100644
--- a/lib/capybara/screenshot/diff/screenshot_matcher.rb
+++ b/lib/capybara/screenshot/diff/screenshot_matcher.rb
@@ -72,7 +72,7 @@ def build_screenshot_matches_job
         private
 
         def checkout_base_screenshot
-          CapybaraScreenshotDiff::SnapManager.checkout_base_screenshot(screenshot_path)
+          @snapshot.checkout_base_screenshot
         end
 
         # Try to get screenshot from browser.
diff --git a/lib/capybara/screenshot/diff/vcs.rb b/lib/capybara/screenshot/diff/vcs.rb
index 846cea65..e44df9ca 100644
--- a/lib/capybara/screenshot/diff/vcs.rb
+++ b/lib/capybara/screenshot/diff/vcs.rb
@@ -31,7 +31,7 @@ def self.restore_git_revision(screenshot_path, checkout_path)
           end
         end
 
-        def self.checkout_vcs(screenshot_path, checkout_path, root = Screenshot.screenshot_area_abs)
+        def self.checkout_vcs(root, screenshot_path, checkout_path)
           if svn?(root)
             restore_svn_revision(screenshot_path, checkout_path)
           else
diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index 30485d64..d40dee7c 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -7,19 +7,25 @@ module CapybaraScreenshotDiff
   class SnapManager
 
     class Snap
-      attr_reader :screenshot_full_name, :screenshot_format, :screenshot_path, :base_screenshot_path
+      attr_reader :screenshot_full_name, :screenshot_format, :screenshot_path, :base_screenshot_path, :manager
 
-      def initialize(screenshot_full_name, screenshot_format)
+      def initialize(screenshot_full_name, screenshot_format, manager = SnapManager.instance)
         @screenshot_full_name = screenshot_full_name
         @screenshot_format = screenshot_format
-        @screenshot_path = SnapManager.image_path_for(screenshot_full_name, screenshot_format)
-        @base_screenshot_path = SnapManager.base_image_path_from(@screenshot_path)
+        @screenshot_path = manager.abs_path_for(Pathname.new(@screenshot_full_name).sub_ext(".#{@screenshot_format}"))
+        @base_screenshot_path = manager.abs_path_for(@screenshot_path.sub_ext(".base#{screenshot_path.extname}"))
+        @manager = manager
       end
 
       def delete!
         @screenshot_path.delete if @screenshot_path.exist?
         @base_screenshot_path.delete if @base_screenshot_path.exist?
       end
+
+      def checkout_base_screenshot
+        @manager.checkout_file(screenshot_path, base_screenshot_path)
+      end
+
     end
 
     attr_reader :root
@@ -32,22 +38,25 @@ def snap_for(screenshot_full_name, screenshot_format)
       Snap.new(screenshot_full_name, screenshot_format)
     end
 
-    def image_path_for(screenshot_full_name, screenshot_format)
-      @root / Pathname.new(screenshot_full_name).sub_ext(".#{screenshot_format}")
+    def abs_path_for(path)
+      @root / path
     end
 
-    def checkout_base_screenshot(screenshot_path)
-      create_output_directory_for(screenshot_path) unless screenshot_path.exist?
-
-      Capybara::Screenshot::Diff::Vcs.checkout_vcs(screenshot_path, base_image_path_from(screenshot_path), root)
+    def checkout_file(path, as_path)
+      create_output_directory_for(path) unless path.exist?
+      Capybara::Screenshot::Diff::Vcs.checkout_vcs(root, path, as_path)
     end
 
     def create_output_directory_for(screenshot_path)
       screenshot_path.dirname.mkpath
     end
 
-    def delete_output_directory_for
+    def clean!
+      FileUtils.rm_rf root
+    end
 
+    def self.clean!
+      instance.clean!
     end
 
     def base_image_path_from(screenshot_path)
@@ -76,46 +85,46 @@ def screenshots
     end
 
     def self.screenshots
-      manager.screenshots
+      instance.screenshots
     end
 
     def self.snapshot(screenshot_full_name, screenshot_format = "png")
-      manager.snap_for(screenshot_full_name, screenshot_format)
+      instance.snap_for(screenshot_full_name, screenshot_format)
     end
 
     def self.image_path_for(screenshot_full_name, screenshot_format)
-      manager.image_path_for(screenshot_full_name, screenshot_format)
-    end
-
-    def self.checkout_base_screenshot(screenshot_path)
-      manager.checkout_base_screenshot(screenshot_path)
+      instance.abs_path_for(screenshot_full_name, screenshot_format)
     end
 
     def self.create_output_directory_for(screenshot_path)
-      manager.create_output_directory_for(screenshot_path)
+      instance.create_output_directory_for(screenshot_path)
     end
 
     def self.base_image_path_from(screenshot_path)
-      manager.base_image_path_from(screenshot_path)
+      instance.base_image_path_from(screenshot_path)
     end
 
     def self.cleanup_attempts_screenshots(base_file)
-      manager.cleanup_attempts_screenshots(base_file)
+      instance.cleanup_attempts_screenshots(base_file)
     end
 
     def self.attempts_screenshot_paths(base_file)
-      manager.attempts_screenshot_paths(base_file)
+      instance.attempts_screenshot_paths(base_file)
     end
 
     def self.gen_next_attempt_path(screenshot_path, iteration)
-      manager.gen_next_attempt_path(screenshot_path, iteration)
+      instance.gen_next_attempt_path(screenshot_path, iteration)
     end
 
     def self.move_screenshot_to(new_screenshot_path, screenshot_path)
-      manager.move_screenshot_to(new_screenshot_path, screenshot_path)
+      instance.move_screenshot_to(new_screenshot_path, screenshot_path)
+    end
+
+    def self.root
+      instance.root
     end
 
-    def self.manager
+    def self.instance
       Capybara::Screenshot::Diff.manager.new(Capybara::Screenshot.screenshot_area_abs)
     end
   end
diff --git a/test/capybara/screenshot/diff/test_methods_test.rb b/test/capybara/screenshot/diff/test_methods_test.rb
index 159d422b..633a0b73 100644
--- a/test/capybara/screenshot/diff/test_methods_test.rb
+++ b/test/capybara/screenshot/diff/test_methods_test.rb
@@ -83,18 +83,20 @@ def test_skip_area_and_stability_time_limit
 
         def test_creates_new_screenshot
           screenshot(:c)
-          assert_predicate (Capybara::Screenshot.screenshot_area_abs / "c.png"), :exist?
+
+          snap = CapybaraScreenshotDiff::SnapManager.snapshot("c")
+          assert_predicate snap.screenshot_path, :exist?
         end
 
         def test_cleanup_base_image_for_no_change
           comparison = make_comparison(:a, :a)
-          assert_image_not_changed("my_test.rb:42", "name", comparison)
+          assert_image_not_changed(["my_test.rb:42"], "name", comparison)
           assert_not comparison.base_image_path.exist?
         end
 
         def test_cleanup_base_image_for_changes
           comparison = make_comparison(:a, :b)
-          assert_image_not_changed("my_test.rb:42", "name", comparison)
+          assert_image_not_changed(["my_test.rb:42"], "name", comparison)
           assert_not comparison.base_image_path.exist?
         end
 
diff --git a/test/capybara/screenshot/diff_test.rb b/test/capybara/screenshot/diff_test.rb
index efb121e3..1fff9234 100644
--- a/test/capybara/screenshot/diff_test.rb
+++ b/test/capybara/screenshot/diff_test.rb
@@ -27,7 +27,7 @@ class DiffTest < ActionDispatch::IntegrationTest
       include Diff::TestMethodsStub
 
       teardown do
-        FileUtils.rm_rf Capybara::Screenshot.screenshot_area_abs
+        CapybaraScreenshotDiff::SnapManager.clean!
 
         Capybara::Screenshot.add_driver_path = @orig_add_driver_path
         Capybara::Screenshot.add_os_path = @orig_add_os_path
diff --git a/test/capybara/screenshot/screenshot_test.rb b/test/capybara/screenshot/screenshot_test.rb
index cc242037..4fb273db 100644
--- a/test/capybara/screenshot/screenshot_test.rb
+++ b/test/capybara/screenshot/screenshot_test.rb
@@ -6,7 +6,7 @@
 module Capybara
   class ScreenshotTest < ActionDispatch::IntegrationTest
     def test_screenshot_area_abs_is_absolute
-      assert Capybara::Screenshot.screenshot_area_abs.absolute?
+      assert CapybaraScreenshotDiff::SnapManager.root.absolute?
     end
 
     def test_root_is_a_pathname
diff --git a/test/support/stub_test_methods.rb b/test/support/stub_test_methods.rb
index a979c3c0..21f9ecb6 100644
--- a/test/support/stub_test_methods.rb
+++ b/test/support/stub_test_methods.rb
@@ -27,7 +27,7 @@ def make_comparison(fixture_base_image, fixture_new_image, destination: nil, **o
           ImageCompare.new(destination, CapybaraScreenshotDiff::SnapManager.base_image_path_from(destination), **options)
         end
 
-        def set_test_images(destination, original_base_image, original_new_image, ext: "png")
+        def set_test_images(destination, original_base_image, original_new_image)
           destination = Pathname.new(destination) unless destination.is_a?(Pathname)
           destination = Capybara::Screenshot.screenshot_area_abs.join(destination) unless destination.absolute?
           destination.dirname.mkpath unless destination.dirname.exist?
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 95496abe..1cb6d3bb 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -49,7 +49,7 @@ def assert_stored_screenshot(filename)
     assert_includes(
       CapybaraScreenshotDiff::SnapManager.screenshots,
       filename,
-      "Screenshot #{filename} not found in #{Capybara::Screenshot.screenshot_area_abs}"
+      "Screenshot #{filename} not found in #{CapybaraScreenshotDiff::SnapManager.instance.root}"
     )
   end
 end

From 7a41b376986092db72a967349e037591d6778c1c Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 02:15:39 +0200
Subject: [PATCH 04/23] refact: removes usage of the filesystem to access to
 the screenshots

---
 .../screenshot/diff/screenshot_matcher.rb     |  4 +-
 lib/capybara/screenshot/diff/test_methods.rb  |  6 +--
 lib/capybara_screenshot_diff/snap_manager.rb  | 40 ++++++-------------
 .../screenshot/diff/test_methods_test.rb      |  8 ++--
 test/capybara/screenshot/diff_test.rb         | 11 +++--
 test/support/stub_test_methods.rb             | 23 +++++------
 6 files changed, 38 insertions(+), 54 deletions(-)

diff --git a/lib/capybara/screenshot/diff/screenshot_matcher.rb b/lib/capybara/screenshot/diff/screenshot_matcher.rb
index b56604b5..976eaf1b 100644
--- a/lib/capybara/screenshot/diff/screenshot_matcher.rb
+++ b/lib/capybara/screenshot/diff/screenshot_matcher.rb
@@ -22,11 +22,11 @@ def initialize(screenshot_full_name, options = {})
         end
 
         def screenshot_path
-          @snapshot.screenshot_path
+          @snapshot.path
         end
 
         def base_screenshot_path
-          @snapshot.base_screenshot_path
+          @snapshot.base_path
         end
 
         def build_screenshot_matches_job
diff --git a/lib/capybara/screenshot/diff/test_methods.rb b/lib/capybara/screenshot/diff/test_methods.rb
index 4b162ff5..2e9d7db4 100644
--- a/lib/capybara/screenshot/diff/test_methods.rb
+++ b/lib/capybara/screenshot/diff/test_methods.rb
@@ -26,7 +26,7 @@ module Screenshot
     module Diff
       module TestMethods
         # @!attribute [rw] test_screenshots
-        #   @return [Array(Array(Array(String), String, ImageCompare))] An array where each element is an array containing the caller context,
+        #   @return [Array(Array(Array(String), String, ImageCompare | Minitest::Mock))] An array where each element is an array containing the caller context,
         #     the name of the screenshot, and the comparison object. This attribute stores information about each screenshot
         #     scheduled for comparison to ensure they do not show any unintended differences.
         def initialize(*)
@@ -146,7 +146,7 @@ def screenshot(name, skip_stack_frames: 0, **options)
 
         # Asserts that an image has not changed compared to its baseline.
         #
-        # @param caller [Array] The caller context, used for error reporting.
+        # @param caller [Array(String)] The caller context, used for error reporting.
         # @param name [String] The name of the screenshot being verified.
         # @param comparison [Object] The comparison object containing the result and details of the comparison.
         # @return [String, nil] Returns an error message if the screenshot differs from the baseline, otherwise nil.
@@ -163,7 +163,7 @@ def assert_image_not_changed(caller, name, comparison)
 
           return unless result
 
-          "Screenshot does not match for '#{name}' #{comparison.error_message}\n#{caller}"
+          "Screenshot does not match for '#{name}' #{comparison.error_message}\n#{caller.join(", ")}"
         end
 
         private
diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index d40dee7c..8d253812 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -7,23 +7,23 @@ module CapybaraScreenshotDiff
   class SnapManager
 
     class Snap
-      attr_reader :screenshot_full_name, :screenshot_format, :screenshot_path, :base_screenshot_path, :manager
+      attr_reader :screenshot_full_name, :screenshot_format, :path, :base_path, :manager
 
-      def initialize(screenshot_full_name, screenshot_format, manager = SnapManager.instance)
+      def initialize(screenshot_full_name, screenshot_format, manager: SnapManager.instance)
         @screenshot_full_name = screenshot_full_name
         @screenshot_format = screenshot_format
-        @screenshot_path = manager.abs_path_for(Pathname.new(@screenshot_full_name).sub_ext(".#{@screenshot_format}"))
-        @base_screenshot_path = manager.abs_path_for(@screenshot_path.sub_ext(".base#{screenshot_path.extname}"))
+        @path = manager.abs_path_for(Pathname.new(@screenshot_full_name).sub_ext(".#{@screenshot_format}"))
+        @base_path = @path.sub_ext(".base.#{@screenshot_format}")
         @manager = manager
       end
 
       def delete!
-        @screenshot_path.delete if @screenshot_path.exist?
-        @base_screenshot_path.delete if @base_screenshot_path.exist?
+        path.delete if path.exist?
+        base_path.delete if base_path.exist?
       end
 
       def checkout_base_screenshot
-        @manager.checkout_file(screenshot_path, base_screenshot_path)
+        @manager.checkout_file(path, base_path)
       end
 
     end
@@ -34,8 +34,8 @@ def initialize(root)
       @root = root
     end
 
-    def snap_for(screenshot_full_name, screenshot_format)
-      Snap.new(screenshot_full_name, screenshot_format)
+    def snap_for(screenshot_full_name, screenshot_format = "png")
+      Snap.new(screenshot_full_name, screenshot_format, manager: self)
     end
 
     def abs_path_for(path)
@@ -43,12 +43,12 @@ def abs_path_for(path)
     end
 
     def checkout_file(path, as_path)
-      create_output_directory_for(path) unless path.exist?
+      create_output_directory_for(as_path) unless as_path.exist?
       Capybara::Screenshot::Diff::Vcs.checkout_vcs(root, path, as_path)
     end
 
-    def create_output_directory_for(screenshot_path)
-      screenshot_path.dirname.mkpath
+    def create_output_directory_for(path)
+      path.dirname.mkpath
     end
 
     def clean!
@@ -59,10 +59,6 @@ def self.clean!
       instance.clean!
     end
 
-    def base_image_path_from(screenshot_path)
-      screenshot_path.sub_ext(".base#{screenshot_path.extname}")
-    end
-
     def cleanup_attempts_screenshots(base_file)
       FileUtils.rm_rf attempts_screenshot_paths(base_file)
     end
@@ -92,18 +88,6 @@ def self.snapshot(screenshot_full_name, screenshot_format = "png")
       instance.snap_for(screenshot_full_name, screenshot_format)
     end
 
-    def self.image_path_for(screenshot_full_name, screenshot_format)
-      instance.abs_path_for(screenshot_full_name, screenshot_format)
-    end
-
-    def self.create_output_directory_for(screenshot_path)
-      instance.create_output_directory_for(screenshot_path)
-    end
-
-    def self.base_image_path_from(screenshot_path)
-      instance.base_image_path_from(screenshot_path)
-    end
-
     def self.cleanup_attempts_screenshots(base_file)
       instance.cleanup_attempts_screenshots(base_file)
     end
diff --git a/test/capybara/screenshot/diff/test_methods_test.rb b/test/capybara/screenshot/diff/test_methods_test.rb
index 633a0b73..ffbee4e3 100644
--- a/test/capybara/screenshot/diff/test_methods_test.rb
+++ b/test/capybara/screenshot/diff/test_methods_test.rb
@@ -20,7 +20,7 @@ class TestMethodsTest < ActionDispatch::IntegrationTest
         end
 
         def test_assert_image_not_changed
-          message = assert_image_not_changed("my_test.rb:42", "name", make_comparison(:a, :c))
+          message = assert_image_not_changed(["my_test.rb:42"], "name", make_comparison(:a, :c))
           value = (RUBY_VERSION >= "2.4") ? 187.4 : 188
           assert_equal <<-MSG.strip_heredoc.chomp, message
             Screenshot does not match for 'name' ({"area_size":629,"region":[11,3,48,20],"max_color_distance":#{value}})
@@ -33,7 +33,7 @@ def test_assert_image_not_changed
 
         def test_assert_image_not_changed_with_shift_distance_limit
           message = assert_image_not_changed(
-            "my_test.rb:42",
+            ["my_test.rb:42"],
             "name",
             make_comparison(:a, :c, shift_distance_limit: 1, driver: :chunky_png)
           )
@@ -55,7 +55,7 @@ def test_screenshot_support_drivers_options
         def test_skip_stack_frames
           Vcs.stub(:checkout_vcs, true) do
             assert_predicate @test_screenshots, :blank?
-            make_comparison(:a, :c, destination: Rails.root / "doc/screenshots/a.png")
+            make_comparison(:a, :c, destination: "a.png")
 
             our_screenshot("a", 1)
             assert_equal 1, @test_screenshots.size
@@ -85,7 +85,7 @@ def test_creates_new_screenshot
           screenshot(:c)
 
           snap = CapybaraScreenshotDiff::SnapManager.snapshot("c")
-          assert_predicate snap.screenshot_path, :exist?
+          assert_predicate snap.path, :exist?
         end
 
         def test_cleanup_base_image_for_no_change
diff --git a/test/capybara/screenshot/diff_test.rb b/test/capybara/screenshot/diff_test.rb
index 1fff9234..678274f1 100644
--- a/test/capybara/screenshot/diff_test.rb
+++ b/test/capybara/screenshot/diff_test.rb
@@ -148,7 +148,7 @@ def _test_sample_screenshot_error
           mock.expect(:error_message, "expected error message")
 
           @test_screenshots = []
-          @test_screenshots << ["my_test.rb:42", "sample_screenshot", mock]
+          @test_screenshots << [["my_test.rb:42"], "sample_screenshot", mock]
           mock.expect(:clear_screenshots, @test_screenshots)
         end
       end
@@ -176,7 +176,7 @@ def _test_sample_screenshot_error
           comparison.expect(:base_image_path, Pathname.new("screenshot.base.png"))
           comparison.expect(:error_message, "expected error message for non minitest")
 
-          @test_screenshots << ["my_test.rb:42", "sample_screenshot", comparison]
+          @test_screenshots << [["my_test.rb:42"], "sample_screenshot", comparison]
         end
       end
 
@@ -195,7 +195,9 @@ class ScreenshotFormatTest < ActionDispatch::IntegrationTest
 
         test "use default screenshot format" do
           skip "VIPS not present. Skipping VIPS driver tests." unless defined?(Vips)
-          set_test_images("a.webp", :a, :a)
+          snap = CapybaraScreenshotDiff::SnapManager.snapshot("a", "webp")
+
+          set_test_images(snap, :a, :a)
 
           Capybara::Screenshot.stub(:screenshot_format, "webp") do
             screenshot "a", driver: :vips
@@ -205,7 +207,8 @@ class ScreenshotFormatTest < ActionDispatch::IntegrationTest
         end
 
         test "override default screenshot format" do
-          set_test_images("a.png", :a, :a)
+          snap = CapybaraScreenshotDiff::SnapManager.snapshot("a", "png")
+          set_test_images(snap, :a, :a)
 
           Capybara::Screenshot.stub(:screenshot_format, "webp") do
             screenshot "a", screenshot_format: "png"
diff --git a/test/support/stub_test_methods.rb b/test/support/stub_test_methods.rb
index 21f9ecb6..0acac8e5 100644
--- a/test/support/stub_test_methods.rb
+++ b/test/support/stub_test_methods.rb
@@ -10,6 +10,7 @@ module TestMethodsStub
 
         included do
           setup do
+            @manager = CapybaraScreenshotDiff::SnapManager.new(Rails.root / "doc/screenshots")
             Diff.screenshoter = ScreenshoterStub
           end
 
@@ -19,25 +20,21 @@ module TestMethodsStub
         end
 
         # Prepare comparison images and build ImageCompare for them
-        def make_comparison(fixture_base_image, fixture_new_image, destination: nil, **options)
-          destination ||= Rails.root / "doc/screenshots/screenshot.png"
+        def make_comparison(fixture_base_image, fixture_new_image, destination: "screenshot", **options)
+          snap = @manager.snap_for(destination)
 
-          set_test_images(destination, fixture_base_image, fixture_new_image)
+          set_test_images(snap, fixture_base_image, fixture_new_image)
 
-          ImageCompare.new(destination, CapybaraScreenshotDiff::SnapManager.base_image_path_from(destination), **options)
+          ImageCompare.new(snap.path, snap.base_path, **options)
         end
 
-        def set_test_images(destination, original_base_image, original_new_image)
-          destination = Pathname.new(destination) unless destination.is_a?(Pathname)
-          destination = Capybara::Screenshot.screenshot_area_abs.join(destination) unless destination.absolute?
-          destination.dirname.mkpath unless destination.dirname.exist?
+        def set_test_images(snap, original_base_image, original_new_image, ext: "png")
+          destination = snap.path
+          snap.manager.create_output_directory_for(destination)
 
           ext = destination.extname[1..] if destination.extname.present?
-          FileUtils.cp(TEST_IMAGES_DIR / "#{original_new_image}.#{ext}", destination)
-          FileUtils.cp(
-            TEST_IMAGES_DIR / "#{original_base_image}.#{ext}",
-            CapybaraScreenshotDiff::SnapManager.base_image_path_from(destination)
-          )
+          FileUtils.cp(TEST_IMAGES_DIR / "#{original_new_image}.#{ext}", snap.path)
+          FileUtils.cp(TEST_IMAGES_DIR / "#{original_base_image}.#{ext}", snap.base_path)
         end
 
         ImageCompareStub = Struct.new(

From a7ae9f9600e85ca49f05024544f8800fe54d2ec0 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 11:07:04 +0200
Subject: [PATCH 05/23] chore: cleanup lint

---
 lib/capybara_screenshot_diff.rb              | 1 -
 lib/capybara_screenshot_diff/snap_manager.rb | 2 --
 2 files changed, 3 deletions(-)

diff --git a/lib/capybara_screenshot_diff.rb b/lib/capybara_screenshot_diff.rb
index 21443bdb..3e3bd604 100644
--- a/lib/capybara_screenshot_diff.rb
+++ b/lib/capybara_screenshot_diff.rb
@@ -65,7 +65,6 @@ module Diff
       mattr_accessor(:screenshoter) { Screenshoter }
       mattr_accessor(:manager) { CapybaraScreenshotDiff::SnapManager }
 
-
       AVAILABLE_DRIVERS = Utils.detect_available_drivers.freeze
 
       def self.default_options
diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index 8d253812..add61e76 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -5,7 +5,6 @@
 
 module CapybaraScreenshotDiff
   class SnapManager
-
     class Snap
       attr_reader :screenshot_full_name, :screenshot_format, :path, :base_path, :manager
 
@@ -25,7 +24,6 @@ def delete!
       def checkout_base_screenshot
         @manager.checkout_file(path, base_path)
       end
-
     end
 
     attr_reader :root

From f236c7617dfab537865f742862e724e9ca769d77 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 12:24:25 +0200
Subject: [PATCH 06/23] wip

---
 lib/capybara_screenshot_diff/snap_manager.rb  | 37 ++++++++++++++-----
 .../screenshot/diff/image_compare_test.rb     | 30 +++++++--------
 test/capybara/screenshot/diff/vcs_test.rb     | 10 ++---
 .../snap_manager_test.rb                      | 35 ++++++++++++++++++
 test/support/stub_test_methods.rb             |  9 ++---
 test/test_helper.rb                           |  4 ++
 6 files changed, 87 insertions(+), 38 deletions(-)
 create mode 100644 test/capybara_screenshot_diff/snap_manager_test.rb

diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index add61e76..9ac6f393 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -6,13 +6,13 @@
 module CapybaraScreenshotDiff
   class SnapManager
     class Snap
-      attr_reader :screenshot_full_name, :screenshot_format, :path, :base_path, :manager
+      attr_reader :full_name, :format, :path, :base_path, :manager
 
-      def initialize(screenshot_full_name, screenshot_format, manager: SnapManager.instance)
-        @screenshot_full_name = screenshot_full_name
-        @screenshot_format = screenshot_format
-        @path = manager.abs_path_for(Pathname.new(@screenshot_full_name).sub_ext(".#{@screenshot_format}"))
-        @base_path = @path.sub_ext(".base.#{@screenshot_format}")
+      def initialize(full_name, format, manager: SnapManager.instance)
+        @full_name = full_name
+        @format = format
+        @path = manager.abs_path_for(Pathname.new(@full_name).sub_ext(".#{@format}"))
+        @base_path = @path.sub_ext(".base.#{@format}")
         @manager = manager
       end
 
@@ -24,20 +24,29 @@ def delete!
       def checkout_base_screenshot
         @manager.checkout_file(path, base_path)
       end
+
+      def path_for(version = :actual)
+        case version
+        when :base
+          base_path
+        else
+          path
+        end
+      end
     end
 
     attr_reader :root
 
     def initialize(root)
-      @root = root
+      @root = Pathname.new(root)
     end
 
     def snap_for(screenshot_full_name, screenshot_format = "png")
       Snap.new(screenshot_full_name, screenshot_format, manager: self)
     end
 
-    def abs_path_for(path)
-      @root / path
+    def abs_path_for(relative_path)
+      @root / relative_path
     end
 
     def checkout_file(path, as_path)
@@ -45,6 +54,16 @@ def checkout_file(path, as_path)
       Capybara::Screenshot::Diff::Vcs.checkout_vcs(root, path, as_path)
     end
 
+    def provision_snap_with(snap, path, version: :actual)
+      managed_path = snap.path_for(version)
+      create_output_directory_for(managed_path) unless managed_path.exist?
+      FileUtils.cp(path, managed_path)
+    end
+
+    def self.provision_snap_with(snap, path, version: :actual)
+      instance.provision_snap_with(snap, path, version: version)
+    end
+
     def create_output_directory_for(path)
       path.dirname.mkpath
     end
diff --git a/test/capybara/screenshot/diff/image_compare_test.rb b/test/capybara/screenshot/diff/image_compare_test.rb
index 0aad8542..732e141d 100644
--- a/test/capybara/screenshot/diff/image_compare_test.rb
+++ b/test/capybara/screenshot/diff/image_compare_test.rb
@@ -108,31 +108,29 @@ class IntegrationRegressionTest < ActionDispatch::IntegrationTest
         end
 
         test "the different images should not be quick equal and different" do
+          skip
           images = all_fixtures_images_names
 
           AVAILABLE_DRIVERS.each do |driver|
-            Dir.chdir File.expand_path("../../../images", __dir__) do
-              images.each do |image|
-                other_images = images - [image]
-                other_images.each do |different_image|
-                  comparison = make_comparison(image, different_image, **driver)
-                  assert_not(
-                    comparison.quick_equal?,
-                    "compare #{image} with #{different_image} with #{driver} driver should not be quick_equal"
-                  )
-                  assert(
-                    comparison.different?,
-                    "compare #{image} with #{different_image} with #{driver} driver should be different"
-                  )
-                end
+            images.each do |image|
+              other_images = images - [image]
+              other_images.each do |different_image|
+                comparison = make_comparison(image, different_image, **driver)
+                assert_not(
+                  comparison.quick_equal?,
+                  "compare #{image.inspect} with #{different_image.inspect} using #{driver} driver should not be quick_equal"
+                )
+                assert(
+                  comparison.different?,
+                  "compare #{image.inspect} with #{different_image.inspect} using #{driver} driver should be different"
+                )
               end
             end
           end
         end
 
         def all_fixtures_images_names
-          fixtures_images = Dir[File.expand_path("../../../images/*.png", __dir__)]
-          fixtures_images.map { |f| File.basename(f).chomp(".png") }
+          %w[a a_cropped b c d portrait portrait_b]
         end
       end
     end
diff --git a/test/capybara/screenshot/diff/vcs_test.rb b/test/capybara/screenshot/diff/vcs_test.rb
index 3982c786..2b8a3e3a 100644
--- a/test/capybara/screenshot/diff/vcs_test.rb
+++ b/test/capybara/screenshot/diff/vcs_test.rb
@@ -20,17 +20,13 @@ class VcsTest < ActionDispatch::IntegrationTest
         end
 
         test "checkout of original screenshot" do
-          prev_root = Capybara::Screenshot.root
-          Capybara::Screenshot.root = "."
-
-          screenshot_path = Capybara::Screenshot.root / "test/images/a.png"
+          skip
+          screenshot_path = Pathname.new("test/images/a.png")
           base_screenshot_path = Pathname.new(@base_screenshot.path)
-          assert Vcs.restore_git_revision(screenshot_path, base_screenshot_path)
+          assert Vcs.restore_git_revision(screenshot_path, base_screenshot_path, root: Pathname.new("."))
 
           assert base_screenshot_path.exist?
           assert_equal screenshot_path.size, base_screenshot_path.size
-        ensure
-          Capybara::Screenshot.root = prev_root
         end
       end
     end
diff --git a/test/capybara_screenshot_diff/snap_manager_test.rb b/test/capybara_screenshot_diff/snap_manager_test.rb
new file mode 100644
index 00000000..43a6a2e1
--- /dev/null
+++ b/test/capybara_screenshot_diff/snap_manager_test.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+module CapybaraScreenshotDiff
+  class SnapManagerTest < ActiveSupport::TestCase
+    setup do
+      @manager = SnapManager.new(Dir.mktmpdir("snap_diff-storage"))
+    end
+
+    teardown do
+      @manager.clean!
+    end
+
+    test "#provision_snap_with copies the file to the snap path" do
+      snap = @manager.snap_for("test_image")
+      path = fixture_image_path_from("a")
+
+      @manager.provision_snap_with(snap, path)
+
+      assert_predicate snap.path, :exist?
+      assert_not_predicate snap.base_path, :exist?
+    end
+
+    test "#provision_snap_with populate the base version of the snapshot" do
+      snap = @manager.snap_for("test_image")
+      path = fixture_image_path_from("a")
+
+      @manager.provision_snap_with(snap, path, version: :base)
+
+      assert_not_predicate snap.path, :exist?
+      assert_predicate snap.base_path, :exist?
+    end
+  end
+end
diff --git a/test/support/stub_test_methods.rb b/test/support/stub_test_methods.rb
index 0acac8e5..3de89fb3 100644
--- a/test/support/stub_test_methods.rb
+++ b/test/support/stub_test_methods.rb
@@ -15,6 +15,7 @@ module TestMethodsStub
           end
 
           teardown do
+            @manager.clean!
             Diff.screenshoter = Screenshoter
           end
         end
@@ -29,12 +30,8 @@ def make_comparison(fixture_base_image, fixture_new_image, destination: "screens
         end
 
         def set_test_images(snap, original_base_image, original_new_image, ext: "png")
-          destination = snap.path
-          snap.manager.create_output_directory_for(destination)
-
-          ext = destination.extname[1..] if destination.extname.present?
-          FileUtils.cp(TEST_IMAGES_DIR / "#{original_new_image}.#{ext}", snap.path)
-          FileUtils.cp(TEST_IMAGES_DIR / "#{original_base_image}.#{ext}", snap.base_path)
+          @manager.provision_snap_with(snap, fixture_image_path_from(original_new_image, snap.format), version: :actual)
+          @manager.provision_snap_with(snap, fixture_image_path_from(original_base_image, snap.format), version: :base)
         end
 
         ImageCompareStub = Struct.new(
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 1cb6d3bb..f463fd1b 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -40,6 +40,10 @@ def optional_test
     end
   end
 
+  def fixture_image_path_from(original_new_image, ext = "png")
+    TEST_IMAGES_DIR / "#{original_new_image}.#{ext}"
+  end
+
   def assert_same_images(expected_image_name, image_path)
     expected_image_path = file_fixture("files/comparisons/#{expected_image_name}")
     assert_predicate(Capybara::Screenshot::Diff::ImageCompare.new(image_path, expected_image_path), :quick_equal?)

From 6ad67453b111c7e82e47fe02f4da3bc5616e43bc Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 13:35:31 +0200
Subject: [PATCH 07/23] cleanup vcs

---
 lib/capybara/screenshot/diff/vcs.rb   | 24 +++++++++++++-----------
 test/capybara/screenshot/diff_test.rb |  3 ++-
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/lib/capybara/screenshot/diff/vcs.rb b/lib/capybara/screenshot/diff/vcs.rb
index e44df9ca..0e5d63e6 100644
--- a/lib/capybara/screenshot/diff/vcs.rb
+++ b/lib/capybara/screenshot/diff/vcs.rb
@@ -6,6 +6,19 @@ module Capybara
   module Screenshot
     module Diff
       module Vcs
+
+        def self.checkout_vcs(root, screenshot_path, checkout_path)
+          if svn?(root)
+            restore_svn_revision(screenshot_path, checkout_path)
+          else
+            restore_git_revision(screenshot_path, checkout_path)
+          end
+        end
+
+        def self.svn?(root)
+          (root / ".svn").exist?
+        end
+
         SILENCE_ERRORS = Os::ON_WINDOWS ? "2>nul" : "2>/dev/null"
 
         def self.restore_git_revision(screenshot_path, checkout_path)
@@ -31,14 +44,6 @@ def self.restore_git_revision(screenshot_path, checkout_path)
           end
         end
 
-        def self.checkout_vcs(root, screenshot_path, checkout_path)
-          if svn?(root)
-            restore_svn_revision(screenshot_path, checkout_path)
-          else
-            restore_git_revision(screenshot_path, checkout_path)
-          end
-        end
-
         def self.restore_svn_revision(screenshot_path, checkout_path)
           committed_file_name = screenshot_path + "../.svn/text-base/" + "#{screenshot_path.basename}.svn-base"
           if committed_file_name.exist?
@@ -61,9 +66,6 @@ def self.restore_svn_revision(screenshot_path, checkout_path)
           false
         end
 
-        def self.svn?(root)
-          (root / ".svn").exist?
-        end
       end
     end
   end
diff --git a/test/capybara/screenshot/diff_test.rb b/test/capybara/screenshot/diff_test.rb
index 678274f1..dc8b5ce8 100644
--- a/test/capybara/screenshot/diff_test.rb
+++ b/test/capybara/screenshot/diff_test.rb
@@ -27,7 +27,8 @@ class DiffTest < ActionDispatch::IntegrationTest
       include Diff::TestMethodsStub
 
       teardown do
-        CapybaraScreenshotDiff::SnapManager.clean!
+        # TODO: need to find a way to clean up the snapshots
+        # CapybaraScreenshotDiff::SnapManager.clean!
 
         Capybara::Screenshot.add_driver_path = @orig_add_driver_path
         Capybara::Screenshot.add_os_path = @orig_add_os_path

From 1a122dc71fbc17c26413cda282e9ef3f179edcad Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 14:11:33 +0200
Subject: [PATCH 08/23] chore: cleanup vcs

---
 lib/capybara/screenshot/diff/vcs.rb       | 24 ++++++++++++-----------
 test/capybara/screenshot/diff/vcs_test.rb |  7 +++----
 test/system_test_case.rb                  |  2 +-
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/lib/capybara/screenshot/diff/vcs.rb b/lib/capybara/screenshot/diff/vcs.rb
index 0e5d63e6..25be4791 100644
--- a/lib/capybara/screenshot/diff/vcs.rb
+++ b/lib/capybara/screenshot/diff/vcs.rb
@@ -11,7 +11,7 @@ def self.checkout_vcs(root, screenshot_path, checkout_path)
           if svn?(root)
             restore_svn_revision(screenshot_path, checkout_path)
           else
-            restore_git_revision(screenshot_path, checkout_path)
+            restore_git_revision(screenshot_path, checkout_path, root: root)
           end
         end
 
@@ -21,19 +21,21 @@ def self.svn?(root)
 
         SILENCE_ERRORS = Os::ON_WINDOWS ? "2>nul" : "2>/dev/null"
 
-        def self.restore_git_revision(screenshot_path, checkout_path)
-          vcs_file_path = screenshot_path.relative_path_from(Screenshot.root)
-
+        def self.restore_git_revision(screenshot_path, checkout_path = screenshot_path, root:)
+          vcs_file_path = screenshot_path.relative_path_from(root)
           redirect_target = "#{checkout_path} #{SILENCE_ERRORS}"
           show_command = "git show HEAD~0:./#{vcs_file_path}"
-          if Screenshot.use_lfs
-            `#{show_command} > #{checkout_path}.tmp #{SILENCE_ERRORS}`
-            if $CHILD_STATUS == 0
-              `git lfs smudge < #{checkout_path}.tmp > #{redirect_target}`
+
+          Dir.chdir(root) do
+            if Screenshot.use_lfs
+              system("#{show_command} > #{checkout_path}.tmp #{SILENCE_ERRORS}", exception: !!ENV["DEBUG"])
+
+              `git lfs smudge < #{checkout_path}.tmp > #{redirect_target}` if $CHILD_STATUS == 0
+
+              File.delete "#{checkout_path}.tmp"
+            else
+              system("#{show_command} > #{redirect_target}", exception: !!ENV["DEBUG"])
             end
-            File.delete "#{checkout_path}.tmp"
-          else
-            `#{show_command} > #{redirect_target}`
           end
 
           if $CHILD_STATUS != 0
diff --git a/test/capybara/screenshot/diff/vcs_test.rb b/test/capybara/screenshot/diff/vcs_test.rb
index 2b8a3e3a..0752b419 100644
--- a/test/capybara/screenshot/diff/vcs_test.rb
+++ b/test/capybara/screenshot/diff/vcs_test.rb
@@ -9,7 +9,7 @@ class VcsTest < ActionDispatch::IntegrationTest
         include Vcs
 
         setup do
-          @base_screenshot = Tempfile.new(%w[vcs_base_screenshot attempt.0.png], Rails.root)
+          @base_screenshot = Tempfile.new(%w[vcs_base_screenshot. .attempt.0.png], Screenshot.root)
         end
 
         teardown do
@@ -20,10 +20,9 @@ class VcsTest < ActionDispatch::IntegrationTest
         end
 
         test "checkout of original screenshot" do
-          skip
-          screenshot_path = Pathname.new("test/images/a.png")
+          screenshot_path = Screenshot.root / "../test/images/a.png"
           base_screenshot_path = Pathname.new(@base_screenshot.path)
-          assert Vcs.restore_git_revision(screenshot_path, base_screenshot_path, root: Pathname.new("."))
+          assert Vcs.restore_git_revision(screenshot_path, base_screenshot_path, root: Screenshot.root)
 
           assert base_screenshot_path.exist?
           assert_equal screenshot_path.size, base_screenshot_path.size
diff --git a/test/system_test_case.rb b/test/system_test_case.rb
index 0526fbe5..5fffa4c0 100644
--- a/test/system_test_case.rb
+++ b/test/system_test_case.rb
@@ -61,7 +61,7 @@ def rollback_comparison_runtime_files((_, _, comparison))
     save_annotations_for_debug(comparison)
 
     screenshot_path = comparison.image_path
-    Vcs.restore_git_revision(screenshot_path, screenshot_path)
+    Vcs.restore_git_revision(screenshot_path, root: Capybara::Screenshot.root)
 
     if comparison.difference
       comparison.reporter.clean_tmp_files

From 866c1b77f1f4f750480d2795314f4ea6350ef401 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 15:57:56 +0200
Subject: [PATCH 09/23] chore: cleanup vcs

---
 test/capybara/screenshot/diff/image_compare_test.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/capybara/screenshot/diff/image_compare_test.rb b/test/capybara/screenshot/diff/image_compare_test.rb
index 732e141d..eac0d597 100644
--- a/test/capybara/screenshot/diff/image_compare_test.rb
+++ b/test/capybara/screenshot/diff/image_compare_test.rb
@@ -108,7 +108,6 @@ class IntegrationRegressionTest < ActionDispatch::IntegrationTest
         end
 
         test "the different images should not be quick equal and different" do
-          skip
           images = all_fixtures_images_names
 
           AVAILABLE_DRIVERS.each do |driver|

From c096ac48422cc510b0299b8cfca8a4655f848754 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 16:28:13 +0200
Subject: [PATCH 10/23] refact: use snapshots for screenshoter

---
 .../screenshot/diff/screenshot_matcher.rb     |  6 ++--
 lib/capybara/screenshot/diff/screenshoter.rb  | 35 ++++---------------
 .../screenshot/diff/stable_screenshoter.rb    | 14 ++++----
 lib/capybara_screenshot_diff/snap_manager.rb  | 18 ++++++++++
 4 files changed, 35 insertions(+), 38 deletions(-)

diff --git a/lib/capybara/screenshot/diff/screenshot_matcher.rb b/lib/capybara/screenshot/diff/screenshot_matcher.rb
index 976eaf1b..cdb3bfd7 100644
--- a/lib/capybara/screenshot/diff/screenshot_matcher.rb
+++ b/lib/capybara/screenshot/diff/screenshot_matcher.rb
@@ -60,7 +60,7 @@ def build_screenshot_matches_job
 
           # Load new screenshot from Browser
           comparison_options = driver_options
-          take_comparison_screenshot(capture_options, comparison_options, screenshot_path)
+          take_comparison_screenshot(capture_options, comparison_options, screenshot_path, @snapshot)
 
           # Pre-computation: No need to compare without base screenshot
           return unless base_screenshot_path.exist?
@@ -78,9 +78,9 @@ def checkout_base_screenshot
         # Try to get screenshot from browser.
         # On `stability_time_limit` it checks that page stop updating by comparison several screenshot attempts
         # On reaching `wait` limit then it has been failed. On failing we annotate screenshot attempts to help to debug
-        def take_comparison_screenshot(capture_options, comparison_options, screenshot_path)
+        def take_comparison_screenshot(capture_options, comparison_options, screenshot_path, snapshot = nil)
           screenshoter = build_screenshoter_for(capture_options, comparison_options)
-          screenshoter.take_comparison_screenshot(screenshot_path)
+          screenshoter.take_comparison_screenshot(screenshot_path, snapshot)
         end
 
         def build_screenshoter_for(capture_options, comparison_options = {})
diff --git a/lib/capybara/screenshot/diff/screenshoter.rb b/lib/capybara/screenshot/diff/screenshoter.rb
index ccf0812e..a0318d77 100644
--- a/lib/capybara/screenshot/diff/screenshoter.rb
+++ b/lib/capybara/screenshot/diff/screenshoter.rb
@@ -29,25 +29,12 @@ def capybara_screenshot_options
         @capture_options[:capybara_screenshot_options] || {}
       end
 
-      def self.attempts_screenshot_paths(base_file)
-        CapybaraScreenshotDiff::SnapManager.attempts_screenshot_paths(base_file)
-      end
-
-      def self.cleanup_attempts_screenshots(base_file)
-        CapybaraScreenshotDiff::SnapManager.cleanup_attempts_screenshots(base_file)
-      end
-
       # Try to get screenshot from browser.
       # On `stability_time_limit` it checks that page stop updating by comparison several screenshot attempts
       # On reaching `wait` limit then it has been failed. On failing we annotate screenshot attempts to help to debug
-      def take_comparison_screenshot(screenshot_path)
-        capture_screenshot_at(screenshot_path)
-
-        Screenshoter.cleanup_attempts_screenshots(screenshot_path)
-      end
-
-      def self.gen_next_attempt_path(screenshot_path, iteration)
-        CapybaraScreenshotDiff::SnapManager.gen_next_attempt_path(screenshot_path, iteration)
+      def take_comparison_screenshot(_, snapshot)
+        capture_screenshot_at(snapshot)
+        snapshot.cleanup_attempts
       end
 
       PNG_EXTENSION = ".png"
@@ -99,6 +86,7 @@ def wait_images_loaded(timeout:)
 
         deadline_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) + timeout
         loop do
+          Capybara.default_max_wait_time
           pending_image = BrowserHelpers.pending_image_to_load
           break unless pending_image
 
@@ -121,18 +109,9 @@ def save_and_process_screenshot(screenshot_path)
         File.unlink(tmpfile) if tmpfile
       end
 
-      def capture_screenshot_at(screenshot_path)
-        new_screenshot_path = Screenshoter.gen_next_attempt_path(screenshot_path, 0)
-        take_and_process_screenshot(new_screenshot_path, screenshot_path)
-      end
-
-      def take_and_process_screenshot(new_screenshot_path, screenshot_path)
-        take_screenshot(new_screenshot_path)
-        move_screenshot_to(new_screenshot_path, screenshot_path)
-      end
-
-      def move_screenshot_to(new_screenshot_path, screenshot_path)
-        CapybaraScreenshotDiff::SnapManager.move_screenshot_to(new_screenshot_path, screenshot_path)
+      def capture_screenshot_at(snapshot)
+        take_screenshot(snapshot.next_attempt_path)
+        snapshot.commit_last_attempt
       end
 
       def resize_if_needed(saved_image)
diff --git a/lib/capybara/screenshot/diff/stable_screenshoter.rb b/lib/capybara/screenshot/diff/stable_screenshoter.rb
index b8c53a54..024db9e7 100644
--- a/lib/capybara/screenshot/diff/stable_screenshoter.rb
+++ b/lib/capybara/screenshot/diff/stable_screenshoter.rb
@@ -38,8 +38,8 @@ def initialize(capture_options, comparison_options = nil)
         # @param screenshot_path [String, Pathname] The path where the screenshot will be saved.
         # @return [void]
         # @raise [RuntimeError] If a stable screenshot cannot be obtained within the specified `:wait` time.
-        def take_comparison_screenshot(screenshot_path)
-          new_screenshot_path = take_stable_screenshot(screenshot_path)
+        def take_comparison_screenshot(screenshot_path, snapshot = nil)
+          new_screenshot_path = take_stable_screenshot(screenshot_path, snapshot)
 
           # We failed to get stable browser state! Generate difference between attempts to overview moving parts!
           unless new_screenshot_path
@@ -48,17 +48,17 @@ def take_comparison_screenshot(screenshot_path)
           end
 
           FileUtils.mv(new_screenshot_path, screenshot_path, force: true)
-          Screenshoter.cleanup_attempts_screenshots(screenshot_path)
+          CapybaraScreenshotDiff::SnapManager.cleanup_attempts_screenshots(screenshot_path)
         end
 
-        def take_stable_screenshot(screenshot_path)
+        def take_stable_screenshot(screenshot_path, snapshot = nil)
           screenshot_path = screenshot_path.is_a?(String) ? Pathname.new(screenshot_path) : screenshot_path
           # We try to compare first attempt with checkout version, in order to not run next screenshots
           attempt_path = nil
           deadline_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) + wait
 
           # Cleanup all previous attempts for sure
-          Screenshoter.cleanup_attempts_screenshots(screenshot_path)
+          CapybaraScreenshotDiff::SnapManager.cleanup_attempts_screenshots(screenshot_path)
 
           0.step do |i|
             # FIXME: it should be wait, and wait should be replaced with stability_time_limit
@@ -79,7 +79,7 @@ def attempt_successful?(attempt_path, prev_attempt_path)
         end
 
         def attempt_next_screenshot(prev_attempt_path, i, screenshot_path)
-          new_attempt_path = Screenshoter.gen_next_attempt_path(screenshot_path, i)
+          new_attempt_path = CapybaraScreenshotDiff::SnapManager.gen_next_attempt_path(screenshot_path, i)
           @screenshoter.take_screenshot(new_attempt_path)
           [new_attempt_path, prev_attempt_path]
         end
@@ -94,7 +94,7 @@ def build_comparison_for(attempt_path, previous_attempt_path)
 
         # TODO: Move to the HistoricalReporter
         def annotate_attempts_and_fail!(screenshot_path)
-          screenshot_attempts = Screenshoter.attempts_screenshot_paths(screenshot_path)
+          screenshot_attempts = CapybaraScreenshotDiff::SnapManager.attempts_screenshot_paths(screenshot_path)
 
           annotate_stabilization_images(screenshot_attempts)
 
diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index 9ac6f393..875b46f1 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -14,6 +14,7 @@ def initialize(full_name, format, manager: SnapManager.instance)
         @path = manager.abs_path_for(Pathname.new(@full_name).sub_ext(".#{@format}"))
         @base_path = @path.sub_ext(".base.#{@format}")
         @manager = manager
+        @attempts_count = 0
       end
 
       def delete!
@@ -33,6 +34,23 @@ def path_for(version = :actual)
           path
         end
       end
+
+      def attach_attempt(a_path)
+        @manager.move_screenshot_to(a_path, next_attempt_path)
+        @attempts_count += 1
+      end
+
+      def next_attempt_path
+        @manager.gen_next_attempt_path(path, @attempts_count)
+      end
+
+      def commit_last_attempt
+        @manager.move_screenshot_to(next_attempt_path, path)
+      end
+
+      def cleanup_attempts
+        @manager.cleanup_attempts_screenshots(path)
+      end
     end
 
     attr_reader :root

From 605bbffe0da21ba3c74a396a58fe36a7583c317b Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 17:01:13 +0200
Subject: [PATCH 11/23] test: cleanup tests to use snap manager

---
 lib/capybara_screenshot_diff/snap_manager.rb  |  9 ++++-
 .../diff/stable_screenshoter_test.rb          | 39 +++++++++++++------
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index 875b46f1..39900eb9 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -20,6 +20,7 @@ def initialize(full_name, format, manager: SnapManager.instance)
       def delete!
         path.delete if path.exist?
         base_path.delete if base_path.exist?
+        cleanup_attempts
       end
 
       def checkout_base_screenshot
@@ -51,6 +52,10 @@ def commit_last_attempt
       def cleanup_attempts
         @manager.cleanup_attempts_screenshots(path)
       end
+
+      def attempts_paths
+        Dir[@manager.abs_path_for "**/#{full_name}.attempt_*.#{format}"]
+      end
     end
 
     attr_reader :root
@@ -82,8 +87,8 @@ def self.provision_snap_with(snap, path, version: :actual)
       instance.provision_snap_with(snap, path, version: version)
     end
 
-    def create_output_directory_for(path)
-      path.dirname.mkpath
+    def create_output_directory_for(path = nil)
+      path ? path.dirname.mkpath : root.mkpath
     end
 
     def clean!
diff --git a/test/capybara/screenshot/diff/stable_screenshoter_test.rb b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
index bd17e740..408180b0 100644
--- a/test/capybara/screenshot/diff/stable_screenshoter_test.rb
+++ b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
@@ -8,6 +8,15 @@ module Diff
       class StableScreenshoterTest < ActionDispatch::IntegrationTest
         include TestMethodsStub
 
+        setup do
+          @manager = CapybaraScreenshotDiff::SnapManager.new(Capybara::Screenshot.root / "stable_screenshoter_test")
+          @manager.create_output_directory_for
+        end
+
+        teardown do
+          @manager.delete!
+        end
+
         test "#take_stable_screenshot several iterations to take stable screenshot" do
           image_compare_stub = build_image_compare_stub
 
@@ -18,7 +27,8 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
           mock.expect(:quick_equal?, true)
 
           ImageCompare.stub :new, mock do
-            take_stable_screenshot_with("tmp/02_a.png")
+            snap = @manager.snap_for("02_a")
+            take_stable_screenshot_with(snap.path)
           end
 
           mock.verify
@@ -43,26 +53,30 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
           mock.expect(:quick_equal?, false)
           mock.expect(:quick_equal?, true)
 
-          assert_not (Capybara::Screenshot.root / "02_a.png").exist?
+          snap = @manager.snap_for("02_a")
+          assert_not_predicate snap.path, :exist?
 
           ImageCompare.stub :new, mock do
             StableScreenshoter
               .new({stability_time_limit: 0.5, wait: 1}, image_compare_stub.driver_options)
-              .take_comparison_screenshot("tmp/02_a.png")
+              .take_comparison_screenshot(snap.path, snap)
           end
 
           mock.verify
-          assert_empty Dir[Capybara::Screenshot.root / "**/02_a.attempt_*.png"]
-          assert (Capybara::Screenshot.root / "02_a.png").exist?
-          assert_not_predicate (Capybara::Screenshot.root / "02_a.png").size, :zero?
+          assert_empty snap.attempts_paths
+          assert_predicate snap.path, :exist?
+          assert_not_predicate snap.path.size, :zero?
         end
 
         test "#take_comparison_screenshot fail on missing find stable image in time and generates annotated history screenshots" do
-          screenshot_path = Pathname.new("tmp/01_a.png")
+          snap = @manager.snap_for("01_a")
+
+          screenshot_path = snap.path
 
           # Stub annotated files for generated comparison annotations
           # We need to have different from screenshot_path name because of other stubs
-          annotated_screenshot_path = Pathname.new("tmp/02_a.png")
+          pseudo_snap_for_annotations = @manager.snap_for("02_a")
+          annotated_screenshot_path = pseudo_snap_for_annotations.path
           annotated_attempts_paths = [
             [annotated_screenshot_path.sub_ext(".attempt_01.latest.png"), annotated_screenshot_path.sub_ext(".attempt_01.committed.png")],
             [annotated_screenshot_path.sub_ext(".attempt_02.latest.png"), annotated_screenshot_path.sub_ext(".attempt_02.committed.png")]
@@ -71,9 +85,9 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
           FileUtils.touch(annotated_attempts_paths)
 
           mock = ::Minitest::Mock.new(build_image_compare_stub(equal: false))
-          annotated_attempts_paths.reverse_each do |(latest_path, committed_path)|
-            mock.reporter.expect(:annotated_image_path, latest_path.to_s)
-            mock.reporter.expect(:annotated_base_image_path, committed_path.to_s)
+          annotated_attempts_paths.reverse_each do |(actual_path, base_path)|
+            mock.reporter.expect(:annotated_image_path, actual_path.to_s)
+            mock.reporter.expect(:annotated_base_image_path, base_path.to_s)
           end
 
           assert_raises RuntimeError, "Could not get stable screenshot within 1s" do
@@ -98,7 +112,8 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
           last_annotation = screenshot_path.sub_ext(".attempt_01.png")
           assert_equal 0, last_annotation.size, "#{last_annotation.to_path} should be override with annotated version"
         ensure
-          FileUtils.rm_rf Dir["tmp/01_a*.png"]
+          snap&.delete!
+          pseudo_snap_for_annotations&.delete!
         end
       end
     end

From 496a25c3b454628672dedaca812088247d22ed97 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 17:01:34 +0200
Subject: [PATCH 12/23] chore: fixes lint

---
 lib/capybara/screenshot/diff/vcs.rb | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/capybara/screenshot/diff/vcs.rb b/lib/capybara/screenshot/diff/vcs.rb
index 25be4791..532f0890 100644
--- a/lib/capybara/screenshot/diff/vcs.rb
+++ b/lib/capybara/screenshot/diff/vcs.rb
@@ -6,7 +6,6 @@ module Capybara
   module Screenshot
     module Diff
       module Vcs
-
         def self.checkout_vcs(root, screenshot_path, checkout_path)
           if svn?(root)
             restore_svn_revision(screenshot_path, checkout_path)
@@ -67,7 +66,6 @@ def self.restore_svn_revision(screenshot_path, checkout_path)
 
           false
         end
-
       end
     end
   end

From 648b3bc69e46b27cd8922bdbd81dd8392c5e5c2b Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 17:03:31 +0200
Subject: [PATCH 13/23] test: cleanup tests to use snap manager

---
 lib/capybara_screenshot_diff/snap_manager.rb              | 1 +
 test/capybara/screenshot/diff/stable_screenshoter_test.rb | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index 39900eb9..d5da190f 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -91,6 +91,7 @@ def create_output_directory_for(path = nil)
       path ? path.dirname.mkpath : root.mkpath
     end
 
+    # TODO: rename to delete!
     def clean!
       FileUtils.rm_rf root
     end
diff --git a/test/capybara/screenshot/diff/stable_screenshoter_test.rb b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
index 408180b0..6065b476 100644
--- a/test/capybara/screenshot/diff/stable_screenshoter_test.rb
+++ b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
@@ -14,7 +14,7 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
         end
 
         teardown do
-          @manager.delete!
+          @manager.clean!
         end
 
         test "#take_stable_screenshot several iterations to take stable screenshot" do
@@ -95,7 +95,7 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
               # Wait time is less then stability time, which will generate problem
               StableScreenshoter
                 .new({stability_time_limit: 0.5, wait: 1}, build_image_compare_stub(equal: false).driver_options)
-                .take_comparison_screenshot(screenshot_path.to_s)
+                .take_comparison_screenshot(screenshot_path.to_s, snap)
             end
           end
 

From 98e9cb426230357a50d097e9c1df2bdd4af01ab2 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 17:06:08 +0200
Subject: [PATCH 14/23] refact: removes scrn_path

---
 lib/capybara/screenshot/diff/screenshot_matcher.rb     |  2 +-
 lib/capybara/screenshot/diff/screenshoter.rb           |  2 +-
 lib/capybara/screenshot/diff/stable_screenshoter.rb    | 10 +++++-----
 .../screenshot/diff/stable_screenshoter_test.rb        |  4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/capybara/screenshot/diff/screenshot_matcher.rb b/lib/capybara/screenshot/diff/screenshot_matcher.rb
index cdb3bfd7..786e2ea9 100644
--- a/lib/capybara/screenshot/diff/screenshot_matcher.rb
+++ b/lib/capybara/screenshot/diff/screenshot_matcher.rb
@@ -80,7 +80,7 @@ def checkout_base_screenshot
         # On reaching `wait` limit then it has been failed. On failing we annotate screenshot attempts to help to debug
         def take_comparison_screenshot(capture_options, comparison_options, screenshot_path, snapshot = nil)
           screenshoter = build_screenshoter_for(capture_options, comparison_options)
-          screenshoter.take_comparison_screenshot(screenshot_path, snapshot)
+          screenshoter.take_comparison_screenshot(snapshot)
         end
 
         def build_screenshoter_for(capture_options, comparison_options = {})
diff --git a/lib/capybara/screenshot/diff/screenshoter.rb b/lib/capybara/screenshot/diff/screenshoter.rb
index a0318d77..015fcb9f 100644
--- a/lib/capybara/screenshot/diff/screenshoter.rb
+++ b/lib/capybara/screenshot/diff/screenshoter.rb
@@ -32,7 +32,7 @@ def capybara_screenshot_options
       # Try to get screenshot from browser.
       # On `stability_time_limit` it checks that page stop updating by comparison several screenshot attempts
       # On reaching `wait` limit then it has been failed. On failing we annotate screenshot attempts to help to debug
-      def take_comparison_screenshot(_, snapshot)
+      def take_comparison_screenshot(snapshot)
         capture_screenshot_at(snapshot)
         snapshot.cleanup_attempts
       end
diff --git a/lib/capybara/screenshot/diff/stable_screenshoter.rb b/lib/capybara/screenshot/diff/stable_screenshoter.rb
index 024db9e7..9a96b436 100644
--- a/lib/capybara/screenshot/diff/stable_screenshoter.rb
+++ b/lib/capybara/screenshot/diff/stable_screenshoter.rb
@@ -38,17 +38,17 @@ def initialize(capture_options, comparison_options = nil)
         # @param screenshot_path [String, Pathname] The path where the screenshot will be saved.
         # @return [void]
         # @raise [RuntimeError] If a stable screenshot cannot be obtained within the specified `:wait` time.
-        def take_comparison_screenshot(screenshot_path, snapshot = nil)
-          new_screenshot_path = take_stable_screenshot(screenshot_path, snapshot)
+        def take_comparison_screenshot(snapshot)
+          new_screenshot_path = take_stable_screenshot(snapshot.path, snapshot)
 
           # We failed to get stable browser state! Generate difference between attempts to overview moving parts!
           unless new_screenshot_path
             # FIXME(uwe): Change to store the failure and only report if the test succeeds functionally.
-            annotate_attempts_and_fail!(screenshot_path)
+            annotate_attempts_and_fail!(snapshot.path)
           end
 
-          FileUtils.mv(new_screenshot_path, screenshot_path, force: true)
-          CapybaraScreenshotDiff::SnapManager.cleanup_attempts_screenshots(screenshot_path)
+          FileUtils.mv(new_screenshot_path, snapshot.path, force: true)
+          snapshot.cleanup_attempts
         end
 
         def take_stable_screenshot(screenshot_path, snapshot = nil)
diff --git a/test/capybara/screenshot/diff/stable_screenshoter_test.rb b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
index 6065b476..9c31027a 100644
--- a/test/capybara/screenshot/diff/stable_screenshoter_test.rb
+++ b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
@@ -59,7 +59,7 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
           ImageCompare.stub :new, mock do
             StableScreenshoter
               .new({stability_time_limit: 0.5, wait: 1}, image_compare_stub.driver_options)
-              .take_comparison_screenshot(snap.path, snap)
+              .take_comparison_screenshot(snap)
           end
 
           mock.verify
@@ -95,7 +95,7 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
               # Wait time is less then stability time, which will generate problem
               StableScreenshoter
                 .new({stability_time_limit: 0.5, wait: 1}, build_image_compare_stub(equal: false).driver_options)
-                .take_comparison_screenshot(screenshot_path.to_s, snap)
+                .take_comparison_screenshot(snap)
             end
           end
 

From 469b2e040b23d189f08138fa965384301fd93c69 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 18:06:32 +0200
Subject: [PATCH 15/23] refact: adds usage of snapshot for take screenshot
 methods

---
 lib/capybara/screenshot/diff/screenshoter.rb  |  2 +-
 .../screenshot/diff/stable_screenshoter.rb    | 40 ++++++++-------
 lib/capybara_screenshot_diff/snap_manager.rb  | 51 +++++++++----------
 .../diff/stable_screenshoter_test.rb          |  6 +--
 test/support/stub_test_methods.rb             |  4 +-
 5 files changed, 52 insertions(+), 51 deletions(-)

diff --git a/lib/capybara/screenshot/diff/screenshoter.rb b/lib/capybara/screenshot/diff/screenshoter.rb
index 015fcb9f..26740811 100644
--- a/lib/capybara/screenshot/diff/screenshoter.rb
+++ b/lib/capybara/screenshot/diff/screenshoter.rb
@@ -110,7 +110,7 @@ def save_and_process_screenshot(screenshot_path)
       end
 
       def capture_screenshot_at(snapshot)
-        take_screenshot(snapshot.next_attempt_path)
+        take_screenshot(snapshot.attempt_path)
         snapshot.commit_last_attempt
       end
 
diff --git a/lib/capybara/screenshot/diff/stable_screenshoter.rb b/lib/capybara/screenshot/diff/stable_screenshoter.rb
index 9a96b436..b07365ab 100644
--- a/lib/capybara/screenshot/diff/stable_screenshoter.rb
+++ b/lib/capybara/screenshot/diff/stable_screenshoter.rb
@@ -35,66 +35,70 @@ def initialize(capture_options, comparison_options = nil)
         # or the `:wait` limit is reached. If unable to achieve a stable state within the time limit, it annotates the attempts
         # to aid debugging.
         #
-        # @param screenshot_path [String, Pathname] The path where the screenshot will be saved.
+        # @param snapshot Snap The snapshot details to take a stable screenshot of.
         # @return [void]
         # @raise [RuntimeError] If a stable screenshot cannot be obtained within the specified `:wait` time.
         def take_comparison_screenshot(snapshot)
-          new_screenshot_path = take_stable_screenshot(snapshot.path, snapshot)
+          new_screenshot_path = take_stable_screenshot(snapshot)
 
           # We failed to get stable browser state! Generate difference between attempts to overview moving parts!
           unless new_screenshot_path
             # FIXME(uwe): Change to store the failure and only report if the test succeeds functionally.
-            annotate_attempts_and_fail!(snapshot.path)
+            annotate_attempts_and_fail!(snapshot)
           end
 
-          FileUtils.mv(new_screenshot_path, snapshot.path, force: true)
+          snapshot.attach(new_screenshot_path, version: :actual)
           snapshot.cleanup_attempts
         end
 
-        def take_stable_screenshot(screenshot_path, snapshot = nil)
-          screenshot_path = screenshot_path.is_a?(String) ? Pathname.new(screenshot_path) : screenshot_path
+        def take_stable_screenshot(snapshot)
           # We try to compare first attempt with checkout version, in order to not run next screenshots
-          attempt_path = nil
           deadline_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) + wait
 
           # Cleanup all previous attempts for sure
-          CapybaraScreenshotDiff::SnapManager.cleanup_attempts_screenshots(screenshot_path)
+          snapshot.cleanup_attempts
 
           0.step do |i|
             # FIXME: it should be wait, and wait should be replaced with stability_time_limit
             sleep(stability_time_limit) unless i == 0
-            attempt_path, prev_attempt_path = attempt_next_screenshot(attempt_path, i, screenshot_path)
-            return attempt_path if attempt_successful?(attempt_path, prev_attempt_path)
+            attempt_next_screenshot(snapshot)
+            return snapshot.attempt_path if attempt_successful?(snapshot)
             return nil if timeout?(deadline_at)
           end
         end
 
         private
 
-        def attempt_successful?(attempt_path, prev_attempt_path)
-          return false unless prev_attempt_path
-          build_comparison_for(attempt_path, prev_attempt_path).quick_equal?
+        def attempt_successful?(snapshot)
+          return false unless snapshot.prev_attempt_path
+
+          build_attempts_comparison_for(snapshot).quick_equal?
         rescue ArgumentError
           false
         end
 
-        def attempt_next_screenshot(prev_attempt_path, i, screenshot_path)
-          new_attempt_path = CapybaraScreenshotDiff::SnapManager.gen_next_attempt_path(screenshot_path, i)
+        def attempt_next_screenshot(snapshot)
+          new_attempt_path = snapshot.next_attempt_path!
+
           @screenshoter.take_screenshot(new_attempt_path)
-          [new_attempt_path, prev_attempt_path]
+          new_attempt_path
         end
 
         def timeout?(deadline_at)
           Process.clock_gettime(Process::CLOCK_MONOTONIC) > deadline_at
         end
 
+        def build_attempts_comparison_for(snapshot)
+          build_comparison_for(snapshot.attempt_path, snapshot.prev_attempt_path)
+        end
+
         def build_comparison_for(attempt_path, previous_attempt_path)
           ImageCompare.new(attempt_path, previous_attempt_path, @comparison_options)
         end
 
         # TODO: Move to the HistoricalReporter
-        def annotate_attempts_and_fail!(screenshot_path)
-          screenshot_attempts = CapybaraScreenshotDiff::SnapManager.attempts_screenshot_paths(screenshot_path)
+        def annotate_attempts_and_fail!(snapshot)
+          screenshot_attempts = CapybaraScreenshotDiff::SnapManager.attempts_screenshot_paths(snapshot)
 
           annotate_stabilization_images(screenshot_attempts)
 
diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index d5da190f..f71788a4 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -6,7 +6,7 @@
 module CapybaraScreenshotDiff
   class SnapManager
     class Snap
-      attr_reader :full_name, :format, :path, :base_path, :manager
+      attr_reader :full_name, :format, :path, :base_path, :manager, :prev_attempt_path, :attempts_count
 
       def initialize(full_name, format, manager: SnapManager.instance)
         @full_name = full_name
@@ -37,20 +37,34 @@ def path_for(version = :actual)
       end
 
       def attach_attempt(a_path)
-        @manager.move_screenshot_to(a_path, next_attempt_path)
+        @manager.move_screenshot_to(a_path, attempt_path)
         @attempts_count += 1
       end
 
-      def next_attempt_path
-        @manager.gen_next_attempt_path(path, @attempts_count)
+      def attach(a_path, version: :actual)
+        @manager.move_screenshot_to(a_path, path_for(version))
+      end
+
+      def attempt_path
+        @_attempt_path ||= path.sub_ext(sprintf(".attempt_%02i.#{format}", @attempts_count))
+      end
+
+      def next_attempt_path!
+        @prev_attempt_path = @_attempt_path
+        @_attempt_path = nil
+
+        attempt_path
+      ensure
+        @attempts_count += 1
       end
 
       def commit_last_attempt
-        @manager.move_screenshot_to(next_attempt_path, path)
+        @manager.move_screenshot_to(attempt_path, path)
       end
 
       def cleanup_attempts
-        @manager.cleanup_attempts_screenshots(path)
+        @manager.cleanup_attempts_screenshots(self)
+        @attempts_count = 0
       end
 
       def attempts_paths
@@ -100,17 +114,8 @@ def self.clean!
       instance.clean!
     end
 
-    def cleanup_attempts_screenshots(base_file)
-      FileUtils.rm_rf attempts_screenshot_paths(base_file)
-    end
-
-    def attempts_screenshot_paths(base_file)
-      extname = Pathname.new(base_file).extname
-      Dir["#{base_file.to_s.chomp(extname)}.attempt_*#{extname}"].sort
-    end
-
-    def gen_next_attempt_path(screenshot_path, iteration)
-      screenshot_path.sub_ext(format(".attempt_%02i#{screenshot_path.extname}", iteration))
+    def cleanup_attempts_screenshots(snapshot)
+      FileUtils.rm_rf snapshot.attempts_paths
     end
 
     def move_screenshot_to(new_screenshot_path, screenshot_path)
@@ -129,16 +134,8 @@ def self.snapshot(screenshot_full_name, screenshot_format = "png")
       instance.snap_for(screenshot_full_name, screenshot_format)
     end
 
-    def self.cleanup_attempts_screenshots(base_file)
-      instance.cleanup_attempts_screenshots(base_file)
-    end
-
-    def self.attempts_screenshot_paths(base_file)
-      instance.attempts_screenshot_paths(base_file)
-    end
-
-    def self.gen_next_attempt_path(screenshot_path, iteration)
-      instance.gen_next_attempt_path(screenshot_path, iteration)
+    def self.attempts_screenshot_paths(snapshot)
+      snapshot.attempts_paths
     end
 
     def self.move_screenshot_to(new_screenshot_path, screenshot_path)
diff --git a/test/capybara/screenshot/diff/stable_screenshoter_test.rb b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
index 9c31027a..10742691 100644
--- a/test/capybara/screenshot/diff/stable_screenshoter_test.rb
+++ b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
@@ -28,7 +28,7 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
 
           ImageCompare.stub :new, mock do
             snap = @manager.snap_for("02_a")
-            take_stable_screenshot_with(snap.path)
+            take_stable_screenshot_with(snap)
           end
 
           mock.verify
@@ -36,13 +36,13 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
 
         test "#take_stable_screenshot without wait raises any error" do
           assert_raises ArgumentError, "wait should be provided" do
-            take_stable_screenshot_with("tmp/02_a.png", wait: nil)
+            take_stable_screenshot_with(@manager.snap_for("02_a"), wait: nil)
           end
         end
 
         test "#take_stable_screenshot without stability_time_limit raises any error" do
           assert_raises ArgumentError, "stability_time_limit should be provided" do
-            take_stable_screenshot_with("tmp/02_a.png", stability_time_limit: nil)
+            take_stable_screenshot_with(@manager.snap_for("02_a"), stability_time_limit: nil)
           end
         end
 
diff --git a/test/support/stub_test_methods.rb b/test/support/stub_test_methods.rb
index 3de89fb3..20906c96 100644
--- a/test/support/stub_test_methods.rb
+++ b/test/support/stub_test_methods.rb
@@ -49,9 +49,9 @@ def build_image_compare_stub(equal: true)
           )
         end
 
-        def take_stable_screenshot_with(screenshot_path, stability_time_limit: 0.01, wait: 10)
+        def take_stable_screenshot_with(snap, stability_time_limit: 0.01, wait: 10)
           screenshoter = StableScreenshoter.new({stability_time_limit: stability_time_limit, wait: wait})
-          screenshoter.take_stable_screenshot(screenshot_path)
+          screenshoter.take_stable_screenshot(snap)
         end
       end
     end

From 717e7dc08b1d364c2450a60ff4cc607b310d0097 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 18:34:59 +0200
Subject: [PATCH 16/23] refcat: snap tracks attempts version

---
 .../screenshot/diff/stable_screenshoter.rb    | 23 ++++++++-----------
 lib/capybara_screenshot_diff/snap_manager.rb  |  8 ++-----
 .../diff/stable_screenshoter_test.rb          |  2 +-
 3 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/lib/capybara/screenshot/diff/stable_screenshoter.rb b/lib/capybara/screenshot/diff/stable_screenshoter.rb
index b07365ab..b7194a8b 100644
--- a/lib/capybara/screenshot/diff/stable_screenshoter.rb
+++ b/lib/capybara/screenshot/diff/stable_screenshoter.rb
@@ -60,7 +60,7 @@ def take_stable_screenshot(snapshot)
 
           0.step do |i|
             # FIXME: it should be wait, and wait should be replaced with stability_time_limit
-            sleep(stability_time_limit) unless i == 0
+            sleep(stability_time_limit) unless i == 0 # test prev_attempt_path is nil
             attempt_next_screenshot(snapshot)
             return snapshot.attempt_path if attempt_successful?(snapshot)
             return nil if timeout?(deadline_at)
@@ -72,33 +72,26 @@ def take_stable_screenshot(snapshot)
         def attempt_successful?(snapshot)
           return false unless snapshot.prev_attempt_path
 
-          build_attempts_comparison_for(snapshot).quick_equal?
+          build_last_attempts_comparison_for(snapshot).quick_equal?
         rescue ArgumentError
           false
         end
 
         def attempt_next_screenshot(snapshot)
-          new_attempt_path = snapshot.next_attempt_path!
-
-          @screenshoter.take_screenshot(new_attempt_path)
-          new_attempt_path
+          @screenshoter.take_screenshot(snapshot.next_attempt_path!)
         end
 
         def timeout?(deadline_at)
           Process.clock_gettime(Process::CLOCK_MONOTONIC) > deadline_at
         end
 
-        def build_attempts_comparison_for(snapshot)
-          build_comparison_for(snapshot.attempt_path, snapshot.prev_attempt_path)
-        end
-
-        def build_comparison_for(attempt_path, previous_attempt_path)
-          ImageCompare.new(attempt_path, previous_attempt_path, @comparison_options)
+        def build_last_attempts_comparison_for(snapshot)
+          ImageCompare.new(snapshot.attempt_path, snapshot.prev_attempt_path, @comparison_options)
         end
 
         # TODO: Move to the HistoricalReporter
         def annotate_attempts_and_fail!(snapshot)
-          screenshot_attempts = CapybaraScreenshotDiff::SnapManager.attempts_screenshot_paths(snapshot)
+          screenshot_attempts = snapshot.find_attempts_paths
 
           annotate_stabilization_images(screenshot_attempts)
 
@@ -126,6 +119,10 @@ def annotate_stabilization_images(attempts_screenshot_paths)
             previous_file = file_name
           end
         end
+
+        def build_comparison_for(attempt_path, previous_attempt_path)
+          ImageCompare.new(attempt_path, previous_attempt_path, @comparison_options)
+        end
       end
     end
   end
diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index f71788a4..6c8c3c88 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -67,7 +67,7 @@ def cleanup_attempts
         @attempts_count = 0
       end
 
-      def attempts_paths
+      def find_attempts_paths
         Dir[@manager.abs_path_for "**/#{full_name}.attempt_*.#{format}"]
       end
     end
@@ -115,7 +115,7 @@ def self.clean!
     end
 
     def cleanup_attempts_screenshots(snapshot)
-      FileUtils.rm_rf snapshot.attempts_paths
+      FileUtils.rm_rf snapshot.find_attempts_paths
     end
 
     def move_screenshot_to(new_screenshot_path, screenshot_path)
@@ -134,10 +134,6 @@ def self.snapshot(screenshot_full_name, screenshot_format = "png")
       instance.snap_for(screenshot_full_name, screenshot_format)
     end
 
-    def self.attempts_screenshot_paths(snapshot)
-      snapshot.attempts_paths
-    end
-
     def self.move_screenshot_to(new_screenshot_path, screenshot_path)
       instance.move_screenshot_to(new_screenshot_path, screenshot_path)
     end
diff --git a/test/capybara/screenshot/diff/stable_screenshoter_test.rb b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
index 10742691..717c5fd0 100644
--- a/test/capybara/screenshot/diff/stable_screenshoter_test.rb
+++ b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
@@ -63,7 +63,7 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
           end
 
           mock.verify
-          assert_empty snap.attempts_paths
+          assert_empty snap.find_attempts_paths
           assert_predicate snap.path, :exist?
           assert_not_predicate snap.path.size, :zero?
         end

From efc1ef2d09ab75bdb21bb6d706d73d010401c3c8 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 18:43:05 +0200
Subject: [PATCH 17/23] cleanup

---
 .../screenshot/diff/stable_screenshoter.rb        | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/capybara/screenshot/diff/stable_screenshoter.rb b/lib/capybara/screenshot/diff/stable_screenshoter.rb
index b7194a8b..4f7acdf8 100644
--- a/lib/capybara/screenshot/diff/stable_screenshoter.rb
+++ b/lib/capybara/screenshot/diff/stable_screenshoter.rb
@@ -39,15 +39,18 @@ def initialize(capture_options, comparison_options = nil)
         # @return [void]
         # @raise [RuntimeError] If a stable screenshot cannot be obtained within the specified `:wait` time.
         def take_comparison_screenshot(snapshot)
-          new_screenshot_path = take_stable_screenshot(snapshot)
+          result = take_stable_screenshot(snapshot)
 
           # We failed to get stable browser state! Generate difference between attempts to overview moving parts!
-          unless new_screenshot_path
+          unless result
             # FIXME(uwe): Change to store the failure and only report if the test succeeds functionally.
             annotate_attempts_and_fail!(snapshot)
           end
 
-          snapshot.attach(new_screenshot_path, version: :actual)
+          # store success attempt as actual screenshot
+          snapshot.commit_last_attempt
+
+          # cleanup all previous attempts
           snapshot.cleanup_attempts
         end
 
@@ -61,9 +64,11 @@ def take_stable_screenshot(snapshot)
           0.step do |i|
             # FIXME: it should be wait, and wait should be replaced with stability_time_limit
             sleep(stability_time_limit) unless i == 0 # test prev_attempt_path is nil
+
             attempt_next_screenshot(snapshot)
-            return snapshot.attempt_path if attempt_successful?(snapshot)
-            return nil if timeout?(deadline_at)
+
+            return true if attempt_successful?(snapshot)
+            return false if timeout?(deadline_at)
           end
         end
 

From 260877ede2b75563049e7c2036c0e9055f89714d Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 18:59:19 +0200
Subject: [PATCH 18/23] chore: cleanup

---
 lib/capybara/screenshot/diff/screenshoter.rb  |  3 +-
 lib/capybara_screenshot_diff/snap_manager.rb  | 57 +++++--------------
 .../diff/stable_screenshoter_test.rb          | 14 ++---
 .../snap_manager_test.rb                      |  6 +-
 test/support/stub_test_methods.rb             |  4 +-
 5 files changed, 29 insertions(+), 55 deletions(-)

diff --git a/lib/capybara/screenshot/diff/screenshoter.rb b/lib/capybara/screenshot/diff/screenshoter.rb
index 26740811..2c6dceb3 100644
--- a/lib/capybara/screenshot/diff/screenshoter.rb
+++ b/lib/capybara/screenshot/diff/screenshoter.rb
@@ -110,7 +110,8 @@ def save_and_process_screenshot(screenshot_path)
       end
 
       def capture_screenshot_at(snapshot)
-        take_screenshot(snapshot.attempt_path)
+        take_screenshot(snapshot.next_attempt_path!)
+
         snapshot.commit_last_attempt
       end
 
diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index 6c8c3c88..db6a638b 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -6,7 +6,7 @@
 module CapybaraScreenshotDiff
   class SnapManager
     class Snap
-      attr_reader :full_name, :format, :path, :base_path, :manager, :prev_attempt_path, :attempts_count
+      attr_reader :full_name, :format, :path, :base_path, :manager, :attempt_path, :prev_attempt_path, :attempts_count
 
       def initialize(full_name, format, manager: SnapManager.instance)
         @full_name = full_name
@@ -36,34 +36,19 @@ def path_for(version = :actual)
         end
       end
 
-      def attach_attempt(a_path)
-        @manager.move_screenshot_to(a_path, attempt_path)
-        @attempts_count += 1
-      end
-
-      def attach(a_path, version: :actual)
-        @manager.move_screenshot_to(a_path, path_for(version))
-      end
-
-      def attempt_path
-        @_attempt_path ||= path.sub_ext(sprintf(".attempt_%02i.#{format}", @attempts_count))
-      end
-
       def next_attempt_path!
-        @prev_attempt_path = @_attempt_path
-        @_attempt_path = nil
-
-        attempt_path
+        @prev_attempt_path = @attempt_path
+        @attempt_path = path.sub_ext(sprintf(".attempt_%02i.#{format}", @attempts_count))
       ensure
         @attempts_count += 1
       end
 
       def commit_last_attempt
-        @manager.move_screenshot_to(attempt_path, path)
+        @manager.move(attempt_path, path)
       end
 
       def cleanup_attempts
-        @manager.cleanup_attempts_screenshots(self)
+        @manager.cleanup_attempts!(self)
         @attempts_count = 0
       end
 
@@ -78,10 +63,14 @@ def initialize(root)
       @root = Pathname.new(root)
     end
 
-    def snap_for(screenshot_full_name, screenshot_format = "png")
+    def snapshot(screenshot_full_name, screenshot_format = "png")
       Snap.new(screenshot_full_name, screenshot_format, manager: self)
     end
 
+    def self.snapshot(screenshot_full_name, screenshot_format = "png")
+      instance.snapshot(screenshot_full_name, screenshot_format)
+    end
+
     def abs_path_for(relative_path)
       @root / relative_path
     end
@@ -97,28 +86,20 @@ def provision_snap_with(snap, path, version: :actual)
       FileUtils.cp(path, managed_path)
     end
 
-    def self.provision_snap_with(snap, path, version: :actual)
-      instance.provision_snap_with(snap, path, version: version)
-    end
-
     def create_output_directory_for(path = nil)
       path ? path.dirname.mkpath : root.mkpath
     end
 
     # TODO: rename to delete!
-    def clean!
-      FileUtils.rm_rf root
-    end
-
-    def self.clean!
-      instance.clean!
+    def cleanup!
+      FileUtils.rm_rf root, secure: true
     end
 
-    def cleanup_attempts_screenshots(snapshot)
-      FileUtils.rm_rf snapshot.find_attempts_paths
+    def cleanup_attempts!(snapshot)
+      FileUtils.rm_rf snapshot.find_attempts_paths, secure: true
     end
 
-    def move_screenshot_to(new_screenshot_path, screenshot_path)
+    def move(new_screenshot_path, screenshot_path)
       FileUtils.mv(new_screenshot_path, screenshot_path, force: true)
     end
 
@@ -130,14 +111,6 @@ def self.screenshots
       instance.screenshots
     end
 
-    def self.snapshot(screenshot_full_name, screenshot_format = "png")
-      instance.snap_for(screenshot_full_name, screenshot_format)
-    end
-
-    def self.move_screenshot_to(new_screenshot_path, screenshot_path)
-      instance.move_screenshot_to(new_screenshot_path, screenshot_path)
-    end
-
     def self.root
       instance.root
     end
diff --git a/test/capybara/screenshot/diff/stable_screenshoter_test.rb b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
index 717c5fd0..d651e54a 100644
--- a/test/capybara/screenshot/diff/stable_screenshoter_test.rb
+++ b/test/capybara/screenshot/diff/stable_screenshoter_test.rb
@@ -14,7 +14,7 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
         end
 
         teardown do
-          @manager.clean!
+          @manager.cleanup!
         end
 
         test "#take_stable_screenshot several iterations to take stable screenshot" do
@@ -27,7 +27,7 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
           mock.expect(:quick_equal?, true)
 
           ImageCompare.stub :new, mock do
-            snap = @manager.snap_for("02_a")
+            snap = @manager.snapshot("02_a")
             take_stable_screenshot_with(snap)
           end
 
@@ -36,13 +36,13 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
 
         test "#take_stable_screenshot without wait raises any error" do
           assert_raises ArgumentError, "wait should be provided" do
-            take_stable_screenshot_with(@manager.snap_for("02_a"), wait: nil)
+            take_stable_screenshot_with(@manager.snapshot("02_a"), wait: nil)
           end
         end
 
         test "#take_stable_screenshot without stability_time_limit raises any error" do
           assert_raises ArgumentError, "stability_time_limit should be provided" do
-            take_stable_screenshot_with(@manager.snap_for("02_a"), stability_time_limit: nil)
+            take_stable_screenshot_with(@manager.snapshot("02_a"), stability_time_limit: nil)
           end
         end
 
@@ -53,7 +53,7 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
           mock.expect(:quick_equal?, false)
           mock.expect(:quick_equal?, true)
 
-          snap = @manager.snap_for("02_a")
+          snap = @manager.snapshot("02_a")
           assert_not_predicate snap.path, :exist?
 
           ImageCompare.stub :new, mock do
@@ -69,13 +69,13 @@ class StableScreenshoterTest < ActionDispatch::IntegrationTest
         end
 
         test "#take_comparison_screenshot fail on missing find stable image in time and generates annotated history screenshots" do
-          snap = @manager.snap_for("01_a")
+          snap = @manager.snapshot("01_a")
 
           screenshot_path = snap.path
 
           # Stub annotated files for generated comparison annotations
           # We need to have different from screenshot_path name because of other stubs
-          pseudo_snap_for_annotations = @manager.snap_for("02_a")
+          pseudo_snap_for_annotations = @manager.snapshot("02_a")
           annotated_screenshot_path = pseudo_snap_for_annotations.path
           annotated_attempts_paths = [
             [annotated_screenshot_path.sub_ext(".attempt_01.latest.png"), annotated_screenshot_path.sub_ext(".attempt_01.committed.png")],
diff --git a/test/capybara_screenshot_diff/snap_manager_test.rb b/test/capybara_screenshot_diff/snap_manager_test.rb
index 43a6a2e1..c5315828 100644
--- a/test/capybara_screenshot_diff/snap_manager_test.rb
+++ b/test/capybara_screenshot_diff/snap_manager_test.rb
@@ -9,11 +9,11 @@ class SnapManagerTest < ActiveSupport::TestCase
     end
 
     teardown do
-      @manager.clean!
+      @manager.cleanup!
     end
 
     test "#provision_snap_with copies the file to the snap path" do
-      snap = @manager.snap_for("test_image")
+      snap = @manager.snapshot("test_image")
       path = fixture_image_path_from("a")
 
       @manager.provision_snap_with(snap, path)
@@ -23,7 +23,7 @@ class SnapManagerTest < ActiveSupport::TestCase
     end
 
     test "#provision_snap_with populate the base version of the snapshot" do
-      snap = @manager.snap_for("test_image")
+      snap = @manager.snapshot("test_image")
       path = fixture_image_path_from("a")
 
       @manager.provision_snap_with(snap, path, version: :base)
diff --git a/test/support/stub_test_methods.rb b/test/support/stub_test_methods.rb
index 20906c96..2498e83c 100644
--- a/test/support/stub_test_methods.rb
+++ b/test/support/stub_test_methods.rb
@@ -15,14 +15,14 @@ module TestMethodsStub
           end
 
           teardown do
-            @manager.clean!
+            @manager.cleanup!
             Diff.screenshoter = Screenshoter
           end
         end
 
         # Prepare comparison images and build ImageCompare for them
         def make_comparison(fixture_base_image, fixture_new_image, destination: "screenshot", **options)
-          snap = @manager.snap_for(destination)
+          snap = @manager.snapshot(destination)
 
           set_test_images(snap, fixture_base_image, fixture_new_image)
 

From c0f3292f141301fcb1941506a716211075f44e2e Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 19:08:44 +0200
Subject: [PATCH 19/23] chore: cleanup

---
 .../screenshot/diff/screenshot_matcher.rb     | 50 +++++++++----------
 .../screenshot/diff/stable_screenshoter.rb    |  6 +--
 2 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/lib/capybara/screenshot/diff/screenshot_matcher.rb b/lib/capybara/screenshot/diff/screenshot_matcher.rb
index 786e2ea9..cfd8fdc9 100644
--- a/lib/capybara/screenshot/diff/screenshot_matcher.rb
+++ b/lib/capybara/screenshot/diff/screenshot_matcher.rb
@@ -21,14 +21,6 @@ def initialize(screenshot_full_name, options = {})
           @snapshot = CapybaraScreenshotDiff::SnapManager.snapshot(screenshot_full_name, @screenshot_format)
         end
 
-        def screenshot_path
-          @snapshot.path
-        end
-
-        def base_screenshot_path
-          @snapshot.base_path
-        end
-
         def build_screenshot_matches_job
           # TODO: Move this into screenshot stage, in order to re-evaluate coordinates after page updates
           return if BrowserHelpers.window_size_is_wrong?(Screenshot.window_size)
@@ -42,35 +34,41 @@ def build_screenshot_matches_job
           driver_options[:skip_area] = area_calculator.calculate_skip_area
           driver_options[:driver] = Drivers.for(driver_options[:driver])
 
-          checkout_base_screenshot
+          @snapshot.checkout_base_screenshot
 
           # When fail_if_new is true no need to create screenshot if base screenshot is missing
-          return if Capybara::Screenshot::Diff.fail_if_new && !base_screenshot_path.exist?
-
-          capture_options = {
-            # screenshot options
-            capybara_screenshot_options: driver_options[:capybara_screenshot_options],
-            crop: driver_options.delete(:crop),
-            # delivery options
-            screenshot_format: driver_options[:screenshot_format],
-            # stability options
-            stability_time_limit: driver_options.delete(:stability_time_limit),
-            wait: driver_options.delete(:wait)
-          }
+          return if Capybara::Screenshot::Diff.fail_if_new && !@snapshot.base_path.exist?
+
+          capture_options, comparison_options = extract_capture_and_comparison_options!(driver_options)
 
           # Load new screenshot from Browser
-          comparison_options = driver_options
-          take_comparison_screenshot(capture_options, comparison_options, screenshot_path, @snapshot)
+          take_comparison_screenshot(capture_options, comparison_options, @snapshot)
 
           # Pre-computation: No need to compare without base screenshot
-          return unless base_screenshot_path.exist?
+          return unless @snapshot.base_path.exist?
 
           # Add comparison job in the queue
-          [screenshot_full_name, ImageCompare.new(screenshot_path, base_screenshot_path, driver_options)]
+          [screenshot_full_name, ImageCompare.new(@snapshot.path, @snapshot.base_path, comparison_options)]
         end
 
         private
 
+        def extract_capture_and_comparison_options!(driver_options = {})
+          [
+            {
+              # screenshot options
+              capybara_screenshot_options: driver_options[:capybara_screenshot_options],
+              crop: driver_options.delete(:crop),
+              # delivery options
+              screenshot_format: driver_options[:screenshot_format],
+              # stability options
+              stability_time_limit: driver_options.delete(:stability_time_limit),
+              wait: driver_options.delete(:wait)
+            },
+            driver_options
+          ]
+        end
+
         def checkout_base_screenshot
           @snapshot.checkout_base_screenshot
         end
@@ -78,7 +76,7 @@ def checkout_base_screenshot
         # Try to get screenshot from browser.
         # On `stability_time_limit` it checks that page stop updating by comparison several screenshot attempts
         # On reaching `wait` limit then it has been failed. On failing we annotate screenshot attempts to help to debug
-        def take_comparison_screenshot(capture_options, comparison_options, screenshot_path, snapshot = nil)
+        def take_comparison_screenshot(capture_options, comparison_options, snapshot = nil)
           screenshoter = build_screenshoter_for(capture_options, comparison_options)
           screenshoter.take_comparison_screenshot(snapshot)
         end
diff --git a/lib/capybara/screenshot/diff/stable_screenshoter.rb b/lib/capybara/screenshot/diff/stable_screenshoter.rb
index 4f7acdf8..8a8d38d2 100644
--- a/lib/capybara/screenshot/diff/stable_screenshoter.rb
+++ b/lib/capybara/screenshot/diff/stable_screenshoter.rb
@@ -16,14 +16,14 @@ class StableScreenshoter
         # @param capture_options [Hash] The options for capturing screenshots, must include `:stability_time_limit` and `:wait`.
         # @param comparison_options [Hash, nil] The options for comparing screenshots, defaults to `nil` which uses `Diff.default_options`.
         # @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 = nil)
-          @stability_time_limit, @wait = capture_options.fetch_values(:stability_time_limit, :wait)
+        def initialize(capture_options, comparison_options = {})
+          @stability_time_limit, @wait = capture_options.fetch_values(*STABILITY_OPTIONS)
 
           raise ArgumentError, "wait should be provided for stable screenshots" unless wait
           raise ArgumentError, "stability_time_limit should be provided for stable screenshots" unless stability_time_limit
           raise ArgumentError, "stability_time_limit (#{stability_time_limit}) should be less or equal than wait (#{wait}) for stable screenshots" unless stability_time_limit <= wait
 
-          @comparison_options = comparison_options || Diff.default_options
+          @comparison_options = comparison_options
 
           driver = Diff::Drivers.for(@comparison_options)
           @screenshoter = Diff.screenshoter.new(capture_options.except(*STABILITY_OPTIONS), driver)

From 0f0a915379761b04fb2f5414c91493378d821c9b Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 19:13:02 +0200
Subject: [PATCH 20/23] chore: cleanup

---
 lib/capybara/screenshot/diff/screenshot_matcher.rb | 4 ----
 lib/capybara/screenshot/diff/screenshoter.rb       | 4 ----
 2 files changed, 8 deletions(-)

diff --git a/lib/capybara/screenshot/diff/screenshot_matcher.rb b/lib/capybara/screenshot/diff/screenshot_matcher.rb
index cfd8fdc9..0e92fe21 100644
--- a/lib/capybara/screenshot/diff/screenshot_matcher.rb
+++ b/lib/capybara/screenshot/diff/screenshot_matcher.rb
@@ -69,10 +69,6 @@ def extract_capture_and_comparison_options!(driver_options = {})
           ]
         end
 
-        def checkout_base_screenshot
-          @snapshot.checkout_base_screenshot
-        end
-
         # Try to get screenshot from browser.
         # On `stability_time_limit` it checks that page stop updating by comparison several screenshot attempts
         # On reaching `wait` limit then it has been failed. On failing we annotate screenshot attempts to help to debug
diff --git a/lib/capybara/screenshot/diff/screenshoter.rb b/lib/capybara/screenshot/diff/screenshoter.rb
index 2c6dceb3..7718746c 100644
--- a/lib/capybara/screenshot/diff/screenshoter.rb
+++ b/lib/capybara/screenshot/diff/screenshoter.rb
@@ -21,10 +21,6 @@ def wait
         @capture_options[:wait]
       end
 
-      def screenshot_format
-        @capture_options[:screenshot_format] || "png"
-      end
-
       def capybara_screenshot_options
         @capture_options[:capybara_screenshot_options] || {}
       end

From 7a324e8a102f0a63d4d6877781c416237da1095d Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Wed, 31 Jul 2024 22:09:19 +0200
Subject: [PATCH 21/23] refact: move Snap to separate file

---
 lib/capybara_screenshot_diff/snap.rb         | 55 ++++++++++++++++++++
 lib/capybara_screenshot_diff/snap_manager.rb | 53 +------------------
 2 files changed, 57 insertions(+), 51 deletions(-)
 create mode 100644 lib/capybara_screenshot_diff/snap.rb

diff --git a/lib/capybara_screenshot_diff/snap.rb b/lib/capybara_screenshot_diff/snap.rb
new file mode 100644
index 00000000..cbc95696
--- /dev/null
+++ b/lib/capybara_screenshot_diff/snap.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+module CapybaraScreenshotDiff
+  class Snap
+    attr_reader :full_name, :format, :path, :base_path, :manager, :attempt_path, :prev_attempt_path, :attempts_count
+
+    def initialize(full_name, format, manager: SnapManager.instance)
+      @full_name = full_name
+      @format = format
+      @path = manager.abs_path_for(Pathname.new(@full_name).sub_ext(".#{@format}"))
+      @base_path = @path.sub_ext(".base.#{@format}")
+      @manager = manager
+      @attempts_count = 0
+    end
+
+    def delete!
+      path.delete if path.exist?
+      base_path.delete if base_path.exist?
+      cleanup_attempts
+    end
+
+    def checkout_base_screenshot
+      @manager.checkout_file(path, base_path)
+    end
+
+    def path_for(version = :actual)
+      case version
+      when :base
+        base_path
+      else
+        path
+      end
+    end
+
+    def next_attempt_path!
+      @prev_attempt_path = @attempt_path
+      @attempt_path = path.sub_ext(sprintf(".attempt_%02i.#{format}", @attempts_count))
+    ensure
+      @attempts_count += 1
+    end
+
+    def commit_last_attempt
+      @manager.move(attempt_path, path)
+    end
+
+    def cleanup_attempts
+      @manager.cleanup_attempts!(self)
+      @attempts_count = 0
+    end
+
+    def find_attempts_paths
+      Dir[@manager.abs_path_for "**/#{full_name}.attempt_*.#{format}"]
+    end
+  end
+end
diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index db6a638b..b9c68101 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -3,59 +3,10 @@
 require "capybara/screenshot/diff/vcs"
 require "active_support/core_ext/module/attribute_accessors"
 
+require "capybara_screenshot_diff/snap"
+
 module CapybaraScreenshotDiff
   class SnapManager
-    class Snap
-      attr_reader :full_name, :format, :path, :base_path, :manager, :attempt_path, :prev_attempt_path, :attempts_count
-
-      def initialize(full_name, format, manager: SnapManager.instance)
-        @full_name = full_name
-        @format = format
-        @path = manager.abs_path_for(Pathname.new(@full_name).sub_ext(".#{@format}"))
-        @base_path = @path.sub_ext(".base.#{@format}")
-        @manager = manager
-        @attempts_count = 0
-      end
-
-      def delete!
-        path.delete if path.exist?
-        base_path.delete if base_path.exist?
-        cleanup_attempts
-      end
-
-      def checkout_base_screenshot
-        @manager.checkout_file(path, base_path)
-      end
-
-      def path_for(version = :actual)
-        case version
-        when :base
-          base_path
-        else
-          path
-        end
-      end
-
-      def next_attempt_path!
-        @prev_attempt_path = @attempt_path
-        @attempt_path = path.sub_ext(sprintf(".attempt_%02i.#{format}", @attempts_count))
-      ensure
-        @attempts_count += 1
-      end
-
-      def commit_last_attempt
-        @manager.move(attempt_path, path)
-      end
-
-      def cleanup_attempts
-        @manager.cleanup_attempts!(self)
-        @attempts_count = 0
-      end
-
-      def find_attempts_paths
-        Dir[@manager.abs_path_for "**/#{full_name}.attempt_*.#{format}"]
-      end
-    end
 
     attr_reader :root
 

From c82950f3f58a2376273b4cbdd1f81db7261139e8 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Fri, 2 Aug 2024 11:04:34 +0200
Subject: [PATCH 22/23] refact: extracted annotation

---
 .../screenshot/diff/stable_screenshoter.rb    | 32 ++----------
 .../attempts_reporter.rb                      | 49 +++++++++++++++++++
 lib/capybara_screenshot_diff/snap_manager.rb  |  1 -
 3 files changed, 52 insertions(+), 30 deletions(-)
 create mode 100644 lib/capybara_screenshot_diff/attempts_reporter.rb

diff --git a/lib/capybara/screenshot/diff/stable_screenshoter.rb b/lib/capybara/screenshot/diff/stable_screenshoter.rb
index 8a8d38d2..992e9e8b 100644
--- a/lib/capybara/screenshot/diff/stable_screenshoter.rb
+++ b/lib/capybara/screenshot/diff/stable_screenshoter.rb
@@ -96,37 +96,11 @@ def build_last_attempts_comparison_for(snapshot)
 
         # TODO: Move to the HistoricalReporter
         def annotate_attempts_and_fail!(snapshot)
-          screenshot_attempts = snapshot.find_attempts_paths
-
-          annotate_stabilization_images(screenshot_attempts)
+          require "capybara_screenshot_diff/attempts_reporter"
+          attempts_reporter = CapybaraScreenshotDiff::AttemptsReporter.new(snapshot, @comparison_options, {wait: wait, stability_time_limit: stability_time_limit})
 
           # TODO: Move fail to the queue after tests passed
-          fail("Could not get stable screenshot within #{wait}s:\n#{screenshot_attempts.join("\n")}")
-        end
-
-        # TODO: Add tests that we annotate all files except first one
-        def annotate_stabilization_images(attempts_screenshot_paths)
-          previous_file = nil
-          attempts_screenshot_paths.reverse_each do |file_name|
-            if previous_file && File.exist?(previous_file)
-              attempts_comparison = build_comparison_for(file_name, previous_file)
-
-              if attempts_comparison.different?
-                FileUtils.mv(attempts_comparison.reporter.annotated_base_image_path, previous_file, force: true)
-              else
-                warn "[capybara-screenshot-diff] Some attempts was stable, but mistakenly marked as not: " \
-                       "#{previous_file} and #{file_name} are equal"
-              end
-
-              FileUtils.rm(attempts_comparison.reporter.annotated_image_path, force: true)
-            end
-
-            previous_file = file_name
-          end
-        end
-
-        def build_comparison_for(attempt_path, previous_attempt_path)
-          ImageCompare.new(attempt_path, previous_attempt_path, @comparison_options)
+          fail(attempts_reporter.generate)
         end
       end
     end
diff --git a/lib/capybara_screenshot_diff/attempts_reporter.rb b/lib/capybara_screenshot_diff/attempts_reporter.rb
new file mode 100644
index 00000000..08ee3f92
--- /dev/null
+++ b/lib/capybara_screenshot_diff/attempts_reporter.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require "capybara/screenshot/diff/image_compare"
+
+module CapybaraScreenshotDiff
+  class AttemptsReporter
+    def initialize(snapshot, comparison_options, stability_options = {})
+      @snapshot = snapshot
+      @comparison_options = comparison_options
+      @wait = stability_options[:wait]
+    end
+
+    def generate
+      attempts_screenshot_paths = @snapshot.find_attempts_paths
+
+      annotate_attempts(attempts_screenshot_paths)
+
+      "Could not get stable screenshot within #{@wait}s:\n#{attempts_screenshot_paths.join("\n")}"
+    end
+
+    def build_comparison_for(attempt_path, previous_attempt_path)
+      Capybara::Screenshot::Diff::ImageCompare.new(attempt_path, previous_attempt_path, @comparison_options)
+    end
+
+    private
+
+    def annotate_attempts(attempts_screenshot_paths)
+      previous_file = nil
+      attempts_screenshot_paths.reverse_each do |file_name|
+        if previous_file && File.exist?(previous_file)
+          attempts_comparison = build_comparison_for(file_name, previous_file)
+
+          if attempts_comparison.different?
+            FileUtils.mv(attempts_comparison.reporter.annotated_base_image_path, previous_file, force: true)
+          else
+            warn "[capybara-screenshot-diff] Some attempts was stable, but mistakenly marked as not: " \
+                   "#{previous_file} and #{file_name} are equal"
+          end
+
+          FileUtils.rm(attempts_comparison.reporter.annotated_image_path, force: true)
+        end
+
+        previous_file = file_name
+      end
+
+      previous_file
+    end
+  end
+end
diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index b9c68101..a6f48eb1 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -7,7 +7,6 @@
 
 module CapybaraScreenshotDiff
   class SnapManager
-
     attr_reader :root
 
     def initialize(root)

From dd6945027bca58662d89a0de7bf4192770024023 Mon Sep 17 00:00:00 2001
From: Paul Keen <125715+pftg@users.noreply.github.com>
Date: Sun, 11 Aug 2024 16:28:29 +0200
Subject: [PATCH 23/23] fix: cleanup

---
 lib/capybara_screenshot_diff/snap_manager.rb | 4 ++++
 test/capybara/screenshot/diff_test.rb        | 3 +--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/capybara_screenshot_diff/snap_manager.rb b/lib/capybara_screenshot_diff/snap_manager.rb
index a6f48eb1..0a75c98d 100644
--- a/lib/capybara_screenshot_diff/snap_manager.rb
+++ b/lib/capybara_screenshot_diff/snap_manager.rb
@@ -45,6 +45,10 @@ def cleanup!
       FileUtils.rm_rf root, secure: true
     end
 
+    def self.cleanup!
+      instance.cleanup!
+    end
+
     def cleanup_attempts!(snapshot)
       FileUtils.rm_rf snapshot.find_attempts_paths, secure: true
     end
diff --git a/test/capybara/screenshot/diff_test.rb b/test/capybara/screenshot/diff_test.rb
index dc8b5ce8..90ef430e 100644
--- a/test/capybara/screenshot/diff_test.rb
+++ b/test/capybara/screenshot/diff_test.rb
@@ -27,8 +27,7 @@ class DiffTest < ActionDispatch::IntegrationTest
       include Diff::TestMethodsStub
 
       teardown do
-        # TODO: need to find a way to clean up the snapshots
-        # CapybaraScreenshotDiff::SnapManager.clean!
+        CapybaraScreenshotDiff::SnapManager.cleanup!
 
         Capybara::Screenshot.add_driver_path = @orig_add_driver_path
         Capybara::Screenshot.add_os_path = @orig_add_os_path