From 940272b6fcc03a81f731d0e8d53d9a794847293c Mon Sep 17 00:00:00 2001 From: Mark Dorison Date: Fri, 18 Sep 2026 15:50:21 -0400 Subject: [PATCH] Fix race in OrderReporterTest#test_forking `start` truncates the log file. The test forked `start` and the five `before_test` writers concurrently and only waited on all of them at the end, so the truncate could land after one or more appends and erase them. Seen on main (7074624): ruby-tests (4.0, ~> 5.11) got 4/5 lines back while two other runs of the same SHA passed. Wait for the `start` fork before spawning the writers. This matches production ordering (parent calls `start`, then workers fork) while still exercising the lazy per-worker `File.open` path the test exists for. Also drop `delete_log`: never called, and `File.exists?` was removed in Ruby 3.2. Assisted-By: devx/5182f76b-a2d3-465f-8030-398a21f6a485 --- ruby/test/minitest/queue/order_reporter_test.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/ruby/test/minitest/queue/order_reporter_test.rb b/ruby/test/minitest/queue/order_reporter_test.rb index 3ba2952d..ad29d58c 100644 --- a/ruby/test/minitest/queue/order_reporter_test.rb +++ b/ruby/test/minitest/queue/order_reporter_test.rb @@ -25,18 +25,16 @@ def test_before_test unless truffleruby? def test_forking - pid = fork do - @reporter.start - end + # `start` truncates the log. In production it runs in the parent before + # workers fork; wait for it here so the truncate can't race the appends. + Process.waitpid(fork { @reporter.start }) pids = 5.times.map do fork do @reporter.before_test(runnable(Process.pid)) @reporter.report end end - (pids + [pid]).map do |pid| - Process.waitpid(pid) - end + pids.each { |pid| Process.waitpid(pid) } assert_equal pids.map { |pid| "Minitest::Test##{pid}" }.sort, File.readlines(log_path).map(&:chomp).sort end @@ -44,10 +42,6 @@ def test_forking private - def delete_log - File.delete(log_path) if File.exists?(log_path) - end - def log_path @path ||= File.join(Dir.tmpdir, 'test_order.log') end