test/upgrade: cleanup and robustness improvements

Avoid calling top-level methods inside other tests in case some
versions of test-unit or minitest can call setup/teardown twice.
Avoid Timeout, as it is expensive and unnecessary
in some cases.

1 files changed, 50 insertions, 31 deletions

diff --git a/test/upgrade.rb b/test/upgrade.rb
index 3811b8f..b59dd9e 100644
--- a/test/upgrade.rb
+++ b/test/upgrade.rb
@@ -56,6 +56,7 @@ class TestUpgrade < Test::Unit::TestCase
     _, status = Process.waitpid2(tmp_pid)
     assert status.success?, status.inspect
 
+    wait_for_pidfile(@pid_path)
     assert_http_running
     old_pid = assert_pidfile_valid(@pid_path)
 
@@ -67,17 +68,19 @@ class TestUpgrade < Test::Unit::TestCase
     # both old and new should be running
     first_pid = assert_pidfile_valid(@old)
     assert_equal old_pid, first_pid
-    assert File.exist?(@pid_path)
+    assert File.exist?(@pid_path), "#@pid_path exists"
     new_pid = assert_pidfile_valid(@pid_path)
     assert new_pid != old_pid
     [ old_pid, new_pid ]
   end
 
-  def test_upgrade_kill(new_sig = :QUIT, wp = nil)
+  def _test_upgrade_kill(new_sig = :QUIT, wp = nil)
     old_pid, new_pid = upgrade_prepare_full(wp)
     Process.kill(new_sig, new_pid)
     wait_for_death(new_pid)
-    Timeout.timeout(30) { sleep(0.01) while File.exist?(@old) }
+    stop = Time.now + 30
+    sleep(0.01) while File.exist?(@old) && Time.now < stop
+    raise "Timed out waiting for #@old to disappear" if File.exist?(@old)
     wait_for_pidfile(@pid_path)
     orig_pid = assert_pidfile_valid(@pid_path)
     assert_equal old_pid, orig_pid
@@ -85,15 +88,27 @@ class TestUpgrade < Test::Unit::TestCase
     wait_for_death(orig_pid)
   end
 
-  def test_upgrade_kill_KILL(wp = nil)
-    test_upgrade_kill(:KILL, wp)
+  def test_upgrade_kill
+    _test_upgrade_kill
+  end
+
+  def test_upgrade_kill_KILL
+    _test_upgrade_kill_KILL
+  end
+
+  def test_upgrade_kill_ABRT
+    _test_upgrade_kill_ABRT
+  end
+
+  def _test_upgrade_kill_KILL(wp = nil)
+    _test_upgrade_kill(:KILL, wp)
   end
 
-  def test_upgrade_kill_ABRT(wp = nil)
-    test_upgrade_kill(:ABRT, wp)
+  def _test_upgrade_kill_ABRT(wp = nil)
+    _test_upgrade_kill(:ABRT, wp)
   end
 
-  def test_upgrade_normal(wp = nil)
+  def _test_upgrade_normal(wp)
     old_pid, new_pid = upgrade_prepare_full(wp)
     Process.kill(:QUIT, old_pid)
     wait_for_death(old_pid)
@@ -106,44 +121,48 @@ class TestUpgrade < Test::Unit::TestCase
   end
 
   def test_upgrade_kill_KILL_worker_process
-    test_upgrade_kill_KILL(1)
+    _test_upgrade_kill_KILL(1)
   end
 
   def test_upgrade_kill_ABRT_worker_process
-    test_upgrade_kill_ABRT(1)
+    _test_upgrade_kill_ABRT(1)
   end
 
   def test_upgrade_kill_QUIT_worker_process
-    test_upgrade_kill(:QUIT, 1)
+    _test_upgrade_kill(:QUIT, 1)
   end
 
   def test_upgrade_normal_worker_process
-    test_upgrade_normal(1)
+    _test_upgrade_normal(1)
+  end
+
+  def test_upgrade_normal
+    _test_upgrade_normal(nil)
   end
 
   def wait_for_death(pid, seconds = 30)
-    Timeout.timeout(seconds) do
-      begin
-        Process.kill(0, pid)
-        sleep(0.01)
-      rescue Errno::ESRCH
-        break
-      end while true
-    end
+    stop = Time.now + seconds
+    begin
+      Process.kill(0, pid)
+      sleep(0.01)
+    rescue Errno::ESRCH
+      return
+    end while Time.now < stop
+    raise "Timed out waiting for #{pid} to die"
   end
 
   def wait_for_pidfile(pidf, seconds = 30)
-    Timeout.timeout(seconds) do
-      begin
-        if File.exist?(pidf)
-          nr = File.read(pidf)
-          return if nr.to_i > 0
-        end
-        sleep 0.01
-      rescue Errno::ENOENT
-        sleep 0.01
-      end while true
-    end
+    stop = Time.now + seconds
+    begin
+      if File.exist?(pidf)
+        nr = File.read(pidf)
+        return if nr.to_i > 0
+      end
+      sleep 0.01
+    rescue Errno::ENOENT
+      sleep 0.01
+    end while Time.now < stop
+    raise "Timed out waiting for #{pidf} to be useful"
   end
 
   def assert_http_running