Added new specs and updated code to handle more scenarios

Signed-off-by: Vasu1105 <vasundhara.jagdale@msystechnologies.com>

4 files changed, 126 insertions, 15 deletions

diff --git a/lib/chef/provider/windows_task.rb b/lib/chef/provider/windows_task.rb
index dca0156d9c..d2d7397352 100644
--- a/lib/chef/provider/windows_task.rb
+++ b/lib/chef/provider/windows_task.rb
@@ -229,8 +229,7 @@ class Chef
             current_resource.frequency_modifier != new_resource.frequency_modifier ||
             current_resource.frequency != new_resource.frequency ||
             current_resource.idle_time != new_resource.idle_time ||
-            random_delay_updated? ||
-            execution_time_limit_updated? ||
+            random_delay_updated? || execution_time_limit_updated? ||
             (new_resource.start_day && new_resource.start_day != "N/A" && start_day_updated?) ||
             (new_resource.start_time && new_resource.start_time != "N/A" && start_time_updated?)
         begin
@@ -243,18 +242,35 @@ class Chef
         false
       end
 
+      # Comparing random_delay values using ISO8601::Duration object Ref: https://github.com/arnau/ISO8601/blob/master/lib/iso8601/duration.rb#L18-L23
+      # di = ISO8601::Duration.new(65707200)
+      # ds = ISO8601::Duration.new('P65707200S')
+      # dp = ISO8601::Duration.new('P2Y1MT2H')
+      # di == dp # => true
+      # di == ds # => true
       def random_delay_updated?
-        return false if current_resource.random_delay.nil? && new_resource.random_delay.nil?
-        return true if new_resource.random_delay.nil?
-        current_resource.random_delay = 0 if current_resource.random_delay.nil?
-        ISO8601::Duration.new(current_resource.random_delay) != ISO8601::Duration.new(new_resource.random_delay)
+        if new_resource.random_delay.nil?
+          false
+        elsif current_resource.random_delay.nil? &&  new_resource.random_delay == 'PT0S' # when user sets random_dealy to 0 sec
+          false
+        elsif current_resource.random_delay.nil?  && new_resource.random_delay != nil
+          true
+        else
+          ISO8601::Duration.new(current_resource.random_delay) != ISO8601::Duration.new(new_resource.random_delay)
+        end
       end
 
+      # Comparing execution_time_limit values using Ref: https://github.com/arnau/ISO8601/blob/master/lib/iso8601/duration.rb#L18-L23
       def execution_time_limit_updated?
-        return false if current_resource.execution_time_limit.nil? && new_resource.execution_time_limit.nil?
-        return true if new_resource.execution_time_limit.nil?
-        current_resource.execution_time_limit = 0 if current_resource.execution_time_limit.nil?
-        ISO8601::Duration.new(current_resource.execution_time_limit) != ISO8601::Duration.new(new_resource.execution_time_limit)
+        if new_resource.execution_time_limit.nil?
+          false
+        elsif current_resource.execution_time_limit.nil? &&  new_resource.execution_time_limit == 'PT0S' # when user sets random_dealy to 0 sec
+          false
+        elsif current_resource.execution_time_limit.nil?  && new_resource.execution_time_limit != nil
+          true
+        else
+          ISO8601::Duration.new(current_resource.execution_time_limit) != ISO8601::Duration.new(new_resource.execution_time_limit)
+        end
       end
 
       def start_day_updated?
diff --git a/lib/chef/resource/windows_task.rb b/lib/chef/resource/windows_task.rb
index 5ab5715ccf..6784f71f1d 100644
--- a/lib/chef/resource/windows_task.rb
+++ b/lib/chef/resource/windows_task.rb
@@ -205,11 +205,12 @@ class Chef
         end
       end
 
-      # Convert the number of seconds to an ISO8601 duration format
-      # @see http://tools.ietf.org/html/rfc2445#section-4.3.6
-      # @param [Integer] seconds The amount of seconds for this duration
+      # Converts the number of seconds to an ISO8601 duration format
+      # Ref : https://github.com/arnau/ISO8601/blob/master/lib/iso8601/duration.rb#L18-L23
+      # dur = ISO8601::Duration.new(65707200)
+      # dur => 'P65707200S'
       def sec_to_dur(seconds)
-        dur = seconds.to_i == 0 ? nil : ISO8601::Duration.new(seconds.to_i).to_s
+        dur = ISO8601::Duration.new(seconds.to_i).to_s
       end
 
     end
diff --git a/spec/unit/provider/windows_task_spec.rb b/spec/unit/provider/windows_task_spec.rb
index ce0fe9e50e..f06d847b17 100644
--- a/spec/unit/provider/windows_task_spec.rb
+++ b/spec/unit/provider/windows_task_spec.rb
@@ -620,4 +620,98 @@ describe Chef::Provider::WindowsTask do
       expect(provider.send(:frequency_modifier_allowed)).to be(false)
     end
   end
