Refactor

3 files changed, 80 insertions, 64 deletions

diff --git a/lib/chef/provider/file.rb b/lib/chef/provider/file.rb
index 07073cb72e..cb6383fc46 100644
--- a/lib/chef/provider/file.rb
+++ b/lib/chef/provider/file.rb
@@ -340,22 +340,26 @@ class Chef
         if tempfile.path.nil? || !::File.exists?(tempfile.path)
           raise "chef-client is confused, trying to deploy a file that has no path or does not exist..."
         end
+
         # the file? on the next line suppresses the case in why-run when we have a not-file here that would have otherwise been removed
         if ::File.file?(@new_resource.path) && contents_changed?
-          diff.diff(@current_resource.path, tempfile.path)
-          @new_resource.diff( diff.for_reporting ) unless file_created?
-          description = [ "update content in file #{@new_resource.path} from #{short_cksum(@current_resource.checksum)} to #{short_cksum(checksum(tempfile.path))}" ]
-          
           if @new_resource.sensitive
-            Chef::Log.info("redacted sensitive resource: #{@new_resource}")
+            @new_resource.diff('suppressed sensitive resource')
           else
-            description << diff.for_output            
+            diff.diff(@current_resource.path, tempfile.path)
+
+            @new_resource.diff( diff.for_reporting ) unless file_created?
+
+            description = [ "update content in file #{@new_resource.path} from #{short_cksum(@current_resource.checksum)} to #{short_cksum(checksum(tempfile.path))}" ]
+
+            description << diff.for_output
           end
 
           converge_by(description) do
             update_file_contents
           end
         end
+
         # unlink necessary to clean up in why-run mode
         tempfile.unlink
       end
diff --git a/spec/functional/resource/file_spec.rb b/spec/functional/resource/file_spec.rb
index 2a1f2ea1de..d6f56db3e9 100644
--- a/spec/functional/resource/file_spec.rb
+++ b/spec/functional/resource/file_spec.rb
@@ -115,18 +115,4 @@ describe Chef::Resource::File do
       end
     end
   end
-
-  describe '.sensitive' do
-    context 'sensitive' do
-      before { current_resource.sensitive(true) }
-
-      it { expect(current_resource.sensitive).to(be_true) }
-    end
-
-    context 'insensitive' do
-      before { current_resource.sensitive(false) }
-
-      it { expect(current_resource.sensitive).to(be_false) }
-    end
-  end
 end
diff --git a/spec/support/shared/functional/file_resource.rb b/spec/support/shared/functional/file_resource.rb
index 44048598c7..e9e76c95db 100644
--- a/spec/support/shared/functional/file_resource.rb
+++ b/spec/support/shared/functional/file_resource.rb
@@ -54,25 +54,62 @@ shared_examples_for "a file with the wrong content" do
     sha256_checksum(path).should == @expected_checksum
   end
 
-  include_context "diff disabled"
+  describe "when diff is disabled" do
 
-  context "when running action :create" do
-    context "with backups enabled" do
-      before do
-        resource.run_action(:create)
+    include_context "diff disabled"
+
+    context "when running action :create" do
+      context "with backups enabled" do
+        before do
+          resource.run_action(:create)
+        end
+
+        it "overwrites the file with the updated content when the :create action is run" do
+          File.stat(path).mtime.should > @expected_mtime
+          sha256_checksum(path).should_not == @expected_checksum
+        end
+
+        it "backs up the existing file" do
+          Dir.glob(backup_glob).size.should equal(1)
+        end
+
+        it "is marked as updated by last action" do
+          resource.should be_updated_by_last_action
+        end
+
+        it "should restore the security contexts on selinux", :selinux_only do
+          selinux_security_context_restored?(path).should be_true
+        end
       end
 
-      it "overwrites the file with the updated content when the :create action is run" do
-        File.stat(path).mtime.should > @expected_mtime
-        sha256_checksum(path).should_not == @expected_checksum
+      context "with backups disabled" do
+        before do
+          resource.backup(0)
+          resource.run_action(:create)
+        end
+
+        it "should not attempt to backup the existing file if :backup == 0" do
+          Dir.glob(backup_glob).size.should equal(0)
+        end
+
+        it "should restore the security contexts on selinux", :selinux_only do
+          selinux_security_context_restored?(path).should be_true
+        end
       end
+    end
 
-      it "backs up the existing file" do
-        Dir.glob(backup_glob).size.should equal(1)
+    describe "when running action :create_if_missing" do
+      before do
+        resource.run_action(:create_if_missing)
       end
 
-      it "is marked as updated by last action" do
-        resource.should be_updated_by_last_action
+      it "doesn't overwrite the file when the :create_if_missing action is run" do
+        File.stat(path).mtime.should == @expected_mtime
+        sha256_checksum(path).should == @expected_checksum
+      end
+
+      it "is not marked as updated" do
+        resource.should_not be_updated_by_last_action
       end
 
       it "should restore the security contexts on selinux", :selinux_only do
@@ -80,52 +117,41 @@ shared_examples_for "a file with the wrong content" do
       end
     end
 
-    context "with backups disabled" do
+    describe "when running action :delete" do
       before do
-        resource.backup(0)
-        resource.run_action(:create)
+        resource.run_action(:delete)
       end
 
-      it "should not attempt to backup the existing file if :backup == 0" do
-        Dir.glob(backup_glob).size.should equal(0)
+      it "deletes the file" do
+        File.should_not exist(path)
       end
 
-      it "should restore the security contexts on selinux", :selinux_only do
-        selinux_security_context_restored?(path).should be_true
+      it "is marked as updated by last action" do
+        resource.should be_updated_by_last_action
       end
     end
-  end
 
-  describe "when running action :create_if_missing" do
-    before do
-      resource.run_action(:create_if_missing)
-    end
+  end
 
-    it "doesn't overwrite the file when the :create_if_missing action is run" do
-      File.stat(path).mtime.should == @expected_mtime
-      sha256_checksum(path).should == @expected_checksum
-    end
+  context '`.diff`’s enabled' do
+    describe '.sensitive' do
+      context '`.sensitive`’s insensitive by default' do
+        it { expect(resource.sensitive).to(be_false) }
+      end
 
-    it "is not marked as updated" do
-      resource.should_not be_updated_by_last_action
-    end
+      context '`.sensitive`’s sensitive' do
+        before do
+          resource.sensitive(true)
 
-    it "should restore the security contexts on selinux", :selinux_only do
-      selinux_security_context_restored?(path).should be_true
-    end
-  end
+          resource.run_action(:create)
+        end
 
-  describe "when running action :delete" do
-    before do
-      resource.run_action(:delete)
-    end
+        it { expect(resource.sensitive).to(be_true) }
 
-    it "deletes the file" do
-      File.should_not exist(path)
-    end
+        it { expect(resource.diff).to(include('suppressed sensitive resource')) }
 
-    it "is marked as updated by last action" do
-      resource.should be_updated_by_last_action
+        # it { expect(resource.provider.converge_actions).to include('suppressed sensitive resource') }
+      end
     end
   end
 end