fix supports hash issues in service providers

- redhat provider now allows the user to override :status
- gentoo provider now allows the user to override :status and :restart
- service providers now dup the status hash and mutate their private
  copy instead of mutating the new_resource

3 files changed, 40 insertions, 22 deletions

diff --git a/spec/unit/provider/service/gentoo_service_spec.rb b/spec/unit/provider/service/gentoo_service_spec.rb
index c08982acc3..0aa7bf4529 100644
--- a/spec/unit/provider/service/gentoo_service_spec.rb
+++ b/spec/unit/provider/service/gentoo_service_spec.rb
@@ -1,7 +1,7 @@
 #
 # Author:: Lee Jensen (<ljensen@engineyard.com>)
 # Author:: AJ Christensen (<aj@opscode.com>)
-# Copyright:: Copyright (c) 2008 Opscode, Inc.
+# Copyright:: Copyright (c) 2008-2015 Chef Software, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -108,17 +108,17 @@ describe Chef::Provider::Service::Gentoo do
 
     it "should support the status command automatically" do
       @provider.load_current_resource
-      expect(@new_resource.supports[:status]).to be_truthy
+      expect(@provider.supports[:status]).to be true
     end
 
     it "should support the restart command automatically" do
       @provider.load_current_resource
-      expect(@new_resource.supports[:restart]).to be_truthy
+      expect(@provider.supports[:restart]).to be true
     end
 
     it "should not support the reload command automatically" do
       @provider.load_current_resource
-      expect(@new_resource.supports[:reload]).not_to be_truthy
+      expect(@provider.supports[:reload]).to be_falsey
     end
 
   end
diff --git a/spec/unit/provider/service/openbsd_service_spec.rb b/spec/unit/provider/service/openbsd_service_spec.rb
index 1b5206470e..d3c150a14b 100644
--- a/spec/unit/provider/service/openbsd_service_spec.rb
+++ b/spec/unit/provider/service/openbsd_service_spec.rb
@@ -35,10 +35,12 @@ describe Chef::Provider::Service::Openbsd do
     node
   end
 
+  let(:supports) { {:status => false} }
+
   let(:new_resource) do
     new_resource = Chef::Resource::Service.new("sndiod")
     new_resource.pattern("sndiod")
-    new_resource.supports({:status => false})
+    new_resource.supports(supports)
     new_resource
   end
 
@@ -106,9 +108,7 @@ describe Chef::Provider::Service::Openbsd do
     context "when the service supports status" do
       let(:status) { double(:stdout => "", :exitstatus => 0) }
 
-      before do
-        new_resource.supports({:status => true})
-      end
+      let(:supports) { { :status => true } }
 
       it "should run '/etc/rc.d/service_name status'" do
         expect(provider).to receive(:shell_out).with("/etc/rc.d/#{new_resource.service_name} check").and_return(status)
@@ -305,10 +305,12 @@ describe Chef::Provider::Service::Openbsd do
     end
 
     describe Chef::Provider::Service::Openbsd, "restart_service" do
-      it "should call 'restart' on the service_name if the resource supports it" do
-        new_resource.supports({:restart => true})
-        expect(provider).to receive(:shell_out_with_systems_locale!).with("/etc/rc.d/#{new_resource.service_name} restart")
-        provider.restart_service()
+      context "when the new_resource supports restart" do
+        let(:supports) { { restart: true } }
+        it "should call 'restart' on the service_name if the resource supports it" do
+          expect(provider).to receive(:shell_out_with_systems_locale!).with("/etc/rc.d/#{new_resource.service_name} restart")
+          provider.restart_service()
+        end
       end
 
       it "should call the restart_command if one has been specified" do
diff --git a/spec/unit/provider/service/redhat_spec.rb b/spec/unit/provider/service/redhat_spec.rb
index 1cb985714f..5aaf54d9f5 100644
--- a/spec/unit/provider/service/redhat_spec.rb
+++ b/spec/unit/provider/service/redhat_spec.rb
@@ -64,9 +64,33 @@ describe "Chef::Provider::Service::Redhat" do
     end
 
     describe "load current resource" do
-      it "sets the current enabled status to true if the service is enabled for any run level" do
+      before do
         status = double("Status", :exitstatus => 0, :stdout => "" , :stderr => "")
-        expect(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
+        allow(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
+      end
+
+      it "sets supports[:status] to true by default" do
+        chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:off   2:off   3:off   4:off   5:on  6:off", :stderr => "")
+        expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
+        expect(@provider.service_missing).to be false
+        @provider.load_current_resource
+        expect(@provider.supports[:status]).to be true
+      end
+
+      it "lets the user override supports[:status] in the new_resource" do
+        @new_resource.supports( { status: false } )
+        @new_resource.pattern "myservice"
+        chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:off   2:off   3:off   4:off   5:on  6:off", :stderr => "")
+        expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
+        foo_out = double("ps_command", :exitstatus => 0, :stdout => "a line that matches myservice", :stderr => "")
+        expect(@provider).to receive(:shell_out!).with("foo").and_return(foo_out)
+        expect(@provider.service_missing).to be false
+        expect(@provider).not_to receive(:shell_out).with("/sbin/service chef status")
+        @provider.load_current_resource
+        expect(@provider.supports[:status]).to be false
+      end
+
+      it "sets the current enabled status to true if the service is enabled for any run level" do
         chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:off   2:off   3:off   4:off   5:on  6:off", :stderr => "")
         expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
         expect(@provider.service_missing).to be false
@@ -75,8 +99,6 @@ describe "Chef::Provider::Service::Redhat" do
       end
 
       it "sets the current enabled status to false if the regex does not match" do
-        status = double("Status", :exitstatus => 0, :stdout => "" , :stderr => "")
-        expect(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
         chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:off   2:off   3:off   4:off   5:off   6:off", :stderr => "")
         expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
         expect(@provider.service_missing).to be false
@@ -85,9 +107,7 @@ describe "Chef::Provider::Service::Redhat" do
       end
 
       it "sets the current enabled status to true if the service is enabled at specified run levels" do
-        status = double("Status", :exitstatus => 0, :stdout => "" , :stderr => "")
         @new_resource.run_levels([1, 2])
-        expect(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
         chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:on   2:on   3:off   4:off   5:off   6:off", :stderr => "")
         expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
         expect(@provider.service_missing).to be false
@@ -97,9 +117,7 @@ describe "Chef::Provider::Service::Redhat" do
       end
 
       it "sets the current enabled status to false if the service is enabled at a run level it should not" do
-        status = double("Status", :exitstatus => 0, :stdout => "" , :stderr => "")
         @new_resource.run_levels([1, 2])
-        expect(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
         chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:on   2:on   3:on   4:off   5:off   6:off", :stderr => "")
         expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
         expect(@provider.service_missing).to be false
@@ -109,9 +127,7 @@ describe "Chef::Provider::Service::Redhat" do
       end
 
       it "sets the current enabled status to false if the service is not enabled at specified run levels" do
-        status = double("Status", :exitstatus => 0, :stdout => "" , :stderr => "")
         @new_resource.run_levels([ 2 ])
-        expect(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
         chkconfig = double("Chkconfig", :exitstatus => 0, :stdout => "chef    0:off   1:on   2:off   3:off   4:off   5:off   6:off", :stderr => "")
         expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", :returns => [0,1]).and_return(chkconfig)
         expect(@provider.service_missing).to be false