Execute and Script Resource improvements

- Warning on incorrect usage of the command resource in any script
  resource
- Warning on code in script resource being nil
- Specs added to force deprecation of incorrect usage in Chef-13
- Specs added around the (supported) incorrect usage in Chef-12
- Cleanup+Modernization of providers and specs
- Fixed some global state bugs around the Chef::Log.level in the spec
  tests

2 files changed, 157 insertions, 64 deletions

diff --git a/spec/functional/resource/bash_spec.rb b/spec/functional/resource/bash_spec.rb
new file mode 100644
index 0000000000..209ec4a12f
--- /dev/null
+++ b/spec/functional/resource/bash_spec.rb
@@ -0,0 +1,88 @@
+#
+# Author:: Serdar Sutay (<serdar@opscode.com>)
+# Copyright:: Copyright (c) 2014 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'spec_helper'
+require 'functional/resource/base'
+
+describe Chef::Resource::Bash, :unix_only do
+  let(:code) { "echo hello" }
+  let(:resource) {
+    resource = Chef::Resource::Bash.new("foo_resource", run_context)
+    resource.code(code)
+    resource
+  }
+
+  describe "when setting the command attribute" do
+    let (:command) { 'wizard racket' }
+
+    # in Chef-12 the `command` attribute is largely useless, but does set the identity attribute
+    # so that notifications need to target the value of the command.  it will not run the `command`
+    # and if it is given without a code block then it does nothing and always succeeds.
+    describe "in Chef-12", :chef_lt_13_only do
+      it "gets the commmand attribute from the name" do
+        expect(resource.command).to eql("foo_resource")
+      end
+
+      it "sets the resource identity to the command name" do
+        resource.command command
+        expect(resource.identity).to eql(command)
+      end
+
+      it "warns when the code is not present and a useless `command` is present" do
+        expect(Chef::Log).to receive(:warn).with(/coding error/)
+        expect(Chef::Log).to receive(:warn).with(/deprecated/)
+        resource.code nil
+        resource.command command
+        expect { resource.run_action(:run) }.not_to raise_error
+      end
+
+      describe "when the code is not present" do
+        let(:code) { nil }
+        it "warns" do
+          expect(Chef::Log).to receive(:warn)
+          expect { resource.run_action(:run) }.not_to raise_error
+        end
+      end
+    end
+
+    # in Chef-13 the `command` attribute needs to be for internal use only
+    describe "in Chef-13", :chef_gte_13_only do
+      it "should raise an exception when trying to set the command" do
+        expect { resource.command command }.to raise_error # FIXME: add a real error in Chef-13
+      end
+
+      it "should initialize the command to nil" do
+        expect(resource.command).to be_nil
+      end
+
+      describe "when the code is not present" do
+        let(:code) { nil }
+        it "raises an exception" do
+          expect { resource.run_action(:run) }.to raise_error # FIXME: add a real error in Chef-13
+          expect { resource.run_action(:run) }.not_to raise_error
+        end
+      end
+    end
+  end
+
+  it "times out when a timeout is set on the resource" do
+    resource.code 'sleep 600'
+    resource.timeout 0.1
+    expect { resource.run_action(:run) }.to raise_error(Mixlib::ShellOut::CommandTimeout)
+  end
+end
diff --git a/spec/functional/resource/execute_spec.rb b/spec/functional/resource/execute_spec.rb
index 39fef76ab0..cebcc52fcf 100644
--- a/spec/functional/resource/execute_spec.rb
+++ b/spec/functional/resource/execute_spec.rb
@@ -20,94 +20,99 @@ require 'spec_helper'
 require 'functional/resource/base'
 
 describe Chef::Resource::Execute do
