summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--lib/chef/exceptions.rb1
-rw-r--r--lib/chef/mixin/params_validate.rb12
-rw-r--r--lib/chef/property.rb129
-rw-r--r--lib/chef/resource/chef_gem.rb4
-rw-r--r--lib/chef/resource/file.rb2
-rw-r--r--spec/integration/client/client_spec.rb2
-rw-r--r--spec/unit/property/validation_spec.rb215
-rw-r--r--spec/unit/property_spec.rb88
-rw-r--r--spec/unit/provider/package/dpkg_spec.rb38
-rw-r--r--spec/unit/resource_reporter_spec.rb2
10 files changed, 297 insertions, 196 deletions
diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index f9a717471c..08d916064f 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -71,6 +71,7 @@ class Chef
class RoleNotFound < RuntimeError; end
class DuplicateRole < RuntimeError; end
class ValidationFailed < ArgumentError; end
+ class CannotValidateStaticallyError < ArgumentError; end
class InvalidPrivateKey < ArgumentError; end
class MissingKeyAttribute < ArgumentError; end
class KeyCommandInputError < ArgumentError; end
diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb
index 841cf9e6a2..3585def853 100644
--- a/lib/chef/mixin/params_validate.rb
+++ b/lib/chef/mixin/params_validate.rb
@@ -22,7 +22,6 @@ require "chef/delayed_evaluator"
class Chef
module Mixin
module ParamsValidate
-
# Takes a hash of options, along with a map to validate them. Returns the original
# options hash, plus any changes that might have been made (through things like setting
# default values in the validation map)
@@ -333,6 +332,7 @@ class Chef
def _pv_name_property(opts, key, is_name_property=true)
if is_name_property
if opts[key].nil?
+ raise CannotValidateStaticallyError, "name_property cannot be evaluated without a resource." if self == Chef::Mixin::ParamsValidate
opts[key] = self.instance_variable_get(:"@name")
end
end
@@ -403,6 +403,7 @@ class Chef
to_be.each do |tb|
case tb
when Proc
+ raise CannotValidateStaticallyError, "is: proc { } must be evaluated once for each resource" if self == Chef::Mixin::ParamsValidate
return true if instance_exec(value, &tb)
when Property
validate(opts, { key => tb.validation_options })
@@ -436,12 +437,21 @@ class Chef
#
def _pv_coerce(opts, key, coercer)
if opts.has_key?(key.to_s)
+ raise CannotValidateStaticallyError, "coerce must be evaluated for each resource." if self == Chef::Mixin::ParamsValidate
opts[key.to_s] = instance_exec(opts[key], &coercer)
elsif opts.has_key?(key.to_sym)
+ raise CannotValidateStaticallyError, "coerce must be evaluated for each resource." if self == Chef::Mixin::ParamsValidate
opts[key.to_sym] = instance_exec(opts[key], &coercer)
end
end
+ # We allow Chef::Mixin::ParamsValidate.validate(), but we will raise an
+ # error if you try to do anything requiring there to be an actual resource.
+ # This way, you can statically validate things if you have constant validation
+ # (which is the norm).
+ extend self
+
+
# Used by #set_or_return to avoid emitting a deprecation warning for
# "value nil" and to keep default stickiness working exactly the same
# @api private
diff --git a/lib/chef/property.rb b/lib/chef/property.rb
index 8b5c6560a9..437ee10147 100644
--- a/lib/chef/property.rb
+++ b/lib/chef/property.rb
@@ -87,16 +87,21 @@ class Chef
# is fully initialized.
#
def initialize(**options)
- options.each { |k,v| options[k.to_sym] = v if k.is_a?(String) }
+ options.each { |k,v| options[k.to_sym] = v; options.delete(k) if k.is_a?(String) }
+ @options = options
+ options[:name] = options[:name].to_sym if options[:name]
+ options[:instance_variable_name] = options[:instance_variable_name].to_sym if options[:instance_variable_name]
+
# Replace name_attribute with name_property
if options.has_key?(:name_attribute)
# If we have both name_attribute and name_property and they differ, raise an error
if options.has_key?(:name_property)
- raise ArgumentError, "Cannot specify both name_property and name_attribute together on property #{options[:name]}#{options[:declared_in] ? " of resource #{options[:declared_in].resource_name}" : ""}."
+ raise ArgumentError, "Cannot specify both name_property and name_attribute together on property #{self}."
end
# replace name_property with name_attribute in place
options = Hash[options.map { |k,v| k == :name_attribute ? [ :name_property, v ] : [ k,v ] }]
+ @options = options
end
# Only pick the first of :default, :name_property and :name_attribute if
@@ -109,17 +114,22 @@ class Chef
options.delete(:name_property)
preferred_default = :default
end
- Chef.log_deprecation("Cannot specify both default and name_property together on property #{options[:name]}#{options[:declared_in] ? " of resource #{options[:declared_in].resource_name}" : ""}. Only one (#{preferred_default}) will be obeyed. In Chef 13, this will become an error.")
+ Chef.log_deprecation("Cannot specify both default and name_property together on property #{self}. Only one (#{preferred_default}) will be obeyed. In Chef 13, this will become an error. Please remove one or the other from the property.")
end
- @options = options
-
- options[:name] = options[:name].to_sym if options[:name]
- options[:instance_variable_name] = options[:instance_variable_name].to_sym if options[:instance_variable_name]
+ # Validate the default early, so the user gets a good error message, and
+ # cache it so we don't do it again if so
+ begin
+ # If we can validate it all the way to output, do it.
+ @stored_default = input_to_stored_value(nil, default, is_default: true)
+ rescue Chef::Exceptions::CannotValidateStaticallyError
+ # If the validation is not static (i.e. has procs), we will have to
+ # coerce and validate the default each time we run
+ end
end
def to_s
- name
+ "#{name}#{declared_in ? " of resource #{declared_in.resource_name}" : ""}"
end
#
@@ -253,14 +263,26 @@ class Chef
return get(resource)
end
- if value.nil? && !explicitly_accepts_nil?(resource)
+ if value.nil?
# In Chef 12, value(nil) does a *get* instead of a set, so we
# warn if the value would have been changed. In Chef 13, it will be
# equivalent to value = nil.
result = get(resource)
- if !result.nil?
- Chef.log_deprecation("#{name} nil currently does not overwrite the value of #{name}. This will change in Chef 13, and the value will be set to nil instead. Please change your code to explicitly accept nil using \"property :#{name}, [MyType, nil]\", or stop setting this value to nil.")
+
+ # Warn about this becoming a set in Chef 13.
+ begin
+ input_to_stored_value(resource, value)
+ # If nil is valid, and it would change the value, warn that this will change to a set.
+ if !result.nil?
+ Chef.log_deprecation("An attempt was made to change #{name} from #{result.inspect} to nil by calling #{name}(nil). In Chef 12, this does a get rather than a set. In Chef 13, this will change to set the value to nil.")
+ end
+ rescue Chef::Exceptions::DeprecatedFeatureError
+ raise
+ rescue
+ # If nil is invalid, warn that this will become an error.
+ Chef.log_deprecation("nil is an invalid value for #{self}. In Chef 13, this warning will change to an error. Error: #{$!}")
end
+
result
else
# Anything else, such as myprop(value) is a set
@@ -291,16 +313,14 @@ class Chef
# this property, or if the value is required and not set.
#
def get(resource)
+ # If it's set, return it (and evaluate any lazy values)
if is_set?(resource)
value = get_value(resource)
- if value.is_a?(DelayedEvaluator)
- value = exec_in_resource(resource, value)
- value = coerce(resource, value)
- validate(resource, value)
- end
- value
+ value = stored_value_to_output(resource, value)
else
+ # We are getting the default value.
+
# If the user does something like this:
#
# ```
@@ -326,14 +346,14 @@ class Chef
end
if has_default?
- value = default
- if value.is_a?(DelayedEvaluator)
- value = exec_in_resource(resource, value)
+ # If we were able to cache the stored_default, grab it.
+ if defined?(@stored_default)
+ value = @stored_default
+ else
+ # Otherwise, we have to validate it now.
+ value = input_to_stored_value(resource, default, is_default: true)
end
-
- value = coerce(resource, value)
-
- # We don't validate defaults
+ value = stored_value_to_output(resource, value, is_default: true)
# If the value is mutable (non-frozen), we set it on the instance
# so that people can mutate it. (All constant default values are
@@ -366,11 +386,7 @@ class Chef
# this property.
#
def set(resource, value)
- unless value.is_a?(DelayedEvaluator)
- value = coerce(resource, value)
- validate(resource, value)
- end
- set_value(resource, value)
+ set_value(resource, input_to_stored_value(resource, value))
end
#
@@ -423,7 +439,10 @@ class Chef
#
def coerce(resource, value)
if options.has_key?(:coerce)
- value = exec_in_resource(resource, options[:coerce], value)
+ # If we have no default value, `nil` is never coerced or validated
+ unless !has_default? && value.nil?
+ value = exec_in_resource(resource, options[:coerce], value)
+ end
end
value
end
@@ -442,7 +461,10 @@ class Chef
# this property.
#
def validate(resource, value)
- resource.validate({ name => value }, { name => validation_options })
+ # If we have no default value, `nil` is never coerced or validated
+ unless value.nil? && !has_default?
+ (resource || Chef::Mixin::ParamsValidate).validate({ name => value }, { name => validation_options })
+ end
end
#
@@ -595,14 +617,53 @@ class Chef
value = resource.instance_exec(*args, &proc)
end
else
- value = proc.call
+ # If we don't have a resource yet, we can't exec in resource!
+ raise Chef::Exceptions::CannotValidateStaticallyError, "Cannot validate or coerce without a resource"
+ end
+ end
+
+ def input_to_stored_value(resource, value, is_default: false)
+ unless value.is_a?(DelayedEvaluator)
+ value = coerce_and_validate(resource, value, is_default: is_default)
end
+ value
+ end
+ def stored_value_to_output(resource, value, is_default: false)
+ # Crack open lazy values before giving the result to the user
if value.is_a?(DelayedEvaluator)
- value = coerce(resource, value)
- validate(resource, value)
+ value = exec_in_resource(resource, value)
+ value = coerce_and_validate(resource, value, is_default: is_default)
end
value
end
+
+ # Coerces and validates the value. If the value is a default, it will warn
+ # the user that invalid defaults are bad mmkay, and return it as if it were
+ # valid.
+ def coerce_and_validate(resource, value, is_default: false)
+ result = coerce(resource, value)
+ begin
+ # If the input is from a default, we need to emit an invalid default warning on validate.
+ validate(resource, result)
+ rescue Chef::Exceptions::CannotValidateStaticallyError
+ # This one gets re-raised
+ raise
+ rescue
+ # Anything else is just an invalid default: in those cases, we just
+ # warn and return the (possibly coerced) value to the user.
+ if is_default
+ if value.nil?
+ Chef.log_deprecation("Default value nil is invalid for property #{self}. Possible fixes: 1. Remove 'default: nil' if nil means 'undefined'. 2. Set a valid default value if there is a reasonable one. 3. Allow nil as a valid value of your property (for example, 'property #{name.inspect}, [ String, nil ], default: nil'). Error: #{$!}")
+ else
+ Chef.log_deprecation("Default value #{value.inspect} is invalid for property #{self}. In Chef 13 this will become an error: #{$!}.")
+ end
+ else
+ raise
+ end
+ end
+
+ result
+ end
end
end
diff --git a/lib/chef/resource/chef_gem.rb b/lib/chef/resource/chef_gem.rb
index 233ae66026..655534684c 100644
--- a/lib/chef/resource/chef_gem.rb
+++ b/lib/chef/resource/chef_gem.rb
@@ -26,9 +26,9 @@ class Chef
property :gem_binary, default: "#{RbConfig::CONFIG['bindir']}/gem",
callbacks: {
- "The chef_gem resource is restricted to the current gem environment, use gem_package to install to other environments." => proc { false }
+ "The chef_gem resource is restricted to the current gem environment, use gem_package to install to other environments." => proc { |v| v == properties[:gem_binary].default }
}
- property :compile_time, [ true, false ], default: lazy { Chef::Config[:chef_gem_compile_time] }, desired_state: false
+ property :compile_time, [ true, false, nil ], default: lazy { Chef::Config[:chef_gem_compile_time] }, desired_state: false
def after_created
# Chef::Resource.run_action: Caveat: this skips Chef::Runner.run_action, where notifications are handled
diff --git a/lib/chef/resource/file.rb b/lib/chef/resource/file.rb
index 0a530e27c9..52cb409226 100644
--- a/lib/chef/resource/file.rb
+++ b/lib/chef/resource/file.rb
@@ -49,7 +49,7 @@ class Chef
allowed_actions :create, :delete, :touch, :create_if_missing
property :path, String, name_property: true, identity: true
- property :atomic_update, [ true, false ], desired_state: false, default: Chef::Config[:file_atomic_update]
+ property :atomic_update, [ true, false ], desired_state: false, default: lazy { Chef::Config[:file_atomic_update] }
property :backup, [ Integer, false ], desired_state: false, default: 5
property :checksum, [ /^[a-zA-Z0-9]{64}$/, nil ]
property :content, [ String, nil ], desired_state: false
diff --git a/spec/integration/client/client_spec.rb b/spec/integration/client/client_spec.rb
index d08935d854..24ce69ac03 100644
--- a/spec/integration/client/client_spec.rb
+++ b/spec/integration/client/client_spec.rb
@@ -430,7 +430,7 @@ EOM
expect(run_complete).to be >= 0
# Make sure there is exactly one result for each, and that it occurs *after* the complete message.
- expect(match_indices(/nil currently does not overwrite the value of/, result.stdout)).to match([ be > run_complete ])
+ expect(match_indices(/An attempt was made to change x from \[\] to nil by calling x\(nil\). In Chef 12, this does a get rather than a set. In Chef 13, this will change to set the value to nil./, result.stdout)).to match([ be > run_complete ])
end
end
diff --git a/spec/unit/property/validation_spec.rb b/spec/unit/property/validation_spec.rb
index 7e84a651e2..5a40cce145 100644
--- a/spec/unit/property/validation_spec.rb
+++ b/spec/unit/property/validation_spec.rb
@@ -72,7 +72,7 @@ describe "Chef::Resource.property validation" do
end
end
- def self.validation_test(validation, success_values, failure_values, getter_values=[], *tags)
+ def self.validation_test(validation, success_values, failure_values, *tags)
with_property ":x, #{validation}", *tags do
it "gets nil when retrieving the initial (non-set) value" do
expect(resource.x).to be_nil
@@ -91,11 +91,52 @@ describe "Chef::Resource.property validation" do
expect { resource.x v }.to raise_error Chef::Exceptions::ValidationFailed
end
end
- getter_values.each do |v|
- it "setting value to #{v.inspect} does not change the value" do
+ it "setting x to nil when it is already nil does not emit a warning" do
+ expect(resource.x nil).to be_nil
+ expect(resource.x).to be_nil
+ end
+ it "changing x to nil warns that the get will change to a set in Chef 13 and does not change the value" do
+ resource.instance_eval { @x = "default" }
+ expect { resource.x nil }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
+ /An attempt was made to change x from "default" to nil by calling x\(nil\). In Chef 12, this does a get rather than a set. In Chef 13, this will change to set the value to nil./
+ Chef::Config[:treat_deprecation_warnings_as_errors] = false
+ expect(resource.x nil).to eq "default"
+ expect(resource.x).to eq "default"
+ end
+ end
+ if tags.include?(:nil_is_valid)
+ with_property ":x, #{validation}, default: nil" do
+ it "setting x to nil when it is already nil does not emit a warning" do
+ expect(resource.x nil).to be_nil
+ expect(resource.x).to be_nil
+ end
+ it "changing x to nil warns that the get will change to a set in Chef 13 and does not change the value" do
+ resource.instance_eval { @x = "default" }
+ expect { resource.x nil }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
+ /An attempt was made to change x from "default" to nil by calling x\(nil\). In Chef 12, this does a get rather than a set. In Chef 13, this will change to set the value to nil./
Chef::Config[:treat_deprecation_warnings_as_errors] = false
+ expect(resource.x nil).to eq "default"
+ expect(resource.x).to eq "default"
+ end
+ end
+ else
+ it "property :x, #{validation}, default: nil warns that the default is invalid" do
+ expect { resource_class.class_eval("property :x, #{validation}, default: nil", __FILE__, __LINE__) }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
+ /Default value nil is invalid for property x of resource chef_resource_property_spec_(\d+). Possible fixes: 1. Remove 'default: nil' if nil means 'undefined'. 2. Set a valid default value if there is a reasonable one. 3. Allow nil as a valid value of your property \(for example, 'property :x, \[ String, nil \], default: nil'\)./
+ end
+ context "With property :x, #{validation}, default: nil" do
+ before do
+ Chef::Config[:treat_deprecation_warnings_as_errors] = false
+ resource_class.class_eval("property :x, #{validation}, default: nil", __FILE__, __LINE__)
+ Chef::Config[:treat_deprecation_warnings_as_errors] = true
+ end
+
+ it "changing x to nil emits a warning that the value is invalid and does not change the value" do
resource.instance_eval { @x = "default" }
- expect(resource.x v).to eq "default"
+ expect { resource.x nil }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
+ /nil is an invalid value for x of resource chef_resource_property_spec_(\d+). In Chef 13, this warning will change to an error./
+ Chef::Config[:treat_deprecation_warnings_as_errors] = false
+ expect(resource.x nil).to eq "default"
expect(resource.x).to eq "default"
end
end
@@ -150,9 +191,12 @@ describe "Chef::Resource.property validation" do
it "get succeeds" do
expect(resource.x).to eq "default"
end
- it "set(nil) sets the value" do
- expect(resource.x nil).to be_nil
- expect(resource.x).to be_nil
+ it "set(nil) emits a warning that the value will be set, but does not set the value" do
+ expect { resource.x nil }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
+ /An attempt was made to change x from "default" to nil by calling x\(nil\). In Chef 12, this does a get rather than a set. In Chef 13, this will change to set the value to nil./
+ Chef::Config[:treat_deprecation_warnings_as_errors] = false
+ expect(resource.x nil).to eq "default"
+ expect(resource.x).to eq "default"
end
it "set to valid value succeeds" do
expect(resource.x "str").to eq "str"
@@ -185,46 +229,41 @@ describe "Chef::Resource.property validation" do
context "bare types" do
validation_test "String",
[ "hi" ],
- [ 10 ],
- [ nil ]
+ [ 10 ]
validation_test ":a",
[ :a ],
- [ :b ],
- [ nil ]
+ [ :b ]
validation_test ":a, is: :b",
[ :a, :b ],
- [ :c ],
- [ nil ]
+ [ :c ]
validation_test ":a, is: [ :b, :c ]",
[ :a, :b, :c ],
- [ :d ],
- [ nil ]
+ [ :d ]
validation_test "[ :a, :b ], is: :c",
[ :a, :b, :c ],
- [ :d ],
- [ nil ]
+ [ :d ]
validation_test "[ :a, :b ], is: [ :c, :d ]",
[ :a, :b, :c, :d ],
- [ :e ],
- [ nil ]
+ [ :e ]
validation_test "nil",
- [ nil ],
- [ :a ]
+ [ ],
+ [ :a ],
+ :nil_is_valid
validation_test "[ nil ]",
- [ nil ],
- [ :a ]
+ [ ],
+ [ :a ],
+ :nil_is_valid
validation_test "[]",
[],
- [ :a ],
- [ nil ]
+ [ :a ]
end
# is
@@ -232,35 +271,30 @@ describe "Chef::Resource.property validation" do
# Class
validation_test "is: String",
[ "a", "" ],
- [ :a, 1 ],
- [ nil ]
+ [ :a, 1 ]
# Value
validation_test "is: :a",
[ :a ],
- [ :b ],
- [ nil ]
+ [ :b ]
validation_test "is: [ :a, :b ]",
[ :a, :b ],
- [ [ :a, :b ] ],
- [ nil ]
+ [ [ :a, :b ] ]
validation_test "is: [ [ :a, :b ] ]",
[ [ :a, :b ] ],
- [ :a, :b ],
- [ nil ]
+ [ :a, :b ]
# Regex
validation_test "is: /abc/",
[ "abc", "wowabcwow" ],
- [ "", "abac" ],
- [ nil ]
+ [ "", "abac" ]
# Property
validation_test "is: Chef::Property.new(is: :a)",
[ :a ],
- [ :b, nil ]
+ [ :b ]
# RSpec Matcher
class Globalses
@@ -269,31 +303,30 @@ describe "Chef::Resource.property validation" do
validation_test "is: Globalses.eq(10)",
[ 10 ],
- [ 1 ],
- [ nil ]
+ [ 1 ]
# Proc
validation_test "is: proc { |x| x }",
[ true, 1 ],
- [ false ],
- [ nil ]
+ [ false ]
validation_test "is: proc { |x| x > blah }",
[ 10 ],
[ -1 ]
validation_test "is: nil",
- [ nil ],
- [ "a" ]
+ [ ],
+ [ "a" ],
+ :nil_is_valid
validation_test "is: [ String, nil ]",
- [ "a", nil ],
- [ :b ]
+ [ "a" ],
+ [ :b ],
+ :nil_is_valid
validation_test "is: []",
[],
- [ :a ],
- [ nil ]
+ [ :a ]
end
# Combination
@@ -301,7 +334,7 @@ describe "Chef::Resource.property validation" do
validation_test 'kind_of: String, equal_to: "a"',
[ "a" ],
[ "b" ],
- [ nil ]
+ :nil_is_valid
end
# equal_to
@@ -310,37 +343,38 @@ describe "Chef::Resource.property validation" do
validation_test "equal_to: :a",
[ :a ],
[ :b ],
- [ nil ]
+ :nil_is_valid
validation_test "equal_to: [ :a, :b ]",
[ :a, :b ],
[ [ :a, :b ] ],
- [ nil ]
+ :nil_is_valid
validation_test "equal_to: [ [ :a, :b ] ]",
[ [ :a, :b ] ],
[ :a, :b ],
- [ nil ]
+ :nil_is_valid
validation_test "equal_to: nil",
[ ],
[ "a" ],
- [ nil ]
+ :nil_is_valid
validation_test 'equal_to: [ "a", nil ]',
[ "a" ],
[ "b" ],
- [ nil ]
+ :nil_is_valid
validation_test 'equal_to: [ nil, "a" ]',
[ "a" ],
[ "b" ],
- [ nil ]
+ :nil_is_valid
validation_test "equal_to: []",
[],
[ :a ],
- [ nil ]
+ :nil_is_valid
+
end
# kind_of
@@ -348,37 +382,37 @@ describe "Chef::Resource.property validation" do
validation_test "kind_of: String",
[ "a" ],
[ :b ],
- [ nil ]
+ :nil_is_valid
validation_test "kind_of: [ String, Symbol ]",
[ "a", :b ],
[ 1 ],
- [ nil ]
+ :nil_is_valid
validation_test "kind_of: [ Symbol, String ]",
[ "a", :b ],
[ 1 ],
- [ nil ]
+ :nil_is_valid
validation_test "kind_of: NilClass",
[ ],
[ "a" ],
- [ nil ]
+ :nil_is_valid
validation_test "kind_of: [ NilClass, String ]",
[ "a" ],
[ :a ],
- [ nil ]
+ :nil_is_valid
validation_test "kind_of: []",
[],
[ :a ],
- [ nil ]
+ :nil_is_valid
validation_test "kind_of: nil",
[],
[ :a ],
- [ nil ]
+ :nil_is_valid
end
# regex
@@ -386,53 +420,55 @@ describe "Chef::Resource.property validation" do
validation_test "regex: /abc/",
[ "xabcy" ],
[ "gbh", 123 ],
- [ nil ]
+ :nil_is_valid
validation_test "regex: [ /abc/, /z/ ]",
[ "xabcy", "aza" ],
[ "gbh", 123 ],
- [ nil ]
+ :nil_is_valid
validation_test "regex: [ /z/, /abc/ ]",
[ "xabcy", "aza" ],
[ "gbh", 123 ],
- [ nil ]
+ :nil_is_valid
validation_test "regex: [ [ /z/, /abc/ ], [ /n/ ] ]",
[ "xabcy", "aza", "ana" ],
[ "gbh", 123 ],
- [ nil ]
+ :nil_is_valid
validation_test "regex: []",
[],
[ :a ],
- [ nil ]
+ :nil_is_valid
validation_test "regex: nil",
[],
[ :a ],
- [ nil ]
+ :nil_is_valid
end
# callbacks
context "callbacks" do
validation_test 'callbacks: { "a" => proc { |x| x > 10 }, "b" => proc { |x| x%2 == 0 } }',
[ 12 ],
- [ 11, 4 ]
+ [ 11, 4 ],
+ :nil_is_valid
validation_test 'callbacks: { "a" => proc { |x| x%2 == 0 }, "b" => proc { |x| x > 10 } }',
[ 12 ],
- [ 11, 4 ]
+ [ 11, 4 ],
+ :nil_is_valid
validation_test 'callbacks: { "a" => proc { |x| x.nil? } }',
[ ],
[ "a" ],
- [ nil ]
+ :nil_is_valid
validation_test "callbacks: {}",
[ :a ],
[],
- [ nil ]
+ :nil_is_valid
end
# respond_to
@@ -440,84 +476,84 @@ describe "Chef::Resource.property validation" do
validation_test "respond_to: :split",
[ "hi" ],
[ 1 ],
- [ nil ]
+ :nil_is_valid
validation_test 'respond_to: "split"',
[ "hi" ],
[ 1 ],
- [ nil ]
+ :nil_is_valid
validation_test "respond_to: :to_s",
[ :a ],
[],
- [ nil ]
+ :nil_is_valid
validation_test "respond_to: [ :split, :to_s ]",
[ "hi" ],
[ 1 ],
- [ nil ]
+ :nil_is_valid
validation_test "respond_to: %w(split to_s)",
[ "hi" ],
[ 1 ],
- [ nil ]
+ :nil_is_valid
validation_test "respond_to: [ :to_s, :split ]",
[ "hi" ],
[ 1 ],
- [ nil ]
+ :nil_is_valid
validation_test "respond_to: []",
[ :a ],
[],
- [ nil ]
+ :nil_is_valid
validation_test "respond_to: nil",
[ :a ],
[],
- [ nil ]
+ :nil_is_valid
end
context "cannot_be" do
validation_test "cannot_be: :empty",
[ 1, [1,2], { a: 10 } ],
[ [] ],
- [ nil ]
+ :nil_is_valid
validation_test 'cannot_be: "empty"',
[ 1, [1,2], { a: 10 } ],
[ [] ],
- [ nil ]
+ :nil_is_valid
validation_test "cannot_be: [ :empty, :nil ]",
[ 1, [1,2], { a: 10 } ],
[ [] ],
- [ nil ]
+ :nil_is_valid
validation_test 'cannot_be: [ "empty", "nil" ]',
[ 1, [1,2], { a: 10 } ],
[ [] ],
- [ nil ]
+ :nil_is_valid
validation_test "cannot_be: [ :nil, :empty ]",
[ 1, [1,2], { a: 10 } ],
[ [] ],
- [ nil ]
+ :nil_is_valid
validation_test "cannot_be: [ :empty, :nil, :blahblah ]",
[ 1, [1,2], { a: 10 } ],
[ [] ],
- [ nil ]
+ :nil_is_valid
validation_test "cannot_be: []",
[ :a ],
[],
- [ nil ]
+ :nil_is_valid
validation_test "cannot_be: nil",
[ :a ],
[],
- [ nil ]
+ :nil_is_valid
end
@@ -549,9 +585,8 @@ describe "Chef::Resource.property validation" do
it "if x is not specified, retrieval fails" do
expect { resource.x }.to raise_error Chef::Exceptions::ValidationFailed
end
- it "value nil is valid" do
- expect(resource.x nil).to be_nil
- expect(resource.x).to be_nil
+ it "value nil is not valid (required means 'not nil')" do
+ expect { resource.x nil }.to raise_error Chef::Exceptions::ValidationFailed
end
it "value '1' is valid" do
expect(resource.x "1").to eq "1"
diff --git a/spec/unit/property_spec.rb b/spec/unit/property_spec.rb
index 1fe8abc706..ffdbccf019 100644
--- a/spec/unit/property_spec.rb
+++ b/spec/unit/property_spec.rb
@@ -203,7 +203,7 @@ describe "Chef::Resource.property" do
expect(subresource_class.properties[:y]).not_to be_nil
end
it "y is not on the superclass" do
- expect { resource_class.y 10 }.to raise_error
+ expect { resource_class.y 10 }.to raise_error NoMethodError
expect(resource_class.properties[:y]).to be_nil
end
end
@@ -581,10 +581,17 @@ describe "Chef::Resource.property" do
end
context "validation of defaults" do
- with_property ":x, String, default: 10" do
- it "when the resource is created, no error is raised" do
- resource
+ it "When a class is declared with property :x, String, default: 10, a warning is emitted" do
+ expect { resource_class.class_eval { property :x, String, default: 10 } }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
+ /Default value 10 is invalid for property x of resource chef_resource_property_spec_(\d+). In Chef 13 this will become an error: Option x must be one of: String! You passed 10./
+ end
+ context "With property :x, String, default: 10" do
+ before do
+ Chef::Config[:treat_deprecation_warnings_as_errors] = false
+ resource_class.class_eval { property :x, String, default: 10 }
+ Chef::Config[:treat_deprecation_warnings_as_errors] = true
end
+
it "when x is set, no error is raised" do
expect(resource.x "hi").to eq "hi"
expect(resource.x).to eq "hi"
@@ -592,9 +599,6 @@ describe "Chef::Resource.property" do
it "when x is retrieved, no validation error is raised" do
expect(resource.x).to eq 10
end
- # it "when x is retrieved, a validation error is raised" do
- # expect { resource.x }.to raise_error Chef::Exceptions::ValidationFailed
- # end
end
with_property ":x, String, default: lazy { Namer.next_index }" do
@@ -605,40 +609,31 @@ describe "Chef::Resource.property" do
expect(resource.x "hi").to eq "hi"
expect(resource.x).to eq "hi"
end
- it "when x is retrieved, no validation error is raised" do
- expect(resource.x).to eq 1
+ it "when x is retrieved, an invalid default warning is emitted and the value is returned" do
+ expect { resource.x }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
+ /Default value 1 is invalid for property x of resource chef_resource_property_spec_(\d+). In Chef 13 this will become an error: Option x must be one of: String! You passed 1./
expect(Namer.current_index).to eq 1
+ Chef::Config[:treat_deprecation_warnings_as_errors] = false
+ expect(resource.x).to eq 2
end
- # it "when x is retrieved, a validation error is raised" do
- # expect { resource.x }.to raise_error Chef::Exceptions::ValidationFailed
- # expect(Namer.current_index).to eq 1
- # end
end
with_property ":x, default: lazy { Namer.next_index.to_s }, is: proc { |v| Namer.next_index; true }" do
- it "validation is not run at all on the default value" do
+ it "coercion and validation is only run the first time" do
expect(resource.x).to eq "1"
- expect(Namer.current_index).to eq 1
+ expect(Namer.current_index).to eq 2
+ expect(resource.x).to eq "1"
+ expect(Namer.current_index).to eq 2
end
- # it "validation is run each time" do
- # expect(resource.x).to eq '1'
- # expect(Namer.current_index).to eq 2
- # expect(resource.x).to eq '1'
- # expect(Namer.current_index).to eq 2
- # end
end
with_property ":x, default: lazy { Namer.next_index.to_s.freeze }, is: proc { |v| Namer.next_index; true }" do
- it "validation is not run at all on the default value" do
+ it "coercion and validation is run each time" do
expect(resource.x).to eq "1"
- expect(Namer.current_index).to eq 1
+ expect(Namer.current_index).to eq 2
+ expect(resource.x).to eq "3"
+ expect(Namer.current_index).to eq 4
end
- # it "validation is only run the first time" do
- # expect(resource.x).to eq '1'
- # expect(Namer.current_index).to eq 2
- # expect(resource.x).to eq '1'
- # expect(Namer.current_index).to eq 2
- # end
end
end
@@ -716,12 +711,12 @@ describe "Chef::Resource.property" do
end
with_property ':x, proc { |v| Namer.next_index; true }, coerce: proc { |v| "#{v}#{next_index}" }, default: lazy { 10 }' do
- it "coercion is only run the first time x is retrieved, and validation is not run" do
+ it "coercion and validation is only run the first time x is retrieved" do
expect(Namer.current_index).to eq 0
expect(resource.x).to eq "101"
- expect(Namer.current_index).to eq 1
+ expect(Namer.current_index).to eq 2
expect(resource.x).to eq "101"
- expect(Namer.current_index).to eq 1
+ expect(Namer.current_index).to eq 2
end
end
@@ -732,12 +727,12 @@ describe "Chef::Resource.property" do
end
end
with_property ':x, Integer, coerce: proc { |v| "#{v}#{next_index}" }, default: 10' do
- it "when x is retrieved, it is coerced and not validated" do
- expect(resource.x).to eq "101"
+ it "when x is retrieved, it is coerced and emits an invalid default warning, but still returns the value" do
+ expect { resource.x }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
+ /Default value 10 is invalid for property x of resource chef_resource_property_spec_(\d+). In Chef 13 this will become an error: Option x must be one of: Integer! You passed "101"./
+ Chef::Config[:treat_deprecation_warnings_as_errors] = false
+ expect(resource.x).to eq "102"
end
- # it "when x is retrieved, it is coerced before validating and fails" do
- # expect { resource.x }.to raise_error Chef::Exceptions::ValidationFailed
- # end
end
with_property ':x, String, coerce: proc { |v| "#{v}#{next_index}" }, default: lazy { 10 }' do
it "when x is retrieved, it is coerced before validating and passes" do
@@ -745,20 +740,20 @@ describe "Chef::Resource.property" do
end
end
with_property ':x, Integer, coerce: proc { |v| "#{v}#{next_index}" }, default: lazy { 10 }' do
- it "when x is retrieved, it is coerced and not validated" do
- expect(resource.x).to eq "101"
+ it "when x is retrieved, it is coerced and emits an invalid default warning; the value is still returned." do
+ expect { resource.x }.to raise_error Chef::Exceptions::DeprecatedFeatureError,
+ /Default value 10 is invalid for property x of resource chef_resource_property_spec_(\d+). In Chef 13 this will become an error: Option x must be one of: Integer! You passed "101"./
+ Chef::Config[:treat_deprecation_warnings_as_errors] = false
+ expect(resource.x).to eq "102"
end
- # it "when x is retrieved, it is coerced before validating and fails" do
- # expect { resource.x }.to raise_error Chef::Exceptions::ValidationFailed
- # end
end
with_property ':x, proc { |v| Namer.next_index; true }, coerce: proc { |v| "#{v}#{next_index}" }, default: lazy { 10 }' do
- it "coercion is only run the first time x is retrieved, and validation is not run" do
+ it "coercion is only run the first time x is retrieved, and validation is run" do
expect(Namer.current_index).to eq 0
expect(resource.x).to eq "101"
- expect(Namer.current_index).to eq 1
+ expect(Namer.current_index).to eq 2
expect(resource.x).to eq "101"
- expect(Namer.current_index).to eq 1
+ expect(Namer.current_index).to eq 2
end
end
end
@@ -922,7 +917,8 @@ describe "Chef::Resource.property" do
expect(Namer.current_index).to eq 1
end
it "does not emit a deprecation warning if set to nil" do
- expect(resource.x nil).to eq "1"
+ # nil is never coerced
+ expect(resource.x nil).to be_nil
end
it "coercion sets the value (and coercion does not run on get)" do
expect(resource.x 10).to eq "101"
diff --git a/spec/unit/provider/package/dpkg_spec.rb b/spec/unit/provider/package/dpkg_spec.rb
index e0bf2accde..f4c938ac7a 100644
--- a/spec/unit/provider/package/dpkg_spec.rb
+++ b/spec/unit/provider/package/dpkg_spec.rb
@@ -80,30 +80,26 @@ Conflicts: wget-ssl
expect { provider.run_action(:purge) }.not_to raise_error
end
- it "should raise an exception if a source is nil when :install" do
- new_resource.source nil
- allow(::File).to receive(:exist?).with(source).and_return(false)
- expect { provider.run_action(:install) }.to raise_error(Chef::Exceptions::Package)
- end
+ context "when source is nil" do
+ let(:source) { nil }
- it "should raise an exception if a source is nil when :upgrade" do
- new_resource.source nil
- allow(::File).to receive(:exist?).with(source).and_return(false)
- expect { provider.run_action(:upgrade) }.to raise_error(Chef::Exceptions::Package)
- end
+ it "should raise an exception if a source is nil when :install" do
+ expect { provider.run_action(:install) }.to raise_error(Chef::Exceptions::Package)
+ end
- it "should not raise an exception if a source is nil when :remove" do
- new_resource.source nil
- allow(::File).to receive(:exist?).with(source).and_return(false)
- expect(provider).to receive(:action_remove)
- expect { provider.run_action(:remove) }.not_to raise_error
- end
+ it "should raise an exception if a source is nil when :upgrade" do
+ expect { provider.run_action(:upgrade) }.to raise_error(Chef::Exceptions::Package)
+ end
- it "should not raise an exception if a source is nil when :purge" do
- new_resource.source nil
- allow(::File).to receive(:exist?).with(source).and_return(false)
- expect(provider).to receive(:action_purge)
- expect { provider.run_action(:purge) }.not_to raise_error
+ it "should not raise an exception if a source is nil when :remove" do
+ expect(provider).to receive(:action_remove)
+ expect { provider.run_action(:remove) }.not_to raise_error
+ end
+
+ it "should not raise an exception if a source is nil when :purge" do
+ expect(provider).to receive(:action_purge)
+ expect { provider.run_action(:purge) }.not_to raise_error
+ end
end
end
diff --git a/spec/unit/resource_reporter_spec.rb b/spec/unit/resource_reporter_spec.rb
index aa162fd161..3dcd51deab 100644
--- a/spec/unit/resource_reporter_spec.rb
+++ b/spec/unit/resource_reporter_spec.rb
@@ -271,6 +271,7 @@ describe Chef::ResourceReporter do
@bad_resource = Chef::Resource::File.new("/tmp/nameless_file.txt")
allow(@bad_resource).to receive(:name).and_return(nil)
allow(@bad_resource).to receive(:identity).and_return(nil)
+ allow(@bad_resource).to receive(:path).and_return(nil)
@resource_reporter.resource_action_start(@bad_resource, :create)
@resource_reporter.resource_current_state_loaded(@bad_resource, :create, @current_resource)
@resource_reporter.resource_updated(@bad_resource, :create)
@@ -294,6 +295,7 @@ describe Chef::ResourceReporter do
@bad_resource = Chef::Resource::File.new("/tmp/filename_as_hash.txt")
allow(@bad_resource).to receive(:name).and_return({:foo=>:bar})
allow(@bad_resource).to receive(:identity).and_return({:foo=>:bar})
+ allow(@bad_resource).to receive(:path).and_return({:foo=>:bar})
@resource_reporter.resource_action_start(@bad_resource, :create)
@resource_reporter.resource_current_state_loaded(@bad_resource, :create, @current_resource)
@resource_reporter.resource_updated(@bad_resource, :create)
View Lorry Controller Status