Merge pull request #9932 from chef/script-resources-use-pipes

Change script resources to use pipes rather than writing to temp files

8 files changed, 265 insertions, 292 deletions

diff --git a/lib/chef/provider/batch.rb b/lib/chef/provider/batch.rb
index b79b464cbd..af52b0a36a 100644
--- a/lib/chef/provider/batch.rb
+++ b/lib/chef/provider/batch.rb
@@ -24,22 +24,15 @@ class Chef
 
       provides :batch
 
-      def initialize(new_resource, run_context)
-        super(new_resource, run_context, ".bat")
-      end
-
       def command
-        basepath = is_forced_32bit ? wow64_directory : run_context.node["kernel"]["os_info"]["system_directory"]
-
         interpreter_path = Chef::Util::PathHelper.join(basepath, interpreter)
 
-        "\"#{interpreter_path}\" #{flags} \"#{script_file.path}\""
+        "\"#{interpreter_path}\" #{new_resource.flags} /c \"#{script_file_path}\""
       end
 
-      def flags
-        new_resource.flags.nil? ? "/c" : new_resource.flags + " /c"
+      def script_extension
+        ".bat"
       end
-
     end
   end
 end
diff --git a/lib/chef/provider/powershell_script.rb b/lib/chef/provider/powershell_script.rb
index 5210b65d48..2b7c384246 100644
--- a/lib/chef/provider/powershell_script.rb
+++ b/lib/chef/provider/powershell_script.rb
@@ -26,19 +26,12 @@ class Chef
 
       provides :powershell_script
 
-      def initialize(new_resource, run_context)
-        super(new_resource, run_context, ".ps1")
-        add_exit_status_wrapper
-      end
-
       action :run do
         validate_script_syntax!
         super()
       end
 
       def command
-        basepath = is_forced_32bit ? wow64_directory : run_context.node["kernel"]["os_info"]["system_directory"]
-
         # Powershell.exe is always in "v1.0" folder (for backwards compatibility)
         interpreter_path = Chef::Util::PathHelper.join(basepath, "WindowsPowerShell", "v1.0", interpreter)
 
@@ -48,21 +41,19 @@ class Chef
         # error status of a failed Windows process that ran at the
         # end of the script, it gets changed to '1'.
         #
-        "\"#{interpreter_path}\" #{new_resource.flags} -File \"#{script_file.path}\""
+        "\"#{interpreter_path}\" #{new_resource.flags} -File \"#{script_file_path}\""
       end
 
       protected
 
-      # Process exit codes are strange with PowerShell and require
-      # special handling to cover common use cases.
-      def add_exit_status_wrapper
-        self.code = wrapper_script
+      def code
+        code = wrapper_script
         logger.trace("powershell_script provider called with script code:\n\n#{new_resource.code}\n")
         logger.trace("powershell_script provider will execute transformed code:\n\n#{code}\n")
+        code
       end
 
       def validate_script_syntax!
-        interpreter_arguments = new_resource.flags
         Tempfile.open(["chef_powershell_script-user-code", ".ps1"]) do |user_script_file|
           # Wrap the user's code in a PowerShell script block so that
           # it isn't executed. However, syntactically invalid script
@@ -80,7 +71,7 @@ class Chef
           # written to the file system at this point, which is required since
           # the intent is to execute the code just written to it.
           user_script_file.close
-          validation_command = "\"#{interpreter}\" #{interpreter_arguments} -Command \". '#{user_script_file.path}'\""
+          validation_command = "\"#{interpreter}\" #{new_resource.flags} -Command \". '#{user_script_file.path}'\""
 
           # Note that other script providers like bash allow syntax errors
           # to be suppressed by setting 'returns' to a value that the
@@ -99,6 +90,8 @@ class Chef
         end
       end
 
+      # Process exit codes are strange with PowerShell and require
+      # special handling to cover common use cases.
       # A wrapper script is used to launch user-supplied script while
       # still obtaining useful process exit codes. Unless you
       # explicitly call exit in PowerShell, the powershell.exe
