Fix OrderReporter producing empty test_order.log in parallel DRb mode

ruby/lib/minitest/queue/order_reporter.rb

@@ -2,38 +2,82 @@
 require 'minitest/reporters'
 
 class Minitest::Queue::OrderReporter < Minitest::Reporters::BaseReporter
+  # Hook prepended onto Minitest::Test to capture test order from within the
+  # test process. In DRb parallel mode, reporters only receive prerecord/record
+  # in the parent, but before_setup runs inside the forked worker where we can
+  # identify the process and write to a per-worker file.
+  #
+  # All file management lives here (class-level) so forked workers are
+  # self-contained and don't depend on a reporter instance crossing the fork.
+  module TestOrderTracking
+    def before_setup
+      TestOrderTracking.record(self.class.name, self.name)
+      super
+    end
+
+    @mutex = Mutex.new
+    @files = {}
+    @dir = nil
+    @basename = nil
+    @ext = nil
+
+    class << self
+      def configure(dir:, basename:, ext:)
+        @dir = dir
+        @basename = basename
+        @ext = ext
+      end
+
+      def reset
+        @dir = nil
+        @files.each_value do |f|
+          f.flush
+          f.close
+        end
+        @files.clear
+      end
+
+      def record(klass_name, test_name)
+        return unless @dir
+
+        file.puts("#{klass_name}##{test_name}")
+      end
+
+      private
+
+      def file
+        pid = Process.pid
+        @mutex.synchronize do
+          @files[pid] ||= File.open(
+            File.join(@dir, "#{@basename}.worker-#{pid}#{@ext}"),
+            'a+',
+          ).tap { |f| f.sync = true }
+        end
+      end
+    end
+  end
+
   def initialize(options = {})
     @path = options.delete(:path)
-    @file = nil
-    @flush_every = Integer(ENV.fetch('CI_QUEUE_ORDER_FLUSH_EVERY', '50'))
-    @flush_every = 1 if @flush_every < 1
-    @pending = 0
+    @dir = File.dirname(@path)
+    @basename = File.basename(@path, File.extname(@path))
+    @ext = File.extname(@path)
     super
   end
 
   def start
     super
-    file.truncate(0)
-  end
+    TestOrderTracking.configure(dir: @dir, basename: @basename, ext: @ext)
 
-  def before_test(test)
-    super
-    file.puts("#{test.class.name}##{test.name}")
-    @pending += 1
-    if @pending >= @flush_every
-      file.flush
-      @pending = 0
+    # Clean stale worker files from previous runs
+    Dir.glob(File.join(@dir, "#{@basename}.worker-*#{@ext}")).each do |f|
+      File.delete(f)
     end
   end
 
   def report
-    file.flush
-    file.close
-  end
-
-  private
-
-  def file
-    @file ||= File.open(@path, 'a+')
+    TestOrderTracking.reset
   end
 end
+
+Minitest::Test.prepend(Minitest::Queue::OrderReporter::TestOrderTracking)

ruby/test/minitest/queue/order_reporter_test.rb

@@ -9,47 +9,86 @@ def setup
       @reporter = OrderReporter.new(path: log_path)
     end
 
-    def test_start
+    def teardown
+      OrderReporter::TestOrderTracking.reset
+      cleanup_worker_files
+    end
+
+    def test_start_cleans_stale_worker_files
+      stale_path = worker_path(99999)
+      File.write(stale_path, "stale\n")
+
+      @reporter.start
+      @reporter.report
+
+      refute File.exist?(stale_path), "Stale worker file should be cleaned up"
+    end
+
+    def test_records_test_order_via_hook
       @reporter.start
+      test_instance = Minitest::Test.new('a')
+      test_instance.before_setup
       @reporter.report
-      assert_equal [], File.readlines(log_path).map(&:chomp)
+
+      assert_equal ['Minitest::Test#a'], File.readlines(worker_path(Process.pid)).map(&:chomp)
     end
 
-    def test_before_test
+    def test_records_multiple_tests
       @reporter.start
-      @reporter.before_test(runnable('a'))
-      @reporter.before_test(runnable('b'))
+      Minitest::Test.new('a').before_setup
+      Minitest::Test.new('b').before_setup
       @reporter.report
-      assert_equal ['Minitest::Test#a', 'Minitest::Test#b'], File.readlines(log_path).map(&:chomp)
+
+      assert_equal ['Minitest::Test#a', 'Minitest::Test#b'], File.readlines(worker_path(Process.pid)).map(&:chomp)
+    end
+
+    def test_noop_when_not_started
+      # before_setup should not fail or write when no reporter is active
+      OrderReporter::TestOrderTracking.reset
+      Minitest::Test.new('a').before_setup
+
+      assert_empty Dir.glob(File.join(log_dir, "test_order.worker-*.log"))
     end
 
     unless truffleruby?
-      def test_forking
-        pid = fork do
-          @reporter.start
-        end
-        pids = 5.times.map do
+      def test_forked_workers_write_per_pid_files
+        @reporter.start
+
+        pids = 3.times.map do
           fork do
-            @reporter.before_test(runnable(Process.pid))
-            @reporter.report
+            Minitest::Test.new("test_#{Process.pid}").before_setup
           end
         end
-        (pids + [pid]).map do |pid|
-          Process.waitpid(pid)
-        end
+        pids.each { |pid| Process.waitpid(pid) }
+
+        @reporter.report
 
-        assert_equal pids.map { |pid| "Minitest::Test##{pid}" }.sort, File.readlines(log_path).map(&:chomp).sort
+        pids.each do |pid|
+          path = worker_path(pid)
+          assert File.exist?(path), "Expected #{path} to exist"
+          assert_equal ["Minitest::Test#test_#{pid}"], File.readlines(path).map(&:chomp)
+        end
       end
     end
 
     private
 
-    def delete_log
-      File.delete(log_path) if File.exists?(log_path)
+    def log_dir
+      @log_dir ||= Dir.tmpdir
     end
 
     def log_path
-      @path ||= File.join(Dir.tmpdir, 'test_order.log')
+      @log_path ||= File.join(log_dir, 'test_order.log')
+    end
+
+    def worker_path(pid)
+      File.join(log_dir, "test_order.worker-#{pid}.log")
+    end
+
+    def cleanup_worker_files
+      Dir.glob(File.join(log_dir, "test_order.worker-*.log")).each do |f|
+        File.delete(f)
+      end
     end
   end
 end