Ensure CLI can shutdown cleanly with multiple queues and timeout

CLI will shut down all executor instances on exit; previously only shut down its own instances.

lib/good_job.rb

@@ -126,13 +126,13 @@ def self.restart(timeout: -1)
     _shutdown_all(_executables, :restart, timeout: timeout)
   end
 
-  # Sends +#shutdown+ or +#restart+ to executable objects ({GoodJob::Notifier}, {GoodJob::Poller}, {GoodJob::Scheduler})
+  # Sends +#shutdown+ or +#restart+ to executable objects ({GoodJob::Notifier}, {GoodJob::Poller}, {GoodJob::Scheduler}, {GoodJob::MultiScheduler})
   # @param executables [Array<Notifier, Poller, Scheduler, MultiScheduler>] Objects to shut down.
   # @param method_name [:symbol] Method to call, e.g. +:shutdown+ or +:restart+.
   # @param timeout [nil,Numeric]
   # @return [void]
   def self._shutdown_all(executables, method_name = :shutdown, timeout: -1)
-    if timeout.positive?
+    if timeout.is_a?(Numeric) && timeout.positive?
       executables.each { |executable| executable.send(method_name, timeout: nil) }
 
       stop_at = Time.current + timeout

lib/good_job/adapter.rb

@@ -123,7 +123,7 @@ def shutdown(timeout: :default, wait: nil)
                   timeout
                 end
 
-      executables = [@notifier, @poller, @scheduler].compact
+      executables = [@notifier, @poller, @scheduler, @cron_manager].compact
       GoodJob._shutdown_all(executables, timeout: timeout)
     end
 

lib/good_job/cli.rb

@@ -91,7 +91,7 @@ def start
       scheduler = GoodJob::Scheduler.from_configuration(configuration, warm_cache_on_initialize: true)
       notifier.recipients << [scheduler, :create_thread]
       poller.recipients << [scheduler, :create_thread]
-      cron_manager = GoodJob::CronManager.new(configuration.cron, start_on_initialize: true) if configuration.enable_cron?
+      _cron_manager = GoodJob::CronManager.new(configuration.cron, start_on_initialize: true) if configuration.enable_cron?
       @stop_good_job_executable = false
       %w[INT TERM].each do |signal|
         trap(signal) { @stop_good_job_executable = true }
@@ -102,8 +102,7 @@ def start
         break if @stop_good_job_executable || scheduler.shutdown? || notifier.shutdown?
       end
 
-      executors = [notifier, poller, cron_manager, scheduler].compact
-      GoodJob._shutdown_all(executors, timeout: configuration.shutdown_timeout)
+      GoodJob.shutdown(timeout: configuration.shutdown_timeout)
     end
 
     default_task :start

spec/lib/good_job/cli_spec.rb

@@ -9,6 +9,7 @@
   before do
     stub_const 'GoodJob::CLI::RAILS_ENVIRONMENT_RB', File.expand_path("spec/test_app/config/environment.rb")
     allow(GoodJob::Scheduler).to receive(:new).and_return scheduler_mock
+    allow(GoodJob).to receive(:shutdown).and_call_original
   end
 
   describe '#start' do
@@ -32,7 +33,7 @@
       sleep_until { cli_thread.fulfilled? }
 
       expect(GoodJob::Scheduler).to have_received(:new)
-      expect(scheduler_mock).to have_received(:shutdown)
+      expect(GoodJob).to have_received(:shutdown)
     end
 
     describe 'max threads' do

spec/lib/good_job_spec.rb

@@ -2,8 +2,9 @@
 require 'rails_helper'
 
 describe GoodJob do
-  let(:configuration) { GoodJob::Configuration.new({ queues: 'mice:1', poll_interval: -1 }) }
-  let!(:scheduler) { GoodJob::Scheduler.from_configuration(configuration) }
+  let(:configuration) { GoodJob::Configuration.new({ queues: 'mice:1;elephants:1', poll_interval: -1 }) }
+  let!(:multi_scheduler) { GoodJob::Scheduler.from_configuration(configuration) }
+  let(:scheduler) { multi_scheduler.schedulers.first }
   let!(:notifier) { GoodJob::Notifier.new([scheduler, :create_thread]) }
 
   describe '.shutdown' do
@@ -13,6 +14,13 @@
       expect(scheduler.shutdown?).to eq true
       expect(notifier.shutdown?).to eq true
     end
+
+    it 'shuts down when there is a timeout set' do
+      described_class.shutdown(timeout: 5)
+
+      expect(scheduler.shutdown?).to eq true
+      expect(notifier.shutdown?).to eq true
+    end
   end
 
   describe '.shutdown?' do
@@ -27,9 +35,16 @@
     it 'restarts down all scheduler and notifier instances' do
       described_class.shutdown
 
+      expect(scheduler.shutdown?).to eq true
+      expect(notifier.shutdown?).to eq true
+    end
+  end
+
+  describe '._shutdown_all' do
+    it 'delegates shutdown methods to multiple instances' do
       expect do
-        described_class.restart
-      end.to change(described_class, :shutdown?).from(true).to(false)
+        described_class._shutdown_all([multi_scheduler, notifier], timeout: 5)
+      end.to change(described_class, :shutdown?).from(false).to(true)
     end
   end