Merge pull request #2142 from opscode/jdmundrawala/11-windows-env-fix

Jdmundrawala/11 windows env fix

6 files changed, 329 insertions, 187 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c11c111203..a7d846eb98 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,10 +1,14 @@
 # Chef Client Changelog
 
-## Unreleased: 11.16.2
+## Unreleased:
+
+* Fix bug on Windows where using the env resource on path could render the path unusable
+
+## Last Released: 11.16.2
 
 * This is a packaging-only release there are no code changes
 
-## Last Release: 11.16.0
+## Release: 11.16.0
 
 * Fix a bug in user dscl provider to enable managing password and other properties at the same time.
 * Add `dsc_script` resource to Chef for PowerShell DSC support on Windows
diff --git a/lib/chef/mixin/windows_env_helper.rb b/lib/chef/mixin/windows_env_helper.rb
new file mode 100644
index 0000000000..490b235065
--- /dev/null
+++ b/lib/chef/mixin/windows_env_helper.rb
@@ -0,0 +1,56 @@
+#
+# Author:: Adam Edwards (<adamed@opscode.com>)
+# Copyright:: Copyright (c) 2013 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 'chef/exceptions'
+require 'chef/platform/query_helpers'
+require 'chef/win32/error' if Chef::Platform.windows?
+require 'chef/win32/api/system' if Chef::Platform.windows?
+
+class Chef
+  module Mixin
+    module WindowsEnvHelper
+
+      if Chef::Platform.windows?
+        include Chef::ReservedNames::Win32::API::System
+      end
+
+      #see: http://msdn.microsoft.com/en-us/library/ms682653%28VS.85%29.aspx
+      HWND_BROADCAST = 0xffff
+      WM_SETTINGCHANGE = 0x001A
+      SMTO_BLOCK = 0x0001
+      SMTO_ABORTIFHUNG = 0x0002
+      SMTO_NOTIMEOUTIFNOTHUNG = 0x0008
+
+      def broadcast_env_change
+        flags = SMTO_BLOCK | SMTO_ABORTIFHUNG | SMTO_NOTIMEOUTIFNOTHUNG
+        SendMessageTimeoutA(HWND_BROADCAST, WM_SETTINGCHANGE, 0, FFI::MemoryPointer.from_string('Environment').address, flags, 5000, nil)
+      end
+
+      def expand_path(path)
+        # http://msdn.microsoft.com/en-us/library/windows/desktop/ms724265%28v=vs.85%29.aspx
+        # Max size of env block on windows is 32k
+        buf = 0.chr * 32 * 1024
+        if ExpandEnvironmentStringsA(path, buf, buf.length) == 0
+          Chef::ReservedNames::Win32::Error.raise!
+        end
+        buf.strip
+      end
+    end
+  end
+end
diff --git a/lib/chef/provider/env/windows.rb b/lib/chef/provider/env/windows.rb
index f73cb42f7e..572ec5c633 100644
--- a/lib/chef/provider/env/windows.rb
+++ b/lib/chef/provider/env/windows.rb
@@ -16,13 +16,13 @@
 # limitations under the License.
 #
 
-require 'chef/win32/api/system' if RUBY_PLATFORM =~ /mswin|mingw32|windows/
+require 'chef/mixin/windows_env_helper'
 
 class Chef
   class Provider
     class Env
       class Windows < Chef::Provider::Env
-        include Chef::ReservedNames::Win32::API::System if RUBY_PLATFORM =~ /mswin|mingw32|windows/
+        include Chef::Mixin::WindowsEnvHelper
 
         def create_env
           obj = env_obj(@new_resource.key_name)
@@ -33,7 +33,9 @@ class Chef
           end
           obj.variablevalue = @new_resource.value
           obj.put_
-          ENV[@new_resource.key_name] = @new_resource.value
+          value = @new_resource.value
+          value = expand_path(value) if @new_resource.key_name.upcase == 'PATH'
+          ENV[@new_resource.key_name] = value
           broadcast_env_change
         end
 