@@ -182,6 +175,9 @@ class Chef
         EOH
       end
 
+      def script_extension
+        ".ps1"
+      end
     end
   end
 end
diff --git a/lib/chef/provider/script.rb b/lib/chef/provider/script.rb
index a630fa2efd..71b86e8657 100644
--- a/lib/chef/provider/script.rb
+++ b/lib/chef/provider/script.rb
@@ -34,84 +34,15 @@ class Chef
       provides :ruby
       provides :script
 
-      def_delegators :new_resource, :interpreter, :flags
-
-      attr_accessor :code
-
-      def initialize(new_resource, run_context)
-        super
-        self.code = new_resource.code
-      end
+      def_delegators :new_resource, :interpreter, :flags, :code
 
       def command
-        "\"#{interpreter}\" #{flags} \"#{script_file.path}\""
-      end
-
-      def load_current_resource
-        super
-      end
-
-      action :run do
-        script_file.puts(code)
-        script_file.close
-
-        set_owner_and_group
-
-        super()
-
-        unlink_script_file
-      end
-
-      def set_owner_and_group
-        if ChefUtils.windows?
-          # And on Windows also this is a no-op if there is no user specified.
-          grant_alternate_user_read_access
-        else
-          # FileUtils itself implements a no-op if +user+ or +group+ are nil
-          # You can prove this by running FileUtils.chown(nil,nil,'/tmp/file')
-          # as an unprivileged user.
-          FileUtils.chown(new_resource.user, new_resource.group, script_file.path)
-        end
+        "\"#{interpreter}\" #{flags}"
       end
 
-      def grant_alternate_user_read_access
-        # Do nothing if an alternate user isn't specified -- the file
-        # will already have the correct permissions for the user as part
-        # of the default ACL behavior on Windows.
-        return if new_resource.user.nil?
-
-        # Duplicate the script file's existing DACL
-        # so we can add an ACE later
-        securable_object = Chef::ReservedNames::Win32::Security::SecurableObject.new(script_file.path)
-        aces = securable_object.security_descriptor.dacl.reduce([]) { |result, current| result.push(current) }
-
-        username = new_resource.user
-
-        if new_resource.domain
-          username = new_resource.domain + '\\' + new_resource.user
-        end
-
-        # Create an ACE that allows the alternate user read access to the script
-        # file so it can be read and executed.
-        user_sid = Chef::ReservedNames::Win32::Security::SID.from_account(username)
-        read_ace = Chef::ReservedNames::Win32::Security::ACE.access_allowed(user_sid, Chef::ReservedNames::Win32::API::Security::GENERIC_READ | Chef::ReservedNames::Win32::API::Security::GENERIC_EXECUTE, 0)
-        aces.push(read_ace)
-        acl = Chef::ReservedNames::Win32::Security::ACL.create(aces)
-
-        # This actually applies the modified DACL to the file
-        # Use parentheses to bypass RuboCop / ChefStyle warning
-        # about useless setter
-        (securable_object.dacl = acl)
+      def input
+        code
       end
-
-      def script_file
-        @script_file ||= Tempfile.open("chef-script")
-      end
-
-      def unlink_script_file
-        script_file && script_file.close!
-      end
-
     end
   end
 end
diff --git a/lib/chef/provider/windows_script.rb b/lib/chef/provider/windows_script.rb
index b17d638ca6..4a17a6ff1a 100644
--- a/lib/chef/provider/windows_script.rb
+++ b/lib/chef/provider/windows_script.rb
@@ -23,52 +23,112 @@ class Chef
   class Provider
     class WindowsScript < Chef::Provider::Script
 
-      attr_reader :is_forced_32bit
-
       protected
 
-      include Chef::Mixin::WindowsArchitectureHelper
-
-      def initialize( new_resource, run_context, script_extension = "")
-        super( new_resource, run_context )
-        @script_extension = script_extension
+      attr_accessor :script_file_path
 