+
+  # In windows_task resource sec_to_dur method converts seconds to duration in format 60 == 'PT60S'
+  # random_delay_updated? method use the value return by sec_to_dur as input for comparison for new_resource.random_delay mocking the same here
+  describe "#random_delay_updated?" do
+    before do
+      new_resource.command "chef-client"
+      new_resource.run_level :highest
+      new_resource.frequency :minute
+      new_resource.frequency_modifier 15
+      new_resource.user "SYSTEM"
+    end
+
+    it "returns false if current_resource.random_delay = nil & random_delay is set to '0' seconds" do
+      task_hash[:random_delay] = nil
+      allow(provider).to receive(:load_task_hash).and_return(task_hash)
+      provider.load_current_resource
+      new_resource.random_delay = 'PT0S'
+      expect(provider.send(:random_delay_updated?)).to be(false)
+    end
+
+    it "returns false if current_resource.random_delay = 'P7D' & random_delay is set to '604800' seconds " do
+      task_hash[:random_delay] = 'P7D'
+      allow(provider).to receive(:load_task_hash).and_return(task_hash)
+      provider.load_current_resource
+      new_resource.random_delay = 'PT604800S'
+      expect(provider.send(:random_delay_updated?)).to be(false)
+    end
+
+    it "returns false if current_resource.random_delay = 'P7DT1S' & random_delay is set to '604801' seconds" do
+      task_hash[:random_delay] = 'P7DT1S'
+      allow(provider).to receive(:load_task_hash).and_return(task_hash)
+      provider.load_current_resource
+      new_resource.random_delay = 'PT604801S'
+      expect(provider.send(:random_delay_updated?)).to be(false)
+    end
+
+    it "returns true if current_resource.random_delay = 'PT1S' & random_delay is set to '3600' seconds" do
+      task_hash[:random_delay] = 'PT1S'
+      allow(provider).to receive(:load_task_hash).and_return(task_hash)
+      provider.load_current_resource
+      new_resource.random_delay = 'PT3600S'
+      expect(provider.send(:random_delay_updated?)).to be(true)
+    end
+
+    it "returns false if current_resource.random_delay = 'P2Y1MT2H' & random_delay is set to '65707200' seconds" do
+      task_hash[:random_delay] = 'P2Y1MT2H'
+      allow(provider).to receive(:load_task_hash).and_return(task_hash)
+      provider.load_current_resource
+      new_resource.random_delay = 'PT65707200S'
+      expect(provider.send(:random_delay_updated?)).to be(false)
+    end
+  end
+
+  describe "#execution_time_limit_updated?" do
+    before do
+      new_resource.command "chef-client"
+      new_resource.run_level :highest
+      new_resource.frequency :minute
+      new_resource.frequency_modifier 15
+      new_resource.user "SYSTEM"
+    end
+
+    it "returns false if current_resource.execution_time_limit = 'P7D' & execution_time_limit is set to 604800 seconds " do
+      task_hash[:execution_time_limit] = 'P7D'
+      allow(provider).to receive(:load_task_hash).and_return(task_hash)
+      provider.load_current_resource
+      new_resource.execution_time_limit = 'PT604800S'
+      expect(provider.send(:execution_time_limit_updated?)).to be(false)
+    end
+
+    it "returns false if current_resource.execution_time_limit = 'P7DT1S' & execution_time_limit is set to 604801 seconds" do
+      task_hash[:execution_time_limit] = 'P7DT1S'
+      allow(provider).to receive(:load_task_hash).and_return(task_hash)
+      provider.load_current_resource
+      new_resource.execution_time_limit = 'PT604801S'
+      expect(provider.send(:execution_time_limit_updated?)).to be(false)
+    end
+
+    it "returns true if current_resource.execution_time_limit = 'PT1S' & execution_time_limit is set to '3600' seconds" do
+      task_hash[:execution_time_limit] = 'PT1S'
+      allow(provider).to receive(:load_task_hash).and_return(task_hash)
+      provider.load_current_resource
+      new_resource.execution_time_limit = 'PT3600S'
+      expect(provider.send(:execution_time_limit_updated?)).to be(true)
+    end
+
+    it "returns false if current_resource.execution_time_limit = 'P2Y1MT2H' & execution_time_limit is set to '65707200' seconds" do
+      task_hash[:execution_time_limit] = 'P2Y1MT2H'
+      allow(provider).to receive(:load_task_hash).and_return(task_hash)
+      provider.load_current_resource
+      new_resource.execution_time_limit = 'PT65707200S'
+      expect(provider.send(:execution_time_limit_updated?)).to be(false)
+    end
+  end
 end
diff --git a/spec/unit/resource/windows_task_spec.rb b/spec/unit/resource/windows_task_spec.rb
index a40e143a49..f9349ad9bd 100644
--- a/spec/unit/resource/windows_task_spec.rb
+++ b/spec/unit/resource/windows_task_spec.rb
@@ -282,7 +282,7 @@ describe Chef::Resource::WindowsTask do
 
   context "#sec_to_dur" do
     it "return nil when passed 0" do
-      expect(resource.send(:sec_to_dur, 0)).to eql(nil)
+      expect(resource.send(:sec_to_dur, 0)).to eql('PT0S')
     end
     it "return PT1S when passed 1" do
       expect(resource.send(:sec_to_dur, 1)).to eql("PT1S")