@@ -60,17 +62,6 @@ class Chef
           end
         end
 
-        #see: http://msdn.microsoft.com/en-us/library/ms682653%28VS.85%29.aspx
-        HWND_BROADCAST = 0xffff
-        WM_SETTINGCHANGE = 0x001A
-        SMTO_BLOCK = 0x0001
-        SMTO_ABORTIFHUNG = 0x0002
-        SMTO_NOTIMEOUTIFNOTHUNG = 0x0008
-
-        def broadcast_env_change
-          flags = SMTO_BLOCK | SMTO_ABORTIFHUNG | SMTO_NOTIMEOUTIFNOTHUNG
-          SendMessageTimeoutA(HWND_BROADCAST, WM_SETTINGCHANGE, 0, FFI::MemoryPointer.from_string('Environment').address, flags, 5000, nil)
-        end
       end
     end
   end
diff --git a/lib/chef/win32/api/system.rb b/lib/chef/win32/api/system.rb
index a58c0f38f4..d57699acb4 100644
--- a/lib/chef/win32/api/system.rb
+++ b/lib/chef/win32/api/system.rb
@@ -200,6 +200,15 @@ LRESULT WINAPI SendMessageTimeout(
         safe_attach_function :SendMessageTimeoutW, [:HWND, :UINT, :WPARAM, :LPARAM, :UINT, :UINT, :PDWORD_PTR], :LRESULT
         safe_attach_function :SendMessageTimeoutA, [:HWND, :UINT, :WPARAM, :LPARAM, :UINT, :UINT, :PDWORD_PTR], :LRESULT
 
+=begin
+DWORD WINAPI ExpandEnvironmentStrings(
+  _In_       LPCTSTR lpSrc,
+  _Out_opt_  LPTSTR lpDst,
+  _In_       DWORD nSize
+);
+=end
+        safe_attach_function :ExpandEnvironmentStringsW, [:pointer, :pointer, :DWORD], :DWORD
+        safe_attach_function :ExpandEnvironmentStringsA, [:pointer, :pointer, :DWORD], :DWORD
       end
     end
   end
diff --git a/spec/functional/resource/env_spec.rb b/spec/functional/resource/env_spec.rb
index 902c624760..24fe5e1dff 100755
--- a/spec/functional/resource/env_spec.rb
+++ b/spec/functional/resource/env_spec.rb
@@ -1,137 +1,183 @@
-#

-# Author:: Adam Edwards (<adamed@getchef.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'

-

-describe Chef::Resource::Env, :windows_only do

-  context 'when running on Windows' do

-    let(:chef_env_test_lower_case) { 'chefenvtest' }

-    let(:chef_env_test_mixed_case) { 'chefENVtest' }

-    let(:env_value1) { 'value1' }

-    let(:env_value2) { 'value2' }

-    let(:test_run_context) {

-      node = Chef::Node.new

-      node.default['platform'] = 'windows'

-      node.default['platform_version'] = '6.1'

-      empty_events = Chef::EventDispatch::Dispatcher.new

-      Chef::RunContext.new(node, {}, empty_events)

-    }

-    let(:test_resource) { 

-      Chef::Resource::Env.new('unknown', test_run_context) 

-    } 

-

-    before(:each) do

-      resource_lower = Chef::Resource::Env.new(chef_env_test_lower_case, test_run_context)

-      resource_lower.run_action(:delete)

-      resource_mixed = Chef::Resource::Env.new(chef_env_test_mixed_case, test_run_context)

-      resource_mixed.run_action(:delete)

-    end

-

-    context "when the create action is invoked" do

-      it 'should create an environment variable for action create' do

-        expect(ENV[chef_env_test_lower_case]).to eq(nil)

-        test_resource.key_name(chef_env_test_lower_case)

-        test_resource.value(env_value1)

-        test_resource.run_action(:create)

-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)

-      end

-

-      it "should modify an existing variable's value to a new value and preserve the original name and preseve the original variable's name" do