-  let(:execute_resource) {
-    exec_resource = Chef::Resource::Execute.new("foo_resource", run_context)
-
-    exec_resource.environment(resource_environment) if resource_environment
-    exec_resource.cwd(resource_cwd) if resource_cwd
-    exec_resource.command("echo hello")
-    if guard
-      if guard_options
-        exec_resource.only_if(guard, guard_options)
-      else
-        exec_resource.only_if(guard)
-      end
-    end
-    exec_resource
+  let(:resource) {
+    resource = Chef::Resource::Execute.new("foo_resource", run_context)
+    resource.command("echo hello")
+    resource
   }
 
-  let(:resource_environment) { nil }
-  let(:resource_cwd) { nil }
-  let(:guard) { nil }
-  let(:guard_options) { nil }
-
   describe "when guard is ruby block" do
     it "guard can still run" do
-      execute_resource.only_if do
-        true
-      end
-      execute_resource.run_action(:run)
-      expect(execute_resource).to be_updated_by_last_action
+      resource.only_if { true }
+      resource.run_action(:run)
+      expect(resource).to be_updated_by_last_action
     end
   end
 
   describe "when parent resource sets :cwd" do
-    let(:resource_cwd) { CHEF_SPEC_DATA }
-
     let(:guard) { %{ruby -e 'exit 1 unless File.exists?("./big_json_plus_one.json")'} }
 
-    it "guard inherits :cwd from resource" do
-      execute_resource.run_action(:run)
-      expect(execute_resource).to be_updated_by_last_action
+    it "guard inherits :cwd from resource and runs" do
+      resource.cwd CHEF_SPEC_DATA
+      resource.only_if guard
+      resource.run_action(:run)
+      expect(resource).to be_updated_by_last_action
+    end
+
+    it "guard inherits :cwd from resource and does not run" do
+      resource.cwd CHEF_SPEC_DATA
+      resource.not_if guard
+      resource.run_action(:run)
+      expect(resource).not_to be_updated_by_last_action
     end
   end
 
+  # We use ruby command so that we don't need to deal with platform specific
+  # commands while testing execute resource. We set it so that the resource
+  # will be updated if the ENV variable is set to what we are intending
+  #
+  # FIXME: yeah, but invoking ruby is slow...
   describe "when parent resource sets :environment" do
-    let(:resource_environment) do
-      {
+    before do
+      resource.environment({
         "SAWS_SECRET"  => "supersecret",
-        "SAWS_KEY"     => "qwerty"
-      }
+        "SAWS_KEY"     => "qwerty",
+      })
     end
 
-    # We use ruby command so that we don't need to deal with platform specific
-    # commands while testing execute resource. We set it so that the resource
-    # will be updated if the ENV variable is set to what we are intending
-    let(:guard) { %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] != "supersecret"'} }
-
-    it "guard inherits :environment value from resource" do
-      execute_resource.run_action(:run)
-      expect(execute_resource).to be_updated_by_last_action
+    it "guard inherits :environment value from resource and runs" do
+      resource.only_if %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] != "supersecret"'}
+      resource.run_action(:run)
+      expect(resource).to be_updated_by_last_action
     end
 
-    describe "when guard sets additional values in the :environment" do
-      let(:guard) { %{ruby -e 'exit 1 if ENV["SGCE_SECRET"] != "regularsecret"'} }
-
-      let(:guard_options) do
-        {
-          :environment => { 'SGCE_SECRET' => "regularsecret" }
-        }
-      end
+    it "guard inherits :environment value from resource and does not run" do
+      resource.only_if %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] == "supersecret"'}
+      resource.run_action(:run)
+      expect(resource).not_to be_updated_by_last_action
+    end
 
-      it "guard sees merged value for in its ENV" do
-        execute_resource.run_action(:run)
-        expect(execute_resource).to be_updated_by_last_action
-      end
+    it "guard adds additional values in its :environment and runs" do
+      resource.only_if %{ruby -e 'exit 1 if ENV["SGCE_SECRET"] != "regularsecret"'}, {
+        :environment => { 'SGCE_SECRET' => "regularsecret" }
+      }
+      resource.run_action(:run)
+      expect(resource).to be_updated_by_last_action
     end
 
-    describe "when guard sets same value in the :environment" do
-      let(:guard) { %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] != "regularsecret"'} }
+    it "guard adds additional values in its :environment and does not run" do
+      resource.only_if %{ruby -e 'exit 1 if ENV["SGCE_SECRET"] == "regularsecret"'}, {
+        :environment => { 'SGCE_SECRET' => "regularsecret" }
+      }
+      resource.run_action(:run)
+      expect(resource).not_to be_updated_by_last_action
+    end
 
-      let(:guard_options) do
-        {
-          :environment => { 'SAWS_SECRET' => "regularsecret" }
-        }
-      end
+    it "guard overwrites value with its :environment and runs" do
+      resource.only_if %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] != "regularsecret"'}, {
+        :environment => { 'SAWS_SECRET' => "regularsecret" }
+      }
+      resource.run_action(:run)
+      expect(resource).to be_updated_by_last_action
+    end
 
-      it "guard sees value from guard options in its ENV" do
-        execute_resource.run_action(:run)
-        expect(execute_resource).to be_updated_by_last_action
-      end
+    it "guard overwrites value with its :environment and does not runs" do
+      resource.only_if %{ruby -e 'exit 1 if ENV["SAWS_SECRET"] == "regularsecret"'}, {
+        :environment => { 'SAWS_SECRET' => "regularsecret" }
+      }
+      resource.run_action(:run)
+      expect(resource).not_to be_updated_by_last_action
     end
   end
+
+  it "times out when a timeout is set on the resource" do
+    resource.command %{ruby -e 'sleep 600'}
+    resource.timeout 0.1
+    expect { resource.run_action(:run) }.to raise_error(Mixlib::ShellOut::CommandTimeout)
+  end
 end