-        target_architecture = if new_resource.architecture.nil?
-                                node_windows_architecture(run_context.node)
-                              else
-                                new_resource.architecture
-                              end
-
-        @is_wow64 = wow64_architecture_override_required?(run_context.node, target_architecture)
+      include Chef::Mixin::WindowsArchitectureHelper
 
-        @is_forced_32bit = forced_32bit_override_required?(run_context.node, target_architecture)
+      def target_architecture
+        @target_architecture ||= if new_resource.architecture.nil?
+                                   node_windows_architecture(run_context.node)
+                                 else
+                                   new_resource.architecture
+                                 end
       end
 
-      public
+      def basepath
+        if forced_32bit_override_required?(run_context.node, target_architecture)
+          wow64_directory
+        else
+          run_context.node["kernel"]["os_info"]["system_directory"]
+        end
+      end
 
-      action :run do
+      def with_wow64_redirection_disabled
         wow64_redirection_state = nil
 
-        if @is_wow64
-          wow64_redirection_state = disable_wow64_file_redirection(@run_context.node)
+        if wow64_architecture_override_required?(run_context.node, target_architecture)
+          wow64_redirection_state = disable_wow64_file_redirection(run_context.node)
         end
 
         begin
-          super()
+          yield
         rescue
           raise
         ensure
           unless wow64_redirection_state.nil?
-            restore_wow64_file_redirection(@run_context.node, wow64_redirection_state)
+            restore_wow64_file_redirection(run_context.node, wow64_redirection_state)
           end
         end
       end
 
-      def script_file
-        base_script_name = "chef-script"
-        temp_file_arguments = [ base_script_name, @script_extension ]
+      def command
+        "\"#{interpreter}\" #{flags} \"#{script_file_path}\""
+      end
+
+      def grant_alternate_user_read_access(file_path)
+        # Do nothing if an alternate user isn't specified -- the file
+        # will already have the correct permissions for the user as part
+        # of the default ACL behavior on Windows.
+        return if new_resource.user.nil?
+
+        # Duplicate the script file's existing DACL
+        # so we can add an ACE later
+        securable_object = Chef::ReservedNames::Win32::Security::SecurableObject.new(file_path)
+        aces = securable_object.security_descriptor.dacl.reduce([]) { |result, current| result.push(current) }
+
+        username = new_resource.user
+
+        if new_resource.domain
+          username = new_resource.domain + '\\' + new_resource.user
+        end
+
+        # Create an ACE that allows the alternate user read access to the script
+        # file so it can be read and executed.
+        user_sid = Chef::ReservedNames::Win32::Security::SID.from_account(username)
+        read_ace = Chef::ReservedNames::Win32::Security::ACE.access_allowed(user_sid, Chef::ReservedNames::Win32::API::Security::GENERIC_READ | Chef::ReservedNames::Win32::API::Security::GENERIC_EXECUTE, 0)
+        aces.push(read_ace)
+        acl = Chef::ReservedNames::Win32::Security::ACL.create(aces)
+
+        # This actually applies the modified DACL to the file
+        # Use parentheses to bypass RuboCop / ChefStyle warning
+        # about useless setter
+        (securable_object.dacl = acl)
+      end
+
+      def with_temp_script_file
+        Tempfile.open(["chef-script", script_extension]) do |script_file|
+          script_file.puts(code)
+          script_file.close
+
+          grant_alternate_user_read_access(script_file.path)
+
+          # This needs to be set here so that the call to #command in Execute works.
+          self.script_file_path = script_file.path
+
+          yield
+
+          self.script_file_path = nil
+        end
+      end
+
+      def input
+        nil
+      end
+
+      public
+
+      action :run do
+        with_wow64_redirection_disabled do
+          with_temp_script_file do
+            super()
+          end
+        end
+      end
 