-        test_resource.key_name(chef_env_test_lower_case)

-        test_resource.value(env_value1)

-        test_resource.run_action(:create)

-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)

-        test_resource.value(env_value2)

-        test_resource.run_action(:create)

-        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)

-      end

-

-      it "should modify an existing variable's value to a new value if the variable name case differs from the existing variable" do

-        test_resource.key_name(chef_env_test_lower_case)

-        test_resource.value(env_value1)

-        test_resource.run_action(:create)

-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)

-        test_resource.key_name(chef_env_test_mixed_case)

-        test_resource.value(env_value2)

-        test_resource.run_action(:create)

-        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)

-      end

-    end

-

-    context "when the modify action is invoked" do

-      it "should raise an exception for modify if the variable doesn't exist" do

-        expect(ENV[chef_env_test_lower_case]).to eq(nil)

-        test_resource.key_name(chef_env_test_lower_case)

-        test_resource.value(env_value1)

-        expect {test_resource.run_action(:modify) }.to raise_error(Chef::Exceptions::Env)

-      end

-

-      it "should modify an existing variable's value to a new value" do

-        test_resource.key_name(chef_env_test_lower_case)

-        test_resource.value(env_value1)

-        test_resource.run_action(:create)

-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)

-        test_resource.value(env_value2)

-        test_resource.run_action(:modify)

-        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)

-      end

-

-      # This examlpe covers Chef Issue #1754

-      it "should modify an existing variable's value to a new value if the variable name case differs from the existing variable" do

-        test_resource.key_name(chef_env_test_lower_case)

-        test_resource.value(env_value1)

-        test_resource.run_action(:create)

-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)

-        test_resource.key_name(chef_env_test_mixed_case)

-        test_resource.value(env_value2)

-        test_resource.run_action(:modify)

-        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)

-      end

-    end

-

-    context "when the delete action is invoked" do

-      it "should delete an environment variable" do

-        test_resource.key_name(chef_env_test_lower_case)

-        test_resource.value(env_value1)

-        test_resource.run_action(:create)

-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)

-        test_resource.run_action(:delete)

-        expect(ENV[chef_env_test_lower_case]).to eq(nil)

-      end

-

-      it "should not raise an exception when a non-existent environment variable is deleted" do

-        expect(ENV[chef_env_test_lower_case]).to eq(nil)

-        test_resource.key_name(chef_env_test_lower_case)

-        test_resource.value(env_value1)

-        expect{test_resource.run_action(:delete)}.not_to raise_error

-        expect(ENV[chef_env_test_lower_case]).to eq(nil)

-      end

-

-      it "should delete an existing variable's value to a new value if the specified variable name case differs from the existing variable" do

-        test_resource.key_name(chef_env_test_lower_case)

-        test_resource.value(env_value1)

-        test_resource.run_action(:create)

-        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)

-        test_resource.key_name(chef_env_test_mixed_case)

-        test_resource.run_action(:delete)

-        expect(ENV[chef_env_test_lower_case]).to eq(nil)

-        expect(ENV[chef_env_test_mixed_case]).to eq(nil)

-      end

-    end

-  end

-end

