From b0c4de2752790422ccd62f3154b140142727b4dc Mon Sep 17 00:00:00 2001
From: Donal McBreen <donal@basecamp.com>
Date: Wed, 11 Oct 2023 15:24:24 +0200
Subject: [PATCH] Reap processes before checking for them

---
 test/integration/processes_lifecycle_test.rb | 15 +++++----------
 test/test_helper.rb                          | 18 ++++++++++--------
 2 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/test/integration/processes_lifecycle_test.rb b/test/integration/processes_lifecycle_test.rb
index 55e9ca30..de863e3e 100644
--- a/test/integration/processes_lifecycle_test.rb
+++ b/test/integration/processes_lifecycle_test.rb
@@ -13,7 +13,7 @@ class ProcessLifecycleTest < ActiveSupport::TestCase
   end
 
   teardown do
-    terminate_supervisor(raise_on_missing: false) if process_exists?(@pid)
+    terminate_supervisor if process_exists?(@pid)
   end
 
   test "enqueue jobs in multiple queues" do
@@ -35,14 +35,9 @@ class ProcessLifecycleTest < ActiveSupport::TestCase
     pause = enqueue_store_result_job("pause", pause: 2.seconds)
 
     signal_process(@pid, :KILL, wait: 1.second)
-    puts @pid
-    puts `ps aux | grep #{@pid}`
     wait_for_jobs_to_finish_for(5.seconds)
-    puts "Waited"
-    puts @pid
-    puts `ps aux | grep #{@pid}`
 
-    assert_not process_exists?(@pid, waitpid: true)
+    assert_not process_exists?(@pid)
 
     assert_completed_job_results("no pause")
     assert_job_status(no_pause, :finished)
@@ -70,7 +65,7 @@ class ProcessLifecycleTest < ActiveSupport::TestCase
     signal_process(@pid, :QUIT, wait: 1.second)
     wait_for_jobs_to_finish_for(5.seconds)
 
-    assert_not process_exists?(@pid, waitpid: true)
+    assert_not process_exists?(@pid)
 
     assert_completed_job_results("no pause")
     assert_job_status(no_pause, :finished)
@@ -186,8 +181,8 @@ class ProcessLifecycleTest < ActiveSupport::TestCase
   end
 
   private
-    def terminate_supervisor(raise_on_missing: true)
-      terminate_process(@pid, raise_on_missing: raise_on_missing)
+    def terminate_supervisor
+      terminate_process(@pid)
     end
 
     def terminate_registered_processes
diff --git a/test/test_helper.rb b/test/test_helper.rb
index e9da9825..afdd9db5 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -56,8 +56,8 @@ def assert_no_registered_processes
       end
     end
 
-    def terminate_process(pid, timeout: 10, signal: :TERM, from_parent: true, raise_on_missing: true)
-      signal_process(pid, signal, raise_on_missing: raise_on_missing)
+    def terminate_process(pid, timeout: 10, signal: :TERM, from_parent: true)
+      signal_process(pid, signal)
       wait_for_process_termination_with_timeout(pid, timeout: timeout, from_parent: from_parent)
     end
 
@@ -78,24 +78,26 @@ def wait_for_process_termination_with_timeout(pid, timeout: 10, from_parent: tru
       raise
     end
 
-    def signal_process(pid, signal, wait: nil, raise_on_missing: true)
+    def signal_process(pid, signal, wait: nil)
       Thread.new do
         sleep(wait) if wait
         Process.kill(signal, pid)
-        sleep(wait) if wait
-      rescue Errno::ESRCH
-        raise if raise_on_missing
       end
     end
 
-    def process_exists?(pid, waitpid: false)
-      Process.waitpid(pid) if waitpid
+    def process_exists?(pid)
+      reap_processes
       Process.getpgid(pid)
       true
     rescue Errno::ESRCH
       false
     end
 
+    def reap_processes
+      Process.waitpid(-1, Process::WNOHANG)
+    rescue Errno::ECHILD
+    end
+
     # Allow skipping AR query cache, necessary when running test code in multiple
     # forks. The queries done in the test might be cached and if we don't perform
     # any non-SELECT queries after previous SELECT queries were cached on the connection