-        @script_file ||= Tempfile.open(temp_file_arguments)
+      def script_extension
+        raise Chef::Exceptions::Override, "You must override #{__method__} in #{self}"
       end
     end
   end
diff --git a/spec/unit/provider/batch_spec.rb b/spec/unit/provider/batch_spec.rb
new file mode 100644
index 0000000000..3ca1334489
--- /dev/null
+++ b/spec/unit/provider/batch_spec.rb
@@ -0,0 +1,130 @@
+#
+# Author:: Adam Jacob (adam@chef.io)
+# Copyright:: Copyright 2009-2016, Opscode
+# 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"
+
+describe Chef::Provider::Batch do
+  let(:node) do
+    node = Chef::Node.new
+    node.default["kernel"] = {}
+    node.default["kernel"][:machine] = :x86_64.to_s
+    node
+  end
+
+  let(:events) { Chef::EventDispatch::Dispatcher.new }
+
+  let(:run_context) { Chef::RunContext.new(node, {}, events) }
+
+  let(:new_resource) do
+    new_resource = Chef::Resource::Batch.new("cmd.exe and conquer")
+    new_resource.code %q{echo "hello"}
+    new_resource
+  end
+
+  let(:provider) { Chef::Provider::Batch.new(new_resource, run_context) }
+
+  context "#grant_alternate_user_read_access" do
+    before do
+      allow(ChefUtils).to receive(:windows?).and_return(true)
+      stub_const("Chef::ReservedNames::Win32::API::Security::GENERIC_READ", 1)
+      stub_const("Chef::ReservedNames::Win32::API::Security::GENERIC_EXECUTE", 4)
+      stub_const("Chef::ReservedNames::Win32::Security", Class.new)
+      stub_const("Chef::ReservedNames::Win32::Security::SecurableObject", Class.new)
+      stub_const("Chef::ReservedNames::Win32::Security::SID", Class.new)
+      stub_const("Chef::ReservedNames::Win32::Security::ACE", Class.new)
+      stub_const("Chef::ReservedNames::Win32::Security::ACL", Class.new)
+
+      provider.singleton_class.send(:public, :grant_alternate_user_read_access)
+    end
+
+    context "when an alternate user is not specified" do
+      it "does not attempt to set the script file's security descriptor" do
+        expect(provider).to receive(:grant_alternate_user_read_access)
+        expect(Chef::ReservedNames::Win32::Security::SecurableObject).not_to receive(:new)
+        provider.grant_alternate_user_read_access("a fake path")
+      end
+    end
+
+    context "when an alternate user is specified" do
+      let(:security_descriptor) { instance_double("Chef::ReservedNames::Win32::Security::SecurityDescriptor", dacl: []) }
+      let(:securable_object) { instance_double("Chef::ReservedNames::Win32::Security::SecurableObject", :security_descriptor => security_descriptor, :dacl= => nil) }
+
+      it "sets the script file's security descriptor" do
+        new_resource.user("toor")
+        expect(Chef::ReservedNames::Win32::Security::SecurableObject).to receive(:new).and_return(securable_object)
+        expect(Chef::ReservedNames::Win32::Security::SID).to receive(:from_account).and_return(nil)
+        expect(Chef::ReservedNames::Win32::Security::ACE).to receive(:access_allowed).and_return(nil)
+        expect(Chef::ReservedNames::Win32::Security::ACL).to receive(:create).and_return(nil)
+        expect(securable_object).to receive(:dacl=)
+        provider.grant_alternate_user_read_access("a fake path")
+      end
+    end
+  end
+
+  describe "#with_temp_script_file" do
+    before do
+      provider.singleton_class.send(:public, :with_temp_script_file)
+      provider.singleton_class.send(:public, :script_file_path)
+    end
+
+    it "should put the contents of the script in the temp file" do
+      temp_file_contents = nil
+
+      provider.with_temp_script_file do
+        temp_file_contents = File.read(provider.script_file_path)
+      end
+
+      expect(temp_file_contents.strip).to eq(%q{echo "hello"})
+    end
+  end
+
+  describe "#command" do
+    let(:basepath) { "C:\\Windows\\system32" }
+    let(:interpreter) { File.join(basepath, "cmd.exe") }
+
+    before do
+      allow(provider).to receive(:basepath).and_return(basepath)
+      provider.singleton_class.send(:public, :with_temp_script_file)
+      provider.singleton_class.send(:public, :script_file_path)
+    end
+
+    it 'should set the command to "interpreter"  "tempfile"' do
+      command = nil
+      script_file_path = nil
+      provider.with_temp_script_file do
+        command = provider.command
+        script_file_path = provider.script_file_path
+      end
+
+      expect(command).to eq(%Q{"#{interpreter}"  /c "#{script_file_path}"})
+    end
+
+    it "should set the command to 'interpreter flags tempfile'" do
+      new_resource.flags "/f"
+
+      command = nil
+      script_file_path = nil
+      provider.with_temp_script_file do
+        command = provider.command
+        script_file_path = provider.script_file_path
+      end
+
+      expect(command).to eq(%Q{"#{interpreter}" /f /c "#{script_file_path}"})
+    end
+  end
+end
diff --git a/spec/unit/provider/powershell_script_spec.rb b/spec/unit/provider/powershell_script_spec.rb
index 88aceda241..e0857a1ea4 100644
--- a/spec/unit/provider/powershell_script_spec.rb
+++ b/spec/unit/provider/powershell_script_spec.rb
@@ -18,34 +18,12 @@
 
 require "spec_helper"
 describe Chef::Provider::PowershellScript, "action_run" do
