From b4009d113f0f549535215a47f5c759fd36911c37 Mon Sep 17 00:00:00 2001
From: Daniel Colson <danieljamescolson@gmail.com>
Date: Wed, 16 Oct 2019 21:17:19 -0400
Subject: [PATCH] Address threadpool test TODOs

These TODOs were added in 9f4edf4c while cleaning up the tests a bit.

For the first TODO, it looks like there was a point to those two lines:
this and the previous test call `Thread.current.kill` in the thread pool
block so the threads will die once some work is added. I tried to make
this more obvious by starting the `auto_reap!` earlier and checking that
it hasn't reaped any threads that are still alive.

For the other TODO I replaced `sleep 10` with the `Thread.pass until
finish` pattern we use in some other tests in this file. Now if
`pool.shutdown(0)` fails to raise an error we finish right away rather
than waiting for the 5 second ConditionVariable timeout. I also added an
assertion to ensure we rescue the `ForceShutdown` error.
---
 test/test_thread_pool.rb | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/test/test_thread_pool.rb b/test/test_thread_pool.rb
index ed51e191..18901d6a 100644
--- a/test/test_thread_pool.rb
+++ b/test/test_thread_pool.rb
@@ -223,27 +223,32 @@ class TestThreadPool < Minitest::Test
   def test_auto_reap_dead_threads
     pool = new_pool(2,2) { Thread.current.kill }
 
+    pool.auto_reap! 0.1
+
+    pause
+
     assert_equal 2, pool.spawned
 
-    # TODO: is there a point to these two lines?
     pool << 1
     pool << 2
 
-    pool.auto_reap! 0.1
-
     pause
 
     assert_equal 0, pool.spawned
   end
 
   def test_force_shutdown_immediately
+    finish = false
+    rescued = false
+
     pool = new_pool(0, 1) do |work|
       begin
         @work_mutex.synchronize do
           @work_done.signal
         end
-        sleep 10 # TODO: do something here other than sleep
+        Thread.pass until finish
       rescue Puma::ThreadPool::ForceShutdown
+        rescued = true
       end
     end
 
@@ -252,7 +257,9 @@ class TestThreadPool < Minitest::Test
     @work_mutex.synchronize do
       @work_done.wait(@work_mutex, 5)
       pool.shutdown(0)
+      finish = true
       assert_equal 0, pool.spawned
+      assert rescued
     end
   end
 end