fix: the HTML report survives Rails' fork-parallel test runs (#258)

docs/reporters.md

@@ -29,46 +29,35 @@ that hook fires in the process holding the results depends on how your runner pa
 | --- | --- |
 | Serial | Written, complete. |
 | `parallelize(with: :threads)` (also the default on JRuby) | Written, complete — same failures and counts as a serial run; only the order of the entries differs. |
-| `parallelize(workers: N)` (Rails' default, forks) | **Not written at all** — and the `[snap_diff] N verified, N changed, N new …` summary line is not printed either. |
+| `parallelize(workers: N)` (Rails' default, forks) | Written, complete — one report at the usual path, merged from every worker. No configuration needed. |
 | One process per worker (`parallel_tests`, RSpec, CI sharding) | Written, but only the **last** process to finish is in it; the others are overwritten. |
 
 Under Rails' forking parallelism the workers hold the results but never finalize: Minitest skips its
 `after_run` hooks in a forked child (`Minitest.allow_fork` defaults to `false`), and the parent
-process — the one that does finalize — recorded nothing. Test results are unaffected: failures still
-fail the suite. Every image artifact is still written too (`*.diff.png`, `*.base.diff.png`,
-`*.heatmap.diff.png`), so a failure stays fully debuggable on disk. Only the HTML index is missing.
+process — the one that does finalize — recorded nothing. So each worker now hands its records to the
+parent on the way out (Rails' `run_cleanup` hook runs *inside* the worker), and the parent merges
+them before it writes the report. `N verified, N changed, N new` are the merged totals for the whole
+suite, not one worker's.
 
-`ActiveSupport.test_parallelization_threshold` defaults to 50, so a suite of 50 tests or fewer runs
-serially and keeps its report. Crossing that threshold is when the report disappears.
+The handoff goes through a scratch directory under the system temp dir, removed as soon as the merge
+is done. Nothing is written inside your repository, and a worker killed mid-write leaves nothing the
+merge will read.
 
-### Getting the report back under forked parallelism
+### Rails versions and older workarounds
 
-Rails runs `parallelize_teardown` hooks *inside* each worker, before it exits. Finalize there, and
-give each worker its own output path so workers cannot overwrite each other:
+The merge uses `ActiveSupport::Testing::Parallelization`'s worker hooks and is registered only when
+Rails is present, so a non-Rails Capybara suite is unaffected.
 
-```ruby
-# test_helper.rb
-require 'snap_diff/reporters/html'
-
-class ActiveSupport::TestCase
-  parallelize(workers: :number_of_processors)
-
-  parallelize_setup do |worker|
-    SnapDiff::Reporting.reporters.clear   # drop the auto-registered shared-path reporter
-    SnapDiff::Reporting.register(
-      SnapDiff::Reporters::HTML.new(output_path: "tmp/snap_diff_report-#{worker}.html")
-    )
-  end
+If your `test_helper.rb` still carries the old per-worker workaround —
 
-  parallelize_teardown do |_worker|
-    SnapDiff::Reporting.finalize!         # Minitest's after_run hook does not fire in a fork
-  end
-end
+```ruby
+parallelize_teardown { SnapDiff::Reporting.finalize! }   # no longer needed
 ```
 
-That gives one report per worker that had failures — a worker with none writes no file — and no
-failure is lost. Alternatively run the suite in a single process with `PARALLEL_WORKERS=1`, which
-restores the one complete report.
+— you can delete it. Left in place it does not corrupt anything: the merged report is written last,
+at the documented path, with every failure in it. It just adds noise — each worker writes its own
+partial report over the same file first, and prints its own partial summary line (`32 verified …`
+four times, then the real one).
 
 Do **not** separate workers with a per-worker `save_path`: `save_path` is also where baselines are
 read from (`SnapDiff.config.screenshot_area`), so changing it per worker points the comparison at an

docs/thread_safety.md

@@ -9,7 +9,7 @@ How `snap_diff` behaves when your test suite runs tests concurrently — Rails
 | --- | --- | --- |
 | Serial | Correct | Written, complete |
 | `parallelize(with: :threads)` — also the default on JRuby | **Correct — fully supported** | Written, complete |
-| `parallelize(workers: N)` — Rails' default, forks | Correct | Not written ([why, and how to get it back](reporters.md#parallel-test-runs)) |
+| `parallelize(workers: N)` — Rails' default, forks | Correct | Written, complete — merged from every worker ([how](reporters.md#parallel-test-runs)) |
 | One process per worker (`parallel_tests`, RSpec, CI sharding) | Correct | Written, but only the last process to finish is in it |
 
 Two rules make all of these safe:

lib/snap_diff/integrations/minitest.rb

@@ -73,4 +73,21 @@ def after_teardown
   end
 end
 
-::Minitest.after_run { SnapDiff::Reporting.finalize! } if ::Minitest.respond_to?(:after_run)
+# Under Rails' `parallelize(workers: N)` the tests run in forked children
+# that never reach `after_run`, so the report and the summary line were
+# lost on every suite past the parallelization threshold (issue #258).
+# Registering the worker-side hook has to happen before `parallelize`
+# forks; with `Bundler.require` this file loads before
+# ActiveSupport::TestCase exists, so try now and again when it appears.
+unless SnapDiff::Reporting.install_parallel_hooks!
+  if defined?(::ActiveSupport) && ::ActiveSupport.respond_to?(:on_load)
+    ::ActiveSupport.on_load(:active_support_test_case) { SnapDiff::Reporting.install_parallel_hooks! }
+  end
+end
+
+if ::Minitest.respond_to?(:after_run)
+  ::Minitest.after_run do
+    SnapDiff::Reporting.merge_parallel_fragments!
+    SnapDiff::Reporting.finalize!
+  end
+end

lib/snap_diff/reporters/html.rb

@@ -64,6 +64,22 @@ def output_path
         @output_path ||= Pathname.new(@explicit_output_path || self.class.default_output_path)
       end
 
+      # Fork-parallel handoff (issue #258). A forked worker never reaches
+      # `Minitest.after_run`, so it hands its records to the parent as
+      # plain data instead. JSON on purpose -- boring, and a half-written
+      # file is discarded rather than half-read. Image paths were resolved
+      # against `output_path`, which is the same in both processes.
+      def dump_state
+        @mutex.synchronize { {"total" => @total, "failures" => @failures} }
+      end
+
+      def merge_state!(state)
+        @mutex.synchronize do
+          @total += state["total"]
+          @failures.concat(state["failures"].map { |entry| entry.transform_keys(&:to_sym) })
+        end
+      end
+
       def passed = total - failures.size
       def failed = failures.size
 

lib/snap_diff/reporting.rb

@@ -1,5 +1,9 @@
 # frozen_string_literal: true
 
+require "fileutils"
+require "json"
+require "tmpdir"
+
 module SnapDiff
   # Process-global reporter lifecycle: registration, per-test notification,
   # end-of-suite finalization. One list of reporters for the whole process,
@@ -88,6 +92,87 @@ def finalize!
         end
       end
 
+      # --- fork-parallel reports (issue #258) ---------------------------
+      #
+      # Under Rails' default `parallelize(workers: N)` the tests run in
+      # forked children, and Minitest skips `after_run` in a forked child
+      # (`allow_fork = false`, minitest.rb:64/79). So every worker holds
+      # records and never finalizes, while the parent finalizes and holds
+      # none: no report, no summary line. Pass/fail is unaffected -- the
+      # failures marshal back over DRb -- which is what makes it quiet.
+      #
+      # Rails runs `run_cleanup_hooks` INSIDE the worker just before it
+      # exits (parallelization/worker.rb:31), so each worker dumps its
+      # records there, and the parent merges the fragments in the
+      # `Minitest.after_run` it does reach.
+
+      # Registers the worker-side dump with Rails, once per process.
+      #
+      # Feature-detected twice over: the gem must load without Rails at
+      # all, and with `Bundler.require` it loads BEFORE
+      # ActiveSupport::TestCase exists, so the caller retries from
+      # `ActiveSupport.on_load`.
+      #
+      # @return [Boolean] true when the hook is registered (now or already)
+      def install_parallel_hooks!
+        return true if @parallel_owner_pid
+        return false unless defined?(::ActiveSupport::Testing::Parallelization)
+
+        @parallel_owner_pid = Process.pid
+        ::ActiveSupport::Testing::Parallelization.run_cleanup_hook { dump_parallel_fragment }
+        true
+      end
+
+      # Outside the repository on purpose: it holds run-scoped scratch, and
+      # the alternative -- somewhere under `save_path` -- is a directory
+      # users `git add`.
+      #
+      # Keyed by the pid recorded at install time, which happens in the
+      # parent before any fork: `Process.pid` here would give each worker a
+      # directory of its own that the parent never looks in.
+      def parallel_fragments_dir
+        File.join(Dir.tmpdir, "snap_diff-fragments-#{@parallel_owner_pid}")
+      end
+
+      # Worker side. Writes to a `.tmp` name and renames it into place, so
+      # a worker killed mid-write leaves nothing the merge will read.
+      def dump_parallel_fragment
+        payload = {
+          "missing_baselines" => @mutex.synchronize { @missing_baselines.to_a },
+          "reporters" => @mutex.synchronize { @reporters.dup }
+            .map { |reporter| reporter.dump_state if reporter.respond_to?(:dump_state) }
+        }
+
+        FileUtils.mkdir_p(parallel_fragments_dir)
+        tmp = File.join(parallel_fragments_dir, "#{Process.pid}.json.tmp")
+        File.write(tmp, JSON.generate(payload))
+        File.rename(tmp, File.join(parallel_fragments_dir, "#{Process.pid}.json"))
+      end
+
+      # Parent side, called just before {finalize!}. A no-op when nothing
+      # forked, which is what keeps serial and `with: :threads` -- both of
+      # which record in the process that finalizes -- exactly as they were.
+      #
+      # Reporters are matched by position: registration happens at require
+      # time, before any fork, so the list is identical in every process.
+      def merge_parallel_fragments!
+        return unless @parallel_owner_pid == Process.pid
+
+        Dir[File.join(parallel_fragments_dir, "*.json")].sort.each do |fragment|
+          payload = JSON.parse(File.read(fragment))
+
+          @mutex.synchronize { payload["missing_baselines"].each { |name| @missing_baselines << name } }
+
+          reporters_snapshot = @mutex.synchronize { @reporters.dup }
+          payload["reporters"].each_with_index do |state, index|
+            reporter = reporters_snapshot[index]
+            reporter.merge_state!(state) if state && reporter.respond_to?(:merge_state!)
+          end
+        end
+
+        FileUtils.rm_rf(parallel_fragments_dir)
+      end
+
       # The reporters' summary line carries the COUNT of screenshots that
       # were captured without a committed baseline ("N new (not
       # verified)"); this line names them, so the next thing the reader