(#6346) Move the trap calls onto Signal so they're easier to stub

Once you stub signal traps in tests, you can hit ctrl+c in the middle of
a spec run and it will stop the run instead of puppet catching the
SIGINT.

I had trouble easily tracking down all the places to stub traps when the
trap was being called as a private method on applications and daemons,
but calling trap on Signal is equivalent since Kernel calls Signal.trap
and Object mixes in Kernel to provide trap as a private method on all
objects.

A bigger solution would be to refactor everywhere we call trap into a
method that's called consistently since right now we sprinkle SIGINT and
SIGTERM trap handling over applications and daemons in inconsistent
ways, returning different error codes and using different messages.
I've captured this info in ticket #6345.

Reviewed-by: Jacob Helwig <jacob@puppetlabs.com>

14 files changed, 16 insertions, 28 deletions

diff --git a/lib/puppet/application/agent.rb b/lib/puppet/application/agent.rb
index 2b75505fd..895156f11 100644
--- a/lib/puppet/application/agent.rb
+++ b/lib/puppet/application/agent.rb
@@ -9,7 +9,7 @@ class Puppet::Application::Agent < Puppet::Application
 
   def preinit
     # Do an initial trap, so that cancels don't get a stack trace.
-    trap(:INT) do
+    Signal.trap(:INT) do
       $stderr.puts "Cancelling startup"
       exit(0)
     end
diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb
index 33a70ce8a..8f5aa86d0 100644
--- a/lib/puppet/application/apply.rb
+++ b/lib/puppet/application/apply.rb
@@ -143,7 +143,7 @@ class Puppet::Application::Apply < Puppet::Application
     client = nil
     server = nil
 
-    trap(:INT) do
+    Signal.trap(:INT) do
       $stderr.puts "Exiting"
       exit(1)
     end
diff --git a/lib/puppet/application/filebucket.rb b/lib/puppet/application/filebucket.rb
index 9c3c79bc3..5c91c4f64 100644
--- a/lib/puppet/application/filebucket.rb
+++ b/lib/puppet/application/filebucket.rb
@@ -52,7 +52,7 @@ class Puppet::Application::Filebucket < Puppet::Application
     @client = nil
     @server = nil
 
-    trap(:INT) do
+    Signal.trap(:INT) do
       $stderr.puts "Cancelling"
       exit(1)
     end
diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb
index 52ef97530..9e2aaed57 100644
--- a/lib/puppet/application/inspect.rb
+++ b/lib/puppet/application/inspect.rb
@@ -82,7 +82,7 @@ Licensed under the GNU General Public License version 2
     Puppet::Util::Log.newdestination(@report)
     Puppet::Util::Log.newdestination(:console) unless options[:logset]
 
-    trap(:INT) do
+    Signal.trap(:INT) do
       $stderr.puts "Exiting"
       exit(1)
     end
diff --git a/lib/puppet/application/kick.rb b/lib/puppet/application/kick.rb
index 37aeb1ef2..b3c95e21d 100644
--- a/lib/puppet/application/kick.rb
+++ b/lib/puppet/application/kick.rb
@@ -151,7 +151,7 @@ class Puppet::Application::Kick < Puppet::Application
 
   def preinit
     [:INT, :TERM].each do |signal|
-      trap(signal) do
+      Signal.trap(signal) do
         $stderr.puts "Cancelling"
         exit(1)
       end
@@ -195,7 +195,7 @@ class Puppet::Application::Kick < Puppet::Application
 
     # If we get a signal, then kill all of our children and get out.
     [:INT, :TERM].each do |signal|
-      trap(signal) do
+      Signal.trap(signal) do
         Puppet.notice "Caught #{signal}; shutting down"
         @children.each do |pid, host|
           Process.kill("INT", pid)
diff --git a/lib/puppet/application/master.rb b/lib/puppet/application/master.rb
index fde474907..6d1cdef1b 100644
--- a/lib/puppet/application/master.rb
+++ b/lib/puppet/application/master.rb
@@ -26,7 +26,7 @@ class Puppet::Application::Master < Puppet::Application
   end
 
   def preinit
-    trap(:INT) do
+    Signal.trap(:INT) do
       $stderr.puts "Cancelling startup"
       exit(0)
     end
diff --git a/lib/puppet/application/queue.rb b/lib/puppet/application/queue.rb
index 239f6b2ad..ede47d0a6 100644
--- a/lib/puppet/application/queue.rb
+++ b/lib/puppet/application/queue.rb
@@ -15,13 +15,13 @@ class Puppet::Application::Queue < Puppet::Application
     # Do an initial trap, so that cancels don't get a stack trace.
 
     # This exits with exit code 1
-    trap(:INT) do
+    Signal.trap(:INT) do
       $stderr.puts "Caught SIGINT; shutting down"
       exit(1)
     end
 
     # This is a normal shutdown, so code 0
-    trap(:TERM) do
+    Signal.trap(:TERM) do
       $stderr.puts "Caught SIGTERM; shutting down"
       exit(0)
     end
diff --git a/lib/puppet/daemon.rb b/lib/puppet/daemon.rb
index c76d63a54..22630ffb8 100755
--- a/lib/puppet/daemon.rb
+++ b/lib/puppet/daemon.rb
@@ -95,7 +95,7 @@ class Puppet::Daemon
     # extended signals not supported under windows
     signals.update({:HUP => :restart, :USR1 => :reload, :USR2 => :reopen_logs }) unless Puppet.features.microsoft_windows?
     signals.each do |signal, method|
-      trap(signal) do
+      Signal.trap(signal) do
         Puppet.notice "Caught #{signal}; calling #{method}"
         send(method)
       end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index ed4e826c9..a374fb008 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -64,6 +64,7 @@ RSpec.configure do |config|
     # these globals are set by Application
     $puppet_application_mode = nil
     $puppet_application_name = nil
+    Signal.stubs(:trap)
 
     # Set the confdir and vardir to gibberish so that tests
     # have to be correctly mocked.
diff --git a/spec/unit/application/agent_spec.rb b/spec/unit/application/agent_spec.rb
index ff504eedf..9fc7879c9 100755
--- a/spec/unit/application/agent_spec.rb
+++ b/spec/unit/application/agent_spec.rb
@@ -50,12 +50,8 @@ describe Puppet::Application::Agent do
   end
 
   describe "in preinit" do
-    before :each do
-      @puppetd.stubs(:trap)
-    end
-
     it "should catch INT" do
-      @puppetd.expects(:trap).with { |arg,block| arg == :INT }
+      Signal.expects(:trap).with { |arg,block| arg == :INT }
 
       @puppetd.preinit
     end
diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb
index 4e1744206..ceba4a333 100755
--- a/spec/unit/application/apply_spec.rb
+++ b/spec/unit/application/apply_spec.rb
@@ -52,7 +52,6 @@ describe Puppet::Application::Apply do
 
     before :each do
       Puppet::Log.stubs(:newdestination)
-      Puppet.stubs(:trap)
       Puppet::Log.stubs(:level=)
       Puppet.stubs(:parse_config)
       Puppet::FileBucket::Dipper.stubs(:new)
@@ -78,7 +77,7 @@ describe Puppet::Application::Apply do
     end
 
     it "should set INT trap" do
-      @apply.expects(:trap).with(:INT)
+      Signal.expects(:trap).with(:INT)
 
       @apply.setup
     end
diff --git a/spec/unit/application/filebucket_spec.rb b/spec/unit/application/filebucket_spec.rb
index e6272f179..95135c7eb 100644
--- a/spec/unit/application/filebucket_spec.rb
+++ b/spec/unit/application/filebucket_spec.rb
@@ -56,7 +56,7 @@ describe Puppet::Application::Filebucket do
     end
 
     it "should trap INT" do
-      @filebucket.expects(:trap).with(:INT)
+      Signal.expects(:trap).with(:INT)
 
       @filebucket.setup
     end
diff --git a/spec/unit/application/queue_spec.rb b/spec/unit/application/queue_spec.rb
index bd0d53ab1..f8ebbd0b4 100755
--- a/spec/unit/application/queue_spec.rb
+++ b/spec/unit/application/queue_spec.rb
@@ -29,12 +29,8 @@ describe Puppet::Application::Queue do
   end
 
   describe "in preinit" do
-    before :each do
-      @queue.stubs(:trap)
-    end
-
     it "should catch INT" do
-      @queue.expects(:trap).with { |arg,block| arg == :INT }
+      Signal.expects(:trap).with { |arg,block| arg == :INT }
 
       @queue.preinit
     end
diff --git a/spec/unit/daemon_spec.rb b/spec/unit/daemon_spec.rb
index e24db7881..39592b7c9 100755
--- a/spec/unit/daemon_spec.rb
+++ b/spec/unit/daemon_spec.rb
@@ -29,13 +29,9 @@ describe Puppet::Daemon do
   end
 
   describe "when setting signal traps" do
-    before do
-      @daemon.stubs(:trap)
-    end
-
     {:INT => :stop, :TERM => :stop, :HUP => :restart, :USR1 => :reload, :USR2 => :reopen_logs}.each do |signal, method|
       it "should log and call #{method} when it receives #{signal}" do
-        @daemon.expects(:trap).with(signal).yields
+        Signal.expects(:trap).with(signal).yields
 
         Puppet.expects(:notice)