Make pid aliveness checks more test-worthy

Rather than stubbing out some internal methods of the test subject,
use known good values for pid (the current process, and negative values)
Much better code coverage test that way

ee/lib/gitlab/sidekiq_cluster/cli.rb

@@ -7,7 +7,6 @@ require 'time'
 module Gitlab
   module SidekiqCluster
     class CLI
-      # How often to check for processes when terminating
       CHECK_TERMINATE_INTERVAL_SECONDS = 1
       # How long to wait in total when asking for a clean termination
       # Sidekiq default to self-terminate is 25s
@@ -69,12 +68,19 @@ module Gitlab
         SidekiqCluster.write_pid(@pid) if @pid
       end
 
+      def continue_waiting?(deadline)
+        SidekiqCluster.any_alive?(@processes) && Gitlab::Metrics::System.monotonic_time < deadline
+      end
+
+      def hard_stop_stuck_pids
+        SidekiqCluster.signal_processes(SidekiqCluster.pids_alive(@processes), :KILL)
+      end
+
       def wait_for_termination(check_interval = CHECK_TERMINATE_INTERVAL_SECONDS, terminate_timeout = TERMINATE_TIMEOUT_SECONDS)
         deadline = Gitlab::Metrics::System.monotonic_time + terminate_timeout
-        sleep(check_interval) while SidekiqCluster.any_alive?(@processes) && Gitlab::Metrics::System.monotonic_time < deadline
+        sleep(check_interval) while continue_waiting?(deadline)
 
-        # Hard-stop any that remain; they're likely stuck
-        SidekiqCluster.signal_processes(SidekiqCluster.pids_alive(@processes), :KILL)
+        hard_stop_stuck_pids
       end
 
       def trap_signals

ee/spec/lib/gitlab/sidekiq_cluster_spec.rb

@@ -128,19 +128,19 @@ describe Gitlab::SidekiqCluster do
     end
   end
 
+  # In the X_alive? checks, we check negative PIDs sometimes as a simple way
+  # to be sure the pids are definitely for non-existent processes.
+  # Note that -1 is special, and sends the signal to every process we have permission
+  # for, so we use -2, -3 etc
   describe '.all_alive?' do
     it 'returns true if all processes are alive' do
-      processes = [1]
-
-      allow(described_class).to receive(:signal).with(1, 0).and_return(true)
+      processes = [Process.pid]
 
       expect(described_class.all_alive?(processes)).to eq(true)
     end
 
     it 'returns false when a thread was not alive' do
-      processes = [1]
-
-      allow(described_class).to receive(:signal).with(1, 0).and_return(false)
+      processes = [-2]
 
       expect(described_class.all_alive?(processes)).to eq(false)
     end
@@ -148,19 +148,13 @@ describe Gitlab::SidekiqCluster do
 
   describe '.any_alive?' do
     it 'returns true if at least one process is alive' do
-      processes = [1, 2]
-
-      allow(described_class).to receive(:signal).with(1, 0).and_return(true)
-      allow(described_class).to receive(:signal).with(2, 0).and_return(false)
+      processes = [Process.pid, -2]
 
       expect(described_class.any_alive?(processes)).to eq(true)
     end
 
     it 'returns false when all threads are dead' do
-      processes = [1, 2]
-
-      allow(described_class).to receive(:signal).with(1, 0).and_return(false)
-      allow(described_class).to receive(:signal).with(2, 0).and_return(false)
+      processes = [-2, -3]
 
       expect(described_class.any_alive?(processes)).to eq(false)
     end