+#
+# Author:: Adam Edwards (<adamed@getchef.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'
+
+describe Chef::Resource::Env, :windows_only do
+  context 'when running on Windows' do
+    let(:chef_env_test_lower_case) { 'chefenvtest' }
+    let(:chef_env_test_mixed_case) { 'chefENVtest' }
+    let(:env_value1) { 'value1' }
+    let(:env_value2) { 'value2' }
+
+    let(:env_value_expandable) { '%SystemRoot%' }
+    let(:test_run_context) {
+      node = Chef::Node.new
+      node.default['platform'] = 'windows'
+      node.default['platform_version'] = '6.1'
+      empty_events = Chef::EventDispatch::Dispatcher.new
+      Chef::RunContext.new(node, {}, empty_events)
+    }
+    let(:test_resource) { 
+      Chef::Resource::Env.new('unknown', test_run_context) 
+    } 
+
+    before(:each) do
+      resource_lower = Chef::Resource::Env.new(chef_env_test_lower_case, test_run_context)
+      resource_lower.run_action(:delete)
+      resource_mixed = Chef::Resource::Env.new(chef_env_test_mixed_case, test_run_context)
+      resource_mixed.run_action(:delete)
+    end
+
+    context "when the create action is invoked" do
+      it 'should create an environment variable for action create' do
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+      end
+
+      it "should modify an existing variable's value to a new value" do
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+        test_resource.value(env_value2)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
+      end
+
+      it "should modify an existing variable's value to a new value if the variable name case differs from the existing variable" do
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+        test_resource.key_name(chef_env_test_mixed_case)
+        test_resource.value(env_value2)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
+      end
+
+      it 'should not expand environment variables if the variable is not PATH' do
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value_expandable)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value_expandable)
+      end
+    end
+
+    context "when the modify action is invoked" do
+      it "should raise an exception for modify if the variable doesn't exist" do
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        expect {test_resource.run_action(:modify) }.to raise_error(Chef::Exceptions::Env)
+      end
+
+      it "should modify an existing variable's value to a new value" do
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+        test_resource.value(env_value2)
+        test_resource.run_action(:modify)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
+      end
+
+      # This examlpe covers Chef Issue #1754
+      it "should modify an existing variable's value to a new value if the variable name case differs from the existing variable" do
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+        test_resource.key_name(chef_env_test_mixed_case)
+        test_resource.value(env_value2)
+        test_resource.run_action(:modify)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value2)
+      end
+
+      it 'should not expand environment variables if the variable is not PATH' do
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+        test_resource.value(env_value_expandable)
+        test_resource.run_action(:modify)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value_expandable)
+      end
+
+      context 'when using PATH' do
+        let(:random_name) { Time.now.to_i }
+        let(:env_val) { "#{env_value_expandable}_#{random_name}"}
+        let(:path_before) { test_resource.provider_for_action(test_resource.action).env_value('PATH') }
+
+        it 'should expand PATH' do
+          path_before.should_not include(env_val)
+          test_resource.key_name('PATH')
+          test_resource.value("#{path_before};#{env_val}")
+          test_resource.run_action(:create)
+          ENV['PATH'].should_not include(env_val)
+          ENV['PATH'].should include("#{random_name}")
+        end
+
+        after(:each) do
+          # cleanup so we don't flood the path
+          test_resource.key_name('PATH')
+          test_resource.value(path_before)
+          test_resource.run_action(:create)
+          if test_resource.provider_for_action(test_resource.action).env_value('PATH') != path_before
+            raise 'Failed to cleanup after ourselves'
+          end
+        end
+      end
+
+    end
+
+    context "when the delete action is invoked" do
+      it "should delete an environment variable" do
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+        test_resource.run_action(:delete)
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+      end
+
+      it "should not raise an exception when a non-existent environment variable is deleted" do
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        expect{test_resource.run_action(:delete)}.not_to raise_error
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+      end
+
+      it "should delete an existing variable's value to a new value if the specified variable name case differs from the existing variable" do
+        test_resource.key_name(chef_env_test_lower_case)
+        test_resource.value(env_value1)
+        test_resource.run_action(:create)
+        expect(ENV[chef_env_test_lower_case]).to eq(env_value1)
+        test_resource.key_name(chef_env_test_mixed_case)
+        test_resource.run_action(:delete)
+        expect(ENV[chef_env_test_lower_case]).to eq(nil)
+        expect(ENV[chef_env_test_mixed_case]).to eq(nil)
+      end
+    end
+  end
+end
diff --git a/spec/unit/provider/env/windows_spec.rb b/spec/unit/provider/env/windows_spec.rb
index 329448ac05..2ea137c1d9 100644
--- a/spec/unit/provider/env/windows_spec.rb
+++ b/spec/unit/provider/env/windows_spec.rb
@@ -19,49 +19,85 @@
 require 'spec_helper'
 
 describe Chef::Provider::Env::Windows, :windows_only do
