Respond to review comments and chefstyletm/rescue_5115

Signed-off-by: Thom May <thom@chef.io>

3 files changed, 18 insertions, 19 deletions

diff --git a/lib/chef/provider/launchd.rb b/lib/chef/provider/launchd.rb
index c58d4bfa34..0ec8d0cfe1 100644
--- a/lib/chef/provider/launchd.rb
+++ b/lib/chef/provider/launchd.rb
@@ -115,7 +115,7 @@ class Chef
         res = Chef::Resource::File.new(@path, run_context)
         res.name(@path) if @path
         res.backup(backup) if backup
-        res.content(content) if content
+        res.content(content) if content?
         res.group(group) if group
         res.mode(mode) if mode
         res.owner(owner) if owner
diff --git a/lib/chef/resource/launchd.rb b/lib/chef/resource/launchd.rb
index 404da13665..27937515ec 100644
--- a/lib/chef/resource/launchd.rb
+++ b/lib/chef/resource/launchd.rb
@@ -29,12 +29,6 @@ class Chef
       default_action :create
       allowed_actions :create, :create_if_missing, :delete, :enable, :disable
 
-      def initialize(name, run_context = nil)
-        super
-        provider = Chef::Provider::Launchd
-        resource_name = :launchd
-      end
-
       property :label, String, default: lazy { name }, identity: true
       property :backup, [Integer, FalseClass]
       property :cookbook, String
@@ -57,11 +51,14 @@ class Chef
       # will not just run on midnight of Sat and Sun, rather it will run _every_ midnight.
       property :start_calendar_interval, [Hash, Array], coerce: proc { |type|
         # Coerce into an array of hashes to make validation easier
-        array = type   if type.is_a?(Array)
-        array = [type] unless array
+        array = if type.is_a?(Array)
+                  type
+                else
+                  [type]
+                end
 
         # Check to make sure that our array only has hashes
-        if array.any? { |obj| !obj.is_a?(Hash) }
+        unless array.all? { |obj| obj.is_a?(Hash) }
           error_msg = "start_calendar_interval must be a single hash or an array of hashes!"
           raise Chef::Exceptions::ValidationFailed, error_msg
         end
@@ -69,13 +66,15 @@ class Chef
         # Make sure the hashes don't have any incorrect keys/values
         array.each do |entry|
           allowed_keys = %w{Minute Hour Day Weekday Month}
-          error_msg = "Invalid key for start_calendar_interval, must be one of: #{allowed_keys.join(", ")}"
-          if entry.keys.any? { |key| !allowed_keys.include?(key) }
+          unless entry.keys.all? { |key| allowed_keys.include?(key) }
+            failed_keys = entry.keys.reject { |k| allowed_keys.include?(k) }.join(", ")
+            error_msg = "The following key(s): #{failed_keys} are invalid for start_calendar_interval, must be one of: #{allowed_keys.join(", ")}"
             raise Chef::Exceptions::ValidationFailed, error_msg
           end
 
-          error_msg = "Invalid value for start_calendar_interval item.  Values must be integers!"
-          if entry.values.any? { |val| !val.is_a?(Fixnum) }
+          unless entry.values.all? { |val| val.is_a?(Fixnum) }
+            failed_values = entry.values.reject { |val| val.is_a?(Fixnum) }.join(", ")
+            error_msg = "Invalid value(s) (#{failed_values}) for start_calendar_interval item.  Values must be integers!"
             raise Chef::Exceptions::ValidationFailed, error_msg
           end
         end
diff --git a/spec/unit/provider/launchd_spec.rb b/spec/unit/provider/launchd_spec.rb
index fa1323bfff..3e45433c62 100644
--- a/spec/unit/provider/launchd_spec.rb
+++ b/spec/unit/provider/launchd_spec.rb
@@ -170,17 +170,17 @@ XML
         it "should not allow invalid ShowCalendarInterval keys" do
           new_resource.program "/Library/scripts/call_mom.sh"
           new_resource.time_out 300
-          expect {
+          expect do
             new_resource.start_calendar_interval "Hourly" => 1
-          }.to raise_error(/Invalid key/)
+          end.to raise_error(/Hourly are invalid/)
         end
 
         it "should not allow non-integer values" do
           new_resource.program "/Library/scripts/call_mom.sh"
           new_resource.time_out 300
-          expect {
-            new_resource.start_calendar_interval "Hour" => "1-2"
-          }.to raise_error(/Invalid value/)
+          expect do
+            new_resource.start_calendar_interval "Weekday" => "1-2"
+          end.to raise_error(/Invalid value.*\(1-2\)/)
         end
       end