-
-  let(:powershell_version) { nil }
-  let(:node) do
-    node = Chef::Node.new
-    node.default["kernel"] = {}
-    node.default["kernel"][:machine] = :x86_64.to_s
-    unless powershell_version.nil?
-      node.default[:languages] = { powershell: { version: powershell_version } }
-    end
-    node
-  end
-
-  # code block is mandatory for the powershell provider
-  let(:code) { "" }
-
   let(:events) { Chef::EventDispatch::Dispatcher.new }
 
-  let(:run_context) { run_context = Chef::RunContext.new(node, {}, events) }
+  let(:run_context) { Chef::RunContext.new(Chef::Node.new, {}, events) }
 
   let(:new_resource) do
-    new_resource = Chef::Resource::PowershellScript.new("run some powershell code", run_context)
-    new_resource.code code
-    new_resource
-  end
-
-  def set_user_defined_flag
-    new_resource.flags "-ExecutionPolicy RemoteSigned"
-    provider
+    Chef::Resource::PowershellScript.new("run some powershell code", run_context)
   end
 
   let(:provider) do
@@ -54,32 +32,12 @@ describe Chef::Provider::PowershellScript, "action_run" do
 
   context "when setting interpreter flags" do
     before(:each) do
-      allow(provider).to receive(:is_forced_32bit).and_return(false)
-      os_info_double = double("os_info")
-      allow(provider.run_context.node["kernel"]).to receive(:[]).with("os_info").and_return(os_info_double)
-      allow(os_info_double).to receive(:[]).with("system_directory").and_return("C:\\Windows\\system32")
+      allow(provider).to receive(:basepath).and_return("C:\\Windows\\system32")
     end
 
     it "sets the -File flag as the last flag" do
       flags = provider.command.split(" ").keep_if { |flag| flag =~ /^-/ }
       expect(flags.pop).to eq("-File")
     end
-
-    let(:execution_policy_flag) do
-      provider_flags = provider.flags.split(" ")
-      # Last occurance of "executionpolicy"
-      execution_policy_index = provider_flags.map(&:downcase).rindex("-executionpolicy")
-
-      execution_policy_index ? provider_flags[execution_policy_index + 1] : nil
-    end
-
-    it "sets default -ExecutionPolicy flag to 'Bypass'" do
-      expect(execution_policy_flag).to eq("Bypass")
-    end
-
-    it "sets user defined -ExecutionPolicy flag to 'RemoteSigned'" do
-      set_user_defined_flag
-      expect(execution_policy_flag).to eq("RemoteSigned")
-    end
   end
 end
