refact: adds usage of snapshot for take screenshot methods

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
 

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)
 

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)

test/capybara/screenshot/diff/stable_screenshoter_test.rb

@@ -28,21 +28,21 @@ 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
         end
 
         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
 

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