-  before do
-    @node = Chef::Node.new
-    @events = Chef::EventDispatch::Dispatcher.new
-    @run_context = Chef::RunContext.new(@node, {}, @events)
-    @new_resource = Chef::Resource::Env.new("CHEF_WINDOWS_ENV_TEST")
-    @new_resource.value("foo")
-    @provider = Chef::Provider::Env::Windows.new(@new_resource, @run_context)
-    @provider.stub(:env_obj).and_return(double('null object').as_null_object)
-  end
+  let(:node) { Chef::Node.new }
+  let(:events) {Chef::EventDispatch::Dispatcher.new }
+  let(:run_context) { Chef::RunContext.new(node, {}, events) }
 
-  describe "action_create" do
-    before do
-      ENV.delete('CHEF_WINDOWS_ENV_TEST')
-      @provider.key_exists = false
-    end
+  context 'when environment variable is not PATH' do
+    let(:new_resource) {
+      new_resource = Chef::Resource::Env.new("CHEF_WINDOWS_ENV_TEST")
+      new_resource.value("foo")
+      new_resource
+    }
+    let(:provider) { 
+      provider = Chef::Provider::Env::Windows.new(new_resource, run_context) 
+      provider.stub(:env_obj).and_return(double('null object').as_null_object)
+      provider
+    }
 
-    it "should update the ruby ENV object when it creates the key" do
-      @provider.action_create
-      expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql('foo')
-    end
-  end
+    describe "action_create" do
+      before do
+        ENV.delete('CHEF_WINDOWS_ENV_TEST')
+        provider.key_exists = false
+      end
 
-  describe "action_modify" do
-    before do
-      ENV['CHEF_WINDOWS_ENV_TEST'] = 'foo'
+      it "should update the ruby ENV object when it creates the key" do
+        provider.action_create
+        expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql('foo')
+      end
     end
 
-    it "should update the ruby ENV object when it updates the value" do
-      @provider.should_receive(:compare_value).and_return(true)
-      @new_resource.value("foobar")
-      @provider.action_modify
-      expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql('foobar')
+    describe "action_modify" do
+      before do
+        ENV['CHEF_WINDOWS_ENV_TEST'] = 'foo'
+      end
+
+      it "should update the ruby ENV object when it updates the value" do
+        provider.should_receive(:compare_value).and_return(true)
+        new_resource.value("foobar")
+        provider.action_modify
+        expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql('foobar')
+      end
+
+      describe "action_delete" do
+        before do
+          ENV['CHEF_WINDOWS_ENV_TEST'] = 'foo'
+        end
+
+        it "should update the ruby ENV object when it deletes the key" do
+          provider.action_delete
+          expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql(nil)
+        end
+      end
     end
   end
 
-  describe "action_delete" do
-    before do
-      ENV['CHEF_WINDOWS_ENV_TEST'] = 'foo'
-    end
+  context 'when environment is PATH' do
+    describe "for PATH" do
+      let(:system_root) {'%SystemRoot%'}
+      let(:system_root_value) { 'D:\Windows' }
+      let(:new_resource) {
+        new_resource = Chef::Resource::Env.new('PATH')
+        new_resource.value(system_root)
+        new_resource
+      }
+      let(:provider) { 
+        provider = Chef::Provider::Env::Windows.new(new_resource, run_context) 
+        provider.stub(:env_obj).and_return(double('null object').as_null_object)
+        provider
+      }
 
-    it "should update the ruby ENV object when it deletes the key" do
-      @provider.action_delete
-      expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql(nil)
+      before do
+        stub_const('ENV', {'PATH' => ''})
+      end
+
+      it "replaces Windows system variables" do
+        provider.should_receive(:compare_value).and_return(true)
+        provider.should_receive(:expand_path).with(system_root).and_return(system_root_value)
+        provider.action_modify
+        expect(ENV['PATH']).to eql(system_root_value)
+      end
     end
+
   end
 end