diff --git a/spec/unit/provider/script_spec.rb b/spec/unit/provider/script_spec.rb
index 68d6bdb697..18a8a3305b 100644
--- a/spec/unit/provider/script_spec.rb
+++ b/spec/unit/provider/script_spec.rb
@@ -34,124 +34,34 @@ describe Chef::Provider::Script, "action_run" do
 
   let(:provider) { Chef::Provider::Script.new(new_resource, run_context) }
 
-  let(:tempfile) { Tempfile.open("rspec-provider-script") }
-
-  before(:each) do
-    allow(provider).to receive(:shell_out!).and_return(true)
-    allow(provider).to receive(:script_file).and_return(tempfile)
-  end
-
-  context "#script_file" do
-    it "creates a temporary file to store the script" do
-      allow(provider).to receive(:script_file).and_call_original
-      expect(provider.script_file).to be_an_instance_of(Tempfile)
+  describe "#command" do
+    it "is only the intepreter in quotes by default" do
+      expect(provider.command.strip).to eq(%q{"perl"})
     end
-  end
 
-  context "#unlink_script_file" do
-    it "unlinks the tempfile" do
-      tempfile_path = tempfile.path
-      provider.unlink_script_file
-      expect(File.exist?(tempfile_path)).to be false
+    it "is the interpreter in quotes with the flags when flags are used" do
+      new_resource.flags "-f"
+      expect(provider.command).to eq(%q{"perl" -f})
     end
   end
 
-  context "when configuring the script file's security" do
-    context "when not running on Windows" do
-      before do
-        allow(ChefUtils).to receive(:windows?).and_return(false)
-      end
-      context "#set_owner_and_group" do
-        it "sets the owner and group for the script file" do
-          new_resource.user "toor"
-          new_resource.group "wheel"
-          expect(FileUtils).to receive(:chown).with("toor", "wheel", tempfile.path)
-          provider.set_owner_and_group
-        end
-      end
-    end
-
-    context "when running on Windows" do
-      before do
-        allow(ChefUtils).to receive(:windows?).and_return(true)
-        expect(new_resource.user).to eq(nil)
-        stub_const("Chef::ReservedNames::Win32::API::Security::GENERIC_READ", 1)
-        stub_const("Chef::ReservedNames::Win32::API::Security::GENERIC_EXECUTE", 4)
-        stub_const("Chef::ReservedNames::Win32::Security", Class.new)
-        stub_const("Chef::ReservedNames::Win32::Security::SecurableObject", Class.new)
-        stub_const("Chef::ReservedNames::Win32::Security::SID", Class.new)
-        stub_const("Chef::ReservedNames::Win32::Security::ACE", Class.new)
-        stub_const("Chef::ReservedNames::Win32::Security::ACL", Class.new)
-      end
-
-      context "when an alternate user is not specified" do
-        it "does not attempt to set the script file's security descriptor" do
-          expect(provider).to receive(:grant_alternate_user_read_access)
-          expect(Chef::ReservedNames::Win32::Security::SecurableObject).not_to receive(:new)
-          provider.set_owner_and_group
-        end
-      end
-
-      context "when an alternate user is specified" do
-        let(:security_descriptor) { instance_double("Chef::ReservedNames::Win32::Security::SecurityDescriptor", dacl: []) }
-        let(:securable_object) { instance_double("Chef::ReservedNames::Win32::Security::SecurableObject", :security_descriptor => security_descriptor, :dacl= => nil) }
-        it "sets the script file's security descriptor" do
-          new_resource.user("toor")
-          expect(Chef::ReservedNames::Win32::Security::SecurableObject).to receive(:new).and_return(securable_object)
-          expect(Chef::ReservedNames::Win32::Security::SID).to receive(:from_account).and_return(nil)
-          expect(Chef::ReservedNames::Win32::Security::ACE).to receive(:access_allowed).and_return(nil)
-          expect(Chef::ReservedNames::Win32::Security::ACL).to receive(:create).and_return(nil)
-          expect(securable_object).to receive(:dacl=)
-          provider.set_owner_and_group
-        end
-      end
-    end
-  end
-
-  context "with the script file set to the correct owner and group" do
+  describe "#action_run" do
     before do
