fix infinite recursion and add more tests

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

2 files changed, 111 insertions, 109 deletions

diff --git a/lib/chef/mixin/shell_out.rb b/lib/chef/mixin/shell_out.rb
index a17b428159..052ba5e94c 100644
--- a/lib/chef/mixin/shell_out.rb
+++ b/lib/chef/mixin/shell_out.rb
@@ -48,33 +48,33 @@ class Chef
 
       def shell_out_compact(*args, **options) # FIXME: deprecate
         if options.empty?
-          shell_out_compact(*args)
+          shell_out(*args)
         else
-          shell_out_compact(*args, **options)
+          shell_out(*args, **options)
         end
       end
 
       def shell_out_compact!(*args, **options) # FIXME: deprecate
         if options.empty?
-          shell_out_compact!(*args)
+          shell_out!(*args)
         else
-          shell_out_compact!(*args, **options)
+          shell_out!(*args, **options)
         end
       end
 
       def shell_out_compact_timeout(*args, **options) # FIXME: deprecate
         if options.empty?
-          shell_out_compact(*args)
+          shell_out(*args)
         else
-          shell_out_compact(*args, **options)
+          shell_out(*args, **options)
         end
       end
 
       def shell_out_compact_timeout!(*args, **options) # FIXME: deprecate
         if options.empty?
-          shell_out_compact!(*args)
+          shell_out!(*args)
         else
-          shell_out_compact!(*args, **options)
+          shell_out!(*args, **options)
         end
       end
 
diff --git a/spec/unit/mixin/shell_out_spec.rb b/spec/unit/mixin/shell_out_spec.rb
index b14a928a40..df35960cc9 100644
--- a/spec/unit/mixin/shell_out_spec.rb
+++ b/spec/unit/mixin/shell_out_spec.rb
@@ -48,110 +48,113 @@ describe Chef::Mixin::ShellOut do
       ENV.update(@original_env)
     end
 
+    let(:retobj) { instance_double(Mixlib::ShellOut, "error!" => false) }
     let(:cmd) { "echo '#{rand(1000)}'" }
 
-    describe "#shell_out" do
-
-      describe "when the last argument is a Hash" do
-        describe "and environment is an option" do
-          it "should not change environment language settings when they are set to nil" do
-            options = { :environment => { "LC_ALL" => nil, "LANGUAGE" => nil, "LANG" => nil, env_path => nil } }
-            expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, options).and_return(true)
-            shell_out_obj.shell_out(cmd, options)
+    [ :shell_out, :shell_out_compact, :shell_out_compact_timeout, :shell_out!, :shell_out_compact!, :shell_out_compact_timeout! ].each do |method|
+      describe "##{method}" do
+
+        describe "when the last argument is a Hash" do
+          describe "and environment is an option" do
+            it "should not change environment language settings when they are set to nil" do
+              options = { :environment => { "LC_ALL" => nil, "LANGUAGE" => nil, "LANG" => nil, env_path => nil } }
+              expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, options).and_return(retobj)
+              shell_out_obj.send(method, cmd, options)
+            end
+
+            it "should not change environment language settings when they are set to non-nil" do
+              options = { :environment => { "LC_ALL" => "en_US.UTF-8", "LANGUAGE" => "en_US.UTF-8", "LANG" => "en_US.UTF-8", env_path => "foo:bar:baz" } }
+              expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, options).and_return(retobj)
+              shell_out_obj.send(method, cmd, options)
+            end
+
+            it "should set environment language settings to the configured internal locale when they are not present" do
+              options = { :environment => { "HOME" => "/Users/morty" } }
+              expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, {
+                :environment => {
+                  "HOME"     => "/Users/morty",
+                  "LC_ALL"   => Chef::Config[:internal_locale],
+                  "LANG"     => Chef::Config[:internal_locale],
+                  "LANGUAGE" => Chef::Config[:internal_locale],
+                  env_path   => sanitized_path,
+                },
+              }).and_return(retobj)
+              shell_out_obj.send(method, cmd, options)
+            end
+
+            it "should not mutate the options hash when it adds language settings" do
+              options = { :environment => { "HOME" => "/Users/morty" } }
+              expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, {
+                :environment => {
+                  "HOME"     => "/Users/morty",
+                  "LC_ALL"   => Chef::Config[:internal_locale],
+                  "LANG"     => Chef::Config[:internal_locale],
+                  "LANGUAGE" => Chef::Config[:internal_locale],
+                  env_path   => sanitized_path,
+                },
+              }).and_return(retobj)
+              shell_out_obj.send(method, cmd, options)
+              expect(options[:environment].has_key?("LC_ALL")).to be false
+            end
           end
 
-          it "should not change environment language settings when they are set to non-nil" do
-            options = { :environment => { "LC_ALL" => "en_US.UTF-8", "LANGUAGE" => "en_US.UTF-8", "LANG" => "en_US.UTF-8", env_path => "foo:bar:baz" } }
-            expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, options).and_return(true)
-            shell_out_obj.shell_out(cmd, options)
+          describe "and env is an option" do
+            it "should not change env when langauge options are set to nil" do
+              options = { :env => { "LC_ALL" => nil, "LANG" => nil, "LANGUAGE" => nil, env_path => nil } }
+              expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, options).and_return(retobj)
+              shell_out_obj.send(method, cmd, options)
+            end
+
+            it "should not change env when language options are set to non-nil" do
+              options = { :env => { "LC_ALL" => "de_DE.UTF-8", "LANG" => "de_DE.UTF-8", "LANGUAGE" => "de_DE.UTF-8", env_path => "foo:bar:baz" } }
+              expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, options).and_return(retobj)
+              shell_out_obj.send(method, cmd, options)
+            end
+
+            it "should set environment language settings to the configured internal locale when they are not present" do
+              options = { :env => { "HOME" => "/Users/morty" } }
+              expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, {
+                :env => {
+                  "HOME"     => "/Users/morty",
+                  "LC_ALL"   => Chef::Config[:internal_locale],
+                  "LANG"     => Chef::Config[:internal_locale],
+                  "LANGUAGE" => Chef::Config[:internal_locale],
+                  env_path   => sanitized_path,
+                },
+              }).and_return(retobj)
+              shell_out_obj.send(method, cmd, options)
+            end
+
+            it "should not mutate the options hash when it adds language settings" do
+              options = { :env => { "HOME" => "/Users/morty" } }
+              expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, {
+                :env => {
+                  "HOME"     => "/Users/morty",
+                  "LC_ALL"   => Chef::Config[:internal_locale],
+                  "LANG"     => Chef::Config[:internal_locale],
+                  "LANGUAGE" => Chef::Config[:internal_locale],
+                  env_path   => sanitized_path,
+                },
+              }).and_return(retobj)
+              shell_out_obj.send(method, cmd, options)
+              expect(options[:env].has_key?("LC_ALL")).to be false
+            end
           end
 
-          it "should set environment language settings to the configured internal locale when they are not present" do
-            options = { :environment => { "HOME" => "/Users/morty" } }
-            expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, {
-              :environment => {
-                "HOME"     => "/Users/morty",
-                "LC_ALL"   => Chef::Config[:internal_locale],
-                "LANG"     => Chef::Config[:internal_locale],
-                "LANGUAGE" => Chef::Config[:internal_locale],
-                env_path   => sanitized_path,
-              },
-            }).and_return(true)
-            shell_out_obj.shell_out(cmd, options)
-          end
-
-          it "should not mutate the options hash when it adds language settings" do
-            options = { :environment => { "HOME" => "/Users/morty" } }
-            expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, {
-              :environment => {
-                "HOME"     => "/Users/morty",
-                "LC_ALL"   => Chef::Config[:internal_locale],
-                "LANG"     => Chef::Config[:internal_locale],
-                "LANGUAGE" => Chef::Config[:internal_locale],
-                env_path   => sanitized_path,
-              },
-            }).and_return(true)
-            shell_out_obj.shell_out(cmd, options)
-            expect(options[:environment].has_key?("LC_ALL")).to be false
-          end
-        end
-
-        describe "and env is an option" do
-          it "should not change env when langauge options are set to nil" do
-            options = { :env => { "LC_ALL" => nil, "LANG" => nil, "LANGUAGE" => nil, env_path => nil } }
-            expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, options).and_return(true)
-            shell_out_obj.shell_out(cmd, options)
-          end
-
-          it "should not change env when language options are set to non-nil" do
-            options = { :env => { "LC_ALL" => "de_DE.UTF-8", "LANG" => "de_DE.UTF-8", "LANGUAGE" => "de_DE.UTF-8", env_path => "foo:bar:baz" } }
-            expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, options).and_return(true)
-            shell_out_obj.shell_out(cmd, options)
-          end
-
-          it "should set environment language settings to the configured internal locale when they are not present" do
-            options = { :env => { "HOME" => "/Users/morty" } }
-            expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, {
-              :env => {
-                "HOME"     => "/Users/morty",
-                "LC_ALL"   => Chef::Config[:internal_locale],
-                "LANG"     => Chef::Config[:internal_locale],
-                "LANGUAGE" => Chef::Config[:internal_locale],
-                env_path   => sanitized_path,
-              },
-            }).and_return(true)
-            shell_out_obj.shell_out(cmd, options)
-          end
-
-          it "should not mutate the options hash when it adds language settings" do
-            options = { :env => { "HOME" => "/Users/morty" } }
-            expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, {
-              :env => {
-                "HOME"     => "/Users/morty",
-                "LC_ALL"   => Chef::Config[:internal_locale],
-                "LANG"     => Chef::Config[:internal_locale],
-                "LANGUAGE" => Chef::Config[:internal_locale],
-                env_path   => sanitized_path,
-              },
-            }).and_return(true)
-            shell_out_obj.shell_out(cmd, options)
-            expect(options[:env].has_key?("LC_ALL")).to be false
-          end
-        end
-
-        describe "and no env/environment option is present" do
-          it "should set environment language settings to the configured internal locale" do
-            options = { :user => "morty" }
-            expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, {
-              :user => "morty",
-              :environment => {
-                "LC_ALL"   => Chef::Config[:internal_locale],
-                "LANG"     => Chef::Config[:internal_locale],
-                "LANGUAGE" => Chef::Config[:internal_locale],
-                env_path   => sanitized_path,
-              },
-            }).and_return(true)
-            shell_out_obj.shell_out(cmd, options)
+          describe "and no env/environment option is present" do
+            it "should set environment language settings to the configured internal locale" do
+              options = { :user => "morty" }
+              expect(Chef::Mixin::ShellOut).to receive(:shell_out_command).with(cmd, {
+                :user => "morty",
+                :environment => {
+                  "LC_ALL"   => Chef::Config[:internal_locale],
+                  "LANG"     => Chef::Config[:internal_locale],
+                  "LANGUAGE" => Chef::Config[:internal_locale],
+                  env_path   => sanitized_path,
+                },
+              }).and_return(retobj)
+              shell_out_obj.send(method, cmd, options)
+            end
           end
         end
       end
@@ -165,11 +168,10 @@ describe Chef::Mixin::ShellOut do
               "LANGUAGE" => Chef::Config[:internal_locale],
               env_path   => sanitized_path,
             },
-          }).and_return(true)
-          shell_out_obj.shell_out(cmd)
+          }).and_return(retobj)
+          shell_out_obj.send(method, cmd)
         end
       end
-
     end
 
     describe "#shell_out_with_systems_locale" do