diff options
author | algaut <algaut35@gmail.com> | 2018-01-29 17:10:33 +0100 |
---|---|---|
committer | Tim Smith <tsmith@chef.io> | 2018-02-14 14:16:34 -0800 |
commit | 9593bad589d79289a80c3cd27393443f0a0b46ec (patch) | |
tree | ff328fe21f3acec501451d71ff1b79f90c12a8ad | |
parent | 87f0be7df49489afcdd080aa50b9fb27a85f2b8c (diff) | |
download | chef-task_chef13.tar.gz |
Fix windows_task idle_time validationtask_chef13
Two improvements:
- Compare frequency to :idle_time instead of comparing to an incomplete list of other frequencies
- Keep all the validation logic in the function instead of splitting it between the function and where the function is called
Added some specs tests too
Signed-off-by: Alan Gauthier <algaut35@gmail.com>
-rw-r--r-- | lib/chef/resource/windows_task.rb | 8 | ||||
-rw-r--r-- | spec/unit/resource/windows_task_spec.rb | 10 |
2 files changed, 13 insertions, 5 deletions
diff --git a/lib/chef/resource/windows_task.rb b/lib/chef/resource/windows_task.rb index 10e8400c34..1b2c331e36 100644 --- a/lib/chef/resource/windows_task.rb +++ b/lib/chef/resource/windows_task.rb @@ -82,7 +82,7 @@ class Chef validate_create_frequency_modifier(frequency, frequency_modifier) validate_create_day(day, frequency) if day validate_create_months(months, frequency) if months - validate_idle_time(idle_time, frequency) if ( !idle_time.nil? && ([:minute, :hourly, :daily, :weekly, :monthly].include? frequency)) || (idle_time.nil? || !(idle_time > 0 && idle_time <= 999)) && !([:minute, :hourly, :daily, :weekly, :monthly].include? frequency) + validate_idle_time(idle_time, frequency) end private @@ -196,13 +196,13 @@ class Chef end def validate_idle_time(idle_time, frequency) - unless [:on_idle].include?(frequency) + if !idle_time.nil? && frequency != :on_idle raise ArgumentError, "idle_time property is only valid for tasks that run on_idle" end - if idle_time.nil? + if idle_time.nil? && frequency == :on_idle raise ArgumentError, "idle_time value should be set for :on_idle frequency." end - unless idle_time > 0 && idle_time <= 999 + unless idle_time.nil? || idle_time > 0 && idle_time <= 999 raise ArgumentError, "idle_time value #{idle_time} is invalid. Valid values for :on_idle frequency are 1 - 999." end end diff --git a/spec/unit/resource/windows_task_spec.rb b/spec/unit/resource/windows_task_spec.rb index f570cc3192..918868c63e 100644 --- a/spec/unit/resource/windows_task_spec.rb +++ b/spec/unit/resource/windows_task_spec.rb @@ -276,7 +276,9 @@ describe Chef::Resource::WindowsTask do context "#validate_idle_time" do it "raises error if frequency is not :on_idle" do - expect { resource.send(:validate_idle_time, 5, :hourly) }.to raise_error(ArgumentError, "idle_time property is only valid for tasks that run on_idle") + [:minute, :hourly, :daily, :weekly, :monthly, :once, :on_logon, :onstart, :none].each do |frequency| + expect { resource.send(:validate_idle_time, 5, frequency) }.to raise_error(ArgumentError, "idle_time property is only valid for tasks that run on_idle") + end end it "raises error if idle_time > 999" do @@ -290,6 +292,12 @@ describe Chef::Resource::WindowsTask do it "raises error if idle_time is not set" do expect { resource.send(:validate_idle_time, nil, :on_idle) }.to raise_error(ArgumentError, "idle_time value should be set for :on_idle frequency.") end + + it "does not raises error if idle_time is not set for other frequencies" do + [:minute, :hourly, :daily, :weekly, :monthly, :once, :on_logon, :onstart, :none].each do |frequency| + expect { resource.send(:validate_idle_time, nil, frequency) }.not_to raise_error + end + end end context "#sec_to_dur" do |