fix: use Minitest.after_run for report summary (#182)

pftg · claude · web-flow · commit fc8dbaaacc62 · 2026-04-13T01:02:27.000+02:00
* fix: use Minitest.after_run for report summary, not at_exit Root cause: at_exit hooks run in LIFO order. Our at_exit was registered after minitest's autorun, so it fired before tests finished — reporter had total=0 because no assertions had been processed yet. Fix: Use Minitest.after_run (runs after all tests complete) when minitest is available, falling back to at_exit for RSpec/Cucumber. Also remove screenshots&.clear from verify_screenshots! ensure block — clearing now happens in registry.reset after notify_reporters, so assertions are available for the reporter before being cleared. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: keep base image when screenshots differ for reporter Previously, base_image_path was deleted after comparison when images differed. This meant the HTML reporter couldn't show the baseline image — it appeared as a broken "Base" alt text. The base image is a VCS checkout that gets re-created on next run, so keeping it has no side effects. The reporter now shows both baseline and current images side-by-side. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: document .gitignore patterns for diff artifacts - Add .gitignore block to README Quick Start - Add .gitignore section to CI integration docs - Add generated artifacts table to organization docs - Add *.base.png to snap_diff:clean rake task - Add *.base.png to test fixtures README Files to gitignore: *.diff.png, *.base.png, *.diff.webp, *.base.webp, snap_diff_report.html. Only commit baseline screenshots. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: register both at_exit and Minitest.after_run hooks Always register at_exit (works for RSpec/Cucumber) AND additionally Minitest.after_run when Minitest is loaded (correct ordering for Minitest). The @Finalized guard in #finalize prevents double execution. Previously the else branch meant RSpec/Cucumber users got no at_exit hook when Minitest was also loaded in the same process. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: avoid double finalize call for Minitest Restore if/else — Minitest.after_run when Minitest is loaded, at_exit otherwise. Nobody runs Minitest and RSpec in the same process, so the @Finalized double-execution guard is unnecessary overhead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: register framework-native hooks for RSpec and Cucumber - Minitest: Minitest.after_run (runs after all tests) - RSpec: RSpec.configure { |c| c.after(:suite) } (runs after suite) - Cucumber/other: at_exit fallback at_exit fires in LIFO order, so it runs BEFORE the test framework finishes when registered after it. Framework-native hooks guarantee correct ordering — reporter has data when finalize runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: move finalize_reporters! to framework adapters Extract finalize_reporters! into CapybaraScreenshotDiff module and call it from each framework's native "after suite" hook: - Minitest: Minitest.after_run - RSpec: config.after(:suite) - Cucumber: AfterAll html.rb no longer detects frameworks — it just registers at_exit as fallback for unknown frameworks. Each adapter owns its hook. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
diff --git a/README.md b/README.md
@@ -60,6 +60,16 @@ doc/screenshots/
   homepage.png          <- your baseline (commit this)
 ```
 
+Add diff artifacts to `.gitignore` — these are generated at runtime and should not be committed:
+```gitignore
+# Screenshot diff artifacts (generated, not committed)
+*.diff.png
+*.base.png
+*.diff.webp
+*.base.webp
+snap_diff_report.html
+```
+
 If you skip Step 2 and push to CI, the build will fail — `fail_if_new` is `true` by default in CI.
 
 For RSpec, Cucumber, or non-Rails setup, see [Framework Setup](docs/framework-setup.md).
diff --git a/Rakefile b/Rakefile
@@ -37,8 +37,8 @@ end
 
 desc "Remove screenshot diff artifacts (keeps baselines)"
 task "snap_diff:clean" do
-  patterns = ["**/*.diff.png", "**/*.base.diff.png", "**/*.heatmap.diff.png",
-    "**/*.diff.webp", "**/*.base.diff.webp", "**/*.heatmap.diff.webp",
+  patterns = ["**/*.diff.png", "**/*.base.png", "**/*.base.diff.png", "**/*.heatmap.diff.png",
+    "**/*.diff.webp", "**/*.base.webp", "**/*.base.diff.webp", "**/*.heatmap.diff.webp",
     "**/snap_diff_report.html"]
   removed = patterns.flat_map { |p| Dir.glob("tmp/#{p}") + Dir.glob("doc/screenshots/#{p}") }.uniq
   removed.each { |f| FileUtils.rm_f(f) }
diff --git a/docs/ci-integration.md b/docs/ci-integration.md
@@ -11,6 +11,21 @@ CapybaraScreenshotDiff.serve("_site")  # or "public", "build", "dist"
 
 This sets up Capybara to serve static files and configures screenshot paths automatically.
 
+## .gitignore Setup
+
+Add these patterns to your `.gitignore` — diff artifacts are generated at runtime and should not be committed:
+
+```gitignore
+# Screenshot diff artifacts (generated, not committed)
+*.diff.png
+*.base.png
+*.diff.webp
+*.base.webp
+snap_diff_report.html
+```
+
+Only commit the baseline screenshots (e.g., `homepage.png`). The `.base.png`, `.diff.png`, `.heatmap.diff.png`, and report files are regenerated on every test run.
+
 ## GitHub Actions Integration
 
 ### 1. Enable the HTML report
diff --git a/docs/organization.md b/docs/organization.md
@@ -28,10 +28,34 @@ doc
     welcome_index
 ```
 
-To store the screen shot history, add the `doc/screenshots` directory to your
-version control system (git, svn, etc).
+To store the screenshot history, add the `doc/screenshots` directory to your
+version control system (git).
 
-    Screen shots are compared to the previously COMMITTED version of the same screen shot.
+Screenshots are compared to the previously COMMITTED version of the same screenshot.
+
+### Generated artifacts (do not commit)
+
+When a screenshot differs, the gem generates temporary diff files alongside the baseline:
+
+| Pattern | Description |
+|---------|-------------|
+| `*.base.png` | VCS checkout of the committed baseline |
+| `*.diff.png` | Annotated diff with changes highlighted |
+| `*.base.diff.png` | Annotated baseline with diff region marked |
+| `*.heatmap.diff.png` | Heatmap of pixel differences |
+| `snap_diff_report.html` | Interactive Web UI report |
+
+Add these to `.gitignore`:
+
+```gitignore
+*.diff.png
+*.base.png
+*.diff.webp
+*.base.webp
+snap_diff_report.html
+```
+
+Clean up artifacts with `rake snap_diff:clean`.
 
 ## Screenshot groups
 
diff --git a/lib/capybara_screenshot_diff/cucumber.rb b/lib/capybara_screenshot_diff/cucumber.rb
@@ -8,3 +8,5 @@
   Capybara::Screenshot::Diff.delayed = false
   Capybara::Screenshot::BrowserHelpers.resize_window_if_needed
 end
+
+AfterAll { CapybaraScreenshotDiff.finalize_reporters! }
diff --git a/lib/capybara_screenshot_diff/minitest.rb b/lib/capybara_screenshot_diff/minitest.rb
@@ -47,3 +47,5 @@ def before_teardown
     end
   end
 end
+
+::Minitest.after_run { CapybaraScreenshotDiff.finalize_reporters! } if ::Minitest.respond_to?(:after_run)
diff --git a/lib/capybara_screenshot_diff/reporters/html.rb b/lib/capybara_screenshot_diff/reporters/html.rb
@@ -136,13 +136,6 @@ def write_report
   CapybaraScreenshotDiff.reporters << CapybaraScreenshotDiff::Reporters::HTML.new(embed_images: !!ENV["CI"])
 end
 
-at_exit do
-  CapybaraScreenshotDiff.reporters_mutex.synchronize { CapybaraScreenshotDiff.reporters.dup }.each do |reporter|
-    reporter.finalize
-    if (msg = reporter.summary)
-      $stdout.puts msg
-    end
-  rescue => e
-    warn "[snap_diff] Reporter #{reporter.class} failed (#{e.class}: #{e.message})"
-  end
-end
+# Fallback for frameworks without explicit integration (e.g., Cucumber).
+# Minitest and RSpec adapters call finalize_reporters! via their native hooks.
+at_exit { CapybaraScreenshotDiff.finalize_reporters! }
diff --git a/lib/capybara_screenshot_diff/rspec.rb b/lib/capybara_screenshot_diff/rspec.rb
@@ -41,4 +41,6 @@
       end
     end
   end
+
+  config.after(:suite) { CapybaraScreenshotDiff.finalize_reporters! }
 end
diff --git a/lib/capybara_screenshot_diff/screenshot_assertion.rb b/lib/capybara_screenshot_diff/screenshot_assertion.rb
@@ -41,8 +41,6 @@ def self.verify_screenshots!(screenshots)
       test_screenshot_errors.compact!
 
       test_screenshot_errors.empty? ? nil : test_screenshot_errors
-    ensure
-      screenshots&.clear
     end
 
     # Asserts that an image has not changed compared to its baseline.
@@ -58,8 +56,6 @@ def self.assert_image_not_changed(backtrace, name, comparison)
       # Cleanup after comparisons
       if !result && comparison.base_image_path.exist?
         FileUtils.mv(comparison.base_image_path, comparison.image_path, force: true)
-      elsif !comparison.dimensions_changed?
-        FileUtils.rm_rf(comparison.base_image_path)
       end
 
       return unless result
@@ -135,6 +131,17 @@ def reporters
 
     attr_reader :reporters_mutex
 
+    def finalize_reporters!
+      reporters_mutex.synchronize { reporters.dup }.each do |reporter|
+        reporter.finalize
+        if (msg = reporter.summary)
+          $stdout.puts msg
+        end
+      rescue => e
+        warn "[snap_diff] Reporter #{reporter.class} failed (#{e.class}: #{e.message})"
+      end
+    end
+
     def_delegator :registry, :screenshot_namer
     def_delegator :registry, :verify
 
diff --git a/test/fixtures/images/README.md b/test/fixtures/images/README.md
@@ -19,6 +19,7 @@ These files are generated during test runs and should NOT be committed:
 
 | Pattern | Description |
 |---------|-------------|
+| `*.base.png` | VCS checkout of committed baseline |
 | `*.diff.png` | Annotated diff overlay |
-| `*.base.diff.png` | Annotated base image |
+| `*.base.diff.png` | Annotated base image with diff region |
 | `*.heatmap.diff.png` | Heatmap of pixel differences |
diff --git a/test/unit/dsl_test.rb b/test/unit/dsl_test.rb
@@ -152,10 +152,10 @@ def assert_no_screenshot_jobs_scheduled
       assert_not comparison.base_image_path.exist?
     end
 
-    test "#assert_image_not_changed cleans up base image when images differ" do
+    test "#assert_image_not_changed keeps base image when images differ" do
       comparison = make_comparison(:a, :b)
       assert_image_not_changed(["my_test.rb:42"], "name", comparison)
-      assert_not comparison.base_image_path.exist?
+      assert comparison.base_image_path.exist?, "base image should be kept for reporter"
     end
 
     private

-Original file line number
+Diff line change
     end
   end
 end
++
 +::Minitest.after_run { CapybaraScreenshotDiff.finalize_reporters! } if ::Minitest.respond_to?(:after_run)