-      allow(provider).to receive(:set_owner_and_group)
+      allow(provider).to receive(:stream_to_stdout?).and_return(false)
     end
 
-    describe "when writing the script to the file" do
-      it "should put the contents of the script in the temp file" do
-        allow(provider).to receive(:unlink_script_file) # stub to avoid remove
-        provider.action_run
-        expect(IO.read(tempfile.path)).to eq("$| = 1; print 'i like beans'\n")
-        provider.unlink_script_file
-      end
-
-      it "closes before executing the script and unlinks it when finished" do
-        tempfile_path = tempfile.path
-        provider.action_run
-        expect(tempfile).to be_closed
-        expect(File.exist?(tempfile_path)).to be false
-      end
-    end
-
-    describe "when running the script" do
-      let (:default_opts) do
-        { timeout: 3600, returns: 0, default_env: false, log_level: :info, log_tag: "script[run some perl code]" }
-      end
-
-      before do
-        allow(STDOUT).to receive(:tty?).and_return(false)
-      end
-
-      it 'should set the command to "interpreter"  "tempfile"' do
-        expect(provider.command).to eq(%Q{"perl"  "#{tempfile.path}"})
-      end
-
-      it "should call shell_out! with the command" do
-        expect(provider).to receive(:shell_out!).with(provider.command, default_opts).and_return(true)
-        provider.action_run
-      end
-
-      it "should set the command to 'interpreter flags tempfile'" do
-        new_resource.flags "-f"
-        expect(provider.command).to eq(%Q{"perl" -f "#{tempfile.path}"})
-      end
+    it "should call shell_out! with the command and correct options" do
+      opts = {
+        timeout: 3600,
+        returns: 0,
+        default_env: false,
+        log_level: :info,
+        log_tag: "script[run some perl code]",
+        input: "$| = 1; print 'i like beans'",
+      }
+
+      expect(provider).to receive(:shell_out!).with(provider.command, opts).and_return(true)
+      provider.action_run
     end
   end
-
 end
diff --git a/spec/unit/resource/powershell_script_spec.rb b/spec/unit/resource/powershell_script_spec.rb
index f949fb498c..6666c4749b 100644
--- a/spec/unit/resource/powershell_script_spec.rb
+++ b/spec/unit/resource/powershell_script_spec.rb
@@ -136,23 +136,18 @@ describe Chef::Resource::PowershellScript do
     it_behaves_like "a Windows script resource"
   end
 
-  context "Attribute: flags" do
+  describe "#flags" do
     let(:resource) { @resource }
-    let(:default) { "FLAGS" }
-    before(:each) do
-      allow(@resource).to receive(:default_flags).and_return(default)
-    end
-    context "When User input given" do
-      it "Appands user input after the default flags" do
-        flags = "USER FLAGS"
-        resource.flags flags
-        expect(resource.flags).to eql(default + " " + flags)
-      end
+
+    it "appends user's flags to the defaults" do
+      flags = %q{-Lunch "tacos"}
+      resource.flags = flags
+
+      expect(resource.flags).to eq("#{resource.default_flags} #{flags}")
     end
-    context "When User input is not given" do
-      it "Uses default flags" do
-        expect(resource.flags).to eql(default)
-      end
+
+    it "uses the defaults when user doesn't provide flags" do
+      expect(resource.flags).to eq(resource.default_flags)
     end
   end
 end