fix comverge_if_changed to compare default valueslcg/converge-if-changed

Fixes #6300.

This may be a breaking change.

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

2 files changed, 33 insertions, 27 deletions

diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb
index fb5697fd0c..b028f3f276 100644
--- a/lib/chef/provider.rb
+++ b/lib/chef/provider.rb
@@ -272,11 +272,17 @@ class Chef
         raise ArgumentError, "converge_if_changed must be passed a block!"
       end
 
-      properties = new_resource.class.state_properties.map(&:name) if properties.empty?
-      properties = properties.map(&:to_sym)
+      properties =
+        if properties.empty?
+          new_resource.class.state_properties
+        else
+          properties.map { |property| new_resource.class.properties[property] }
+        end
+
       if current_resource
         # Collect the list of modified properties
-        specified_properties = properties.select { |property| new_resource.property_is_set?(property) }
+        specified_properties = properties.select { |property| property.is_set?(new_resource) || property.has_default? }
+        specified_properties = specified_properties.map(&:name).map(&:to_sym)
         modified = specified_properties.select { |p| new_resource.send(p) != current_resource.send(p) }
         if modified.empty?
           properties_str = if new_resource.sensitive
@@ -309,15 +315,15 @@ class Chef
       else
         # The resource doesn't exist. Mark that we are *creating* this, and
         # write down any properties we are setting.
-        property_size = properties.map(&:size).max
+        property_size = properties.map(&:name).map(&:to_sym).map(&:size).max
         created = properties.map do |property|
-          default = " (default value)" unless new_resource.property_is_set?(property)
-          properties_str = if new_resource.sensitive || new_resource.class.properties[property].sensitive?
+          default = " (default value)" unless property.is_set?(new_resource)
+          properties_str = if new_resource.sensitive || property.sensitive?
                              "(suppressed sensitive property)"
                            else
-                             new_resource.send(property).inspect
+                             new_resource.send(property.name.to_sym).inspect
                            end
-          "  set #{property.to_s.ljust(property_size)} to #{properties_str}#{default}"
+          "  set #{property.name.to_sym.to_s.ljust(property_size)} to #{properties_str}#{default}"
         end
 
         converge_by([ "create #{new_resource.identity}" ] + created, &converge_block)
diff --git a/spec/integration/recipes/resource_converge_if_changed_spec.rb b/spec/integration/recipes/resource_converge_if_changed_spec.rb
index 605cb2c941..97314646d9 100644
--- a/spec/integration/recipes/resource_converge_if_changed_spec.rb
+++ b/spec/integration/recipes/resource_converge_if_changed_spec.rb
@@ -53,9 +53,9 @@ describe "Resource::ActionClass#converge_if_changed" do
       context "and current_resource with state1=current, state2=current" do
         before :each do
           resource_class.load_current_value do
-            state1 "current_state1"
-            state2 "current_state2"
-            sensitive1 "current_dontprintme"
+            state1 "default_state1"
+            state2 "default_state2"
+            sensitive1 "default_dontprintme"
           end
         end
 
@@ -86,7 +86,7 @@ describe "Resource::ActionClass#converge_if_changed" do
             expect(converged_recipe.stdout).to eq <<~EOM
               * #{resource_name}[blah] action create
                 - update default_identity1
-                -   set state1 to "new_state1" (was "current_state1")
+                -   set state1 to "new_state1" (was "default_state1")
             EOM
           end
         end
@@ -107,8 +107,8 @@ describe "Resource::ActionClass#converge_if_changed" do
             expect(converged_recipe.stdout).to eq <<~EOM
               * #{resource_name}[blah] action create
                 - update default_identity1
-                -   set state1 to "new_state1" (was "current_state1")
-                -   set state2 to "new_state2" (was "current_state2")
+                -   set state1 to "new_state1" (was "default_state1")
+                -   set state2 to "new_state2" (was "default_state2")
             EOM
           end
         end
@@ -160,7 +160,7 @@ describe "Resource::ActionClass#converge_if_changed" do
           let(:converge_recipe) do
             <<-EOM
               #{resource_name} 'blah' do
-                state1 'current_state1'
+                state1 'default_state1'
                 state2 'new_state2'
               end
             EOM
@@ -172,7 +172,7 @@ describe "Resource::ActionClass#converge_if_changed" do
             expect(converged_recipe.stdout).to eq <<~EOM
               * #{resource_name}[blah] action create
                 - update default_identity1
-                -   set state2 to "new_state2" (was "current_state2")
+                -   set state2 to "new_state2" (was "default_state2")
             EOM
           end
         end
@@ -181,8 +181,8 @@ describe "Resource::ActionClass#converge_if_changed" do
           let(:converge_recipe) do
             <<-EOM
               #{resource_name} 'blah' do
-                state1 'current_state1'
-                state2 'current_state2'
+                state1 'default_state1'
+                state2 'default_state2'
               end
             EOM
           end
@@ -344,8 +344,8 @@ describe "Resource::ActionClass#converge_if_changed" do
       context "and current_resource with state1=current, state2=current" do
         before :each do
           resource_class.load_current_value do
-            state1 "current_state1"
-            state2 "current_state2"
+            state1 "default_state1"
+            state2 "default_state2"
           end
         end
 
@@ -377,7 +377,7 @@ describe "Resource::ActionClass#converge_if_changed" do
             expect(converged_recipe.stdout).to eq <<~EOM
               * #{resource_name}[blah] action create
                 - update default_identity1
-                -   set state1 to "new_state1" (was "current_state1")
+                -   set state1 to "new_state1" (was "default_state1")
             EOM
           end
         end
@@ -398,9 +398,9 @@ describe "Resource::ActionClass#converge_if_changed" do
             expect(converged_recipe.stdout).to eq <<~EOM
               * #{resource_name}[blah] action create
                 - update default_identity1
-                -   set state1 to "new_state1" (was "current_state1")
+                -   set state1 to "new_state1" (was "default_state1")
                 - update default_identity1
-                -   set state2 to "new_state2" (was "current_state2")
+                -   set state2 to "new_state2" (was "default_state2")
             EOM
           end
         end
@@ -409,7 +409,7 @@ describe "Resource::ActionClass#converge_if_changed" do
           let(:converge_recipe) do
             <<-EOM
               #{resource_name} 'blah' do
-                state1 'current_state1'
+                state1 'default_state1'
                 state2 'new_state2'
               end
             EOM
@@ -421,7 +421,7 @@ describe "Resource::ActionClass#converge_if_changed" do
             expect(converged_recipe.stdout).to eq <<~EOM
               * #{resource_name}[blah] action create
                 - update default_identity1
-                -   set state2 to "new_state2" (was "current_state2")
+                -   set state2 to "new_state2" (was "default_state2")
             EOM
           end
         end
@@ -430,8 +430,8 @@ describe "Resource::ActionClass#converge_if_changed" do
           let(:converge_recipe) do
             <<-EOM
               #{resource_name} 'blah' do
-                state1 'current_state1'
-                state2 'current_state2'
+                state1 'default_state1'
+                state2 'default_state2'
               end
             EOM
           end