Merge pull request #1548 from opscode/mcquin/shell_out

Add shell_out_with_systems_locale to ShellOut

4 files changed, 156 insertions, 0 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0026b64cd9..779bd12db2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # Chef Client Changelog
 
 ## Unreleased:
+* Add method shell_out_with_systems_locale to ShellOut.
 
 ## Last Release: 11.14.0
 
diff --git a/lib/chef/mixin/shell_out.rb b/lib/chef/mixin/shell_out.rb
index 56b02d780f..97aa7041fd 100644
--- a/lib/chef/mixin/shell_out.rb
+++ b/lib/chef/mixin/shell_out.rb
@@ -44,6 +44,21 @@ class Chef
         cmd
       end
 
+      # environment['LC_ALL'] should be nil or what the user specified
+      def shell_out_with_systems_locale(*command_args)
+        args = command_args.dup
+        if args.last.is_a?(Hash)
+          options = args.last
+          env_key = options.has_key?(:env) ? :env : :environment
+          options[env_key] ||= {}
+          options[env_key]['LC_ALL'] ||= nil
+        else
+          args << { :environment => { 'LC_ALL' => nil } }
+        end
+
+        shell_out(*args)
+      end
+
       DEPRECATED_OPTIONS =
         [ [:command_log_level,   :log_level],
           [:command_log_prepend, :log_tag] ]
diff --git a/spec/functional/mixin/shell_out_spec.rb b/spec/functional/mixin/shell_out_spec.rb
new file mode 100644
index 0000000000..55cd3c7c61
--- /dev/null
+++ b/spec/functional/mixin/shell_out_spec.rb
@@ -0,0 +1,48 @@
+#
+# Copyright:: Copyright (c) 2014 Chef Software, 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'
+
+describe Chef::Mixin::ShellOut do
+  include Chef::Mixin::ShellOut
+
+  describe "shell_out_with_systems_locale" do
+    describe "when environment['LC_ALL'] is not set" do
+      it "should use the default shell_out setting" do
+        cmd = if windows?
+          shell_out_with_systems_locale('echo %LC_ALL%')
+        else
+          shell_out_with_systems_locale('echo $LC_ALL')
+        end
+
+        cmd.stdout.chomp.should eq ENV['LC_ALL'].to_s
+      end
+    end
+
+    describe "when environment['LC_ALL'] is set" do
+      it "should use the option's setting" do
+        cmd = if windows?
+          shell_out_with_systems_locale('echo %LC_ALL%', :environment => {'LC_ALL' => 'POSIX'})
+        else
+          shell_out_with_systems_locale('echo $LC_ALL', :environment => {'LC_ALL' => 'POSIX'})
+        end
+
+        cmd.stdout.chomp.should eq 'POSIX'
+      end
+    end
+  end
+end
diff --git a/spec/unit/mixin/shell_out_spec.rb b/spec/unit/mixin/shell_out_spec.rb
index f325f687d9..1f85ec6bf1 100644
--- a/spec/unit/mixin/shell_out_spec.rb
+++ b/spec/unit/mixin/shell_out_spec.rb
@@ -104,6 +104,98 @@ describe Chef::Mixin::ShellOut do
 
       should_emit_deprecation_warning_about :command_log_prepend, :log_tag
     end
+  end
+
+  describe "#shell_out_with_systems_locale" do
+    before(:each) do
+      @original_env = ENV.to_hash
+      ENV.clear
+    end
+
+    after(:each) do
+      ENV.clear
+      ENV.update(@original_env)
+    end
+
+    let(:shell_out) { Chef::Mixin::ShellOut }
+    let(:cmd) { "echo '#{rand(1000)}'" }
+
+    describe "when the last argument is a Hash" do
+      describe "and environment is an option" do
+        it "should not change environment['LC_ALL'] when set to nil" do
+          options = { :environment => { 'LC_ALL' => nil } }
+          shell_out.should_receive(:shell_out).with(cmd, options).and_return(true)
+          shell_out.shell_out_with_systems_locale(cmd, options)
+        end
+
+        it "should not change environment['LC_ALL'] when set to non-nil" do
+          options = { :environment => { 'LC_ALL' => 'en_US.UTF-8' } }
+          shell_out.should_receive(:shell_out).with(cmd, options).and_return(true)
+          shell_out.shell_out_with_systems_locale(cmd, options)
+        end
+
+        it "should set environment['LC_ALL'] to nil when 'LC_ALL' not present" do
+          options = { :environment => { 'HOME' => '/Users/morty' } }
+          shell_out.should_receive(:shell_out).with(
+            cmd,
+            { :environment => {
+                'HOME' => '/Users/morty',
+                'LC_ALL' => nil }
+            }
+          ).and_return(true)
+          shell_out.shell_out_with_systems_locale(cmd, options)
+        end
+      end
 
+      describe "and env is an option" do
+        it "should not change env when set to nil" do
+          options = { :env => { 'LC_ALL' => nil } }
+          shell_out.should_receive(:shell_out).with(cmd, options).and_return(true)
+          shell_out.shell_out_with_systems_locale(cmd, options)
+        end
+
+        it "should not change env when set to non-nil" do
+          options = { :env => { 'LC_ALL' => 'en_US.UTF-8'}}
+          shell_out.should_receive(:shell_out).with(cmd, options).and_return(true)
+          shell_out.shell_out_with_systems_locale(cmd, options)
+        end
+
+        it "should set env['LC_ALL'] to nil when 'LC_ALL' not present" do
+          options = { :env => { 'HOME' => '/Users/morty' } }
+          shell_out.should_receive(:shell_out).with(
+            cmd,
+            { :env => {
+                'HOME' => '/Users/morty',
+                'LC_ALL' => nil }
+            }
+          ).and_return(true)
+          shell_out.shell_out_with_systems_locale(cmd, options)
+        end
+      end
+
+      describe "and no env/environment option is present" do
+        it "should add environment option and set environment['LC_ALL'] to nil" do
+          options = { :user => 'morty' }
+          shell_out.should_receive(:shell_out).with(
+            cmd,
+            { :environment => { 'LC_ALL' => nil },
+              :user => 'morty'
+            }
+          ).and_return(true)
+          shell_out.shell_out_with_systems_locale(cmd, options)
+        end
+      end
+    end
+
+    describe "when the last argument is not a Hash" do
+      it "should add environment options and set environment['LC_ALL'] to nil" do
+        shell_out.should_receive(:shell_out).with(
+          cmd,
+          { :environment => { 'LC_ALL' => nil } }
+        ).and_return(true)
+        shell_out.shell_out_with_systems_locale(cmd)
+      end
+    end
   end
+
 end