diff options
author | John Keiser <john@johnkeiser.com> | 2016-02-02 10:11:25 -0800 |
---|---|---|
committer | John Keiser <john@johnkeiser.com> | 2016-02-03 12:42:13 -0800 |
commit | 209a9cc2f3eaa1c2046bae485d69d52f4e11399e (patch) | |
tree | 4b893266b1207dc89e7ddc23b65fc8cd86755ddb | |
parent | 0f4a623172676d010782d880a3bc7a9da084d25d (diff) | |
download | chef-209a9cc2f3eaa1c2046bae485d69d52f4e11399e.tar.gz |
Allow multiple property_types in `is`jk/property-is-multiple-types
-rw-r--r-- | lib/chef/mixin/params_validate.rb | 29 | ||||
-rw-r--r-- | spec/unit/property_spec.rb | 91 |
2 files changed, 96 insertions, 24 deletions
diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb index b922f28475..a51b24df24 100644 --- a/lib/chef/mixin/params_validate.rb +++ b/lib/chef/mixin/params_validate.rb @@ -400,23 +400,34 @@ class Chef return true if !opts.has_key?(key.to_s) && !opts.has_key?(key.to_sym) value = _pv_opts_lookup(opts, key) to_be = [ to_be ].flatten(1) - to_be.each do |tb| + errors = [] + passed = to_be.any? 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) + instance_exec(value, &tb) when Property - validate(opts, { key => tb.validation_options }) - return true + begin + validate(opts, { key => tb.validation_options }) + true + rescue Exceptions::ValidationFailed + # re-raise immediately if there is only one "is" so we get a better stack + raise if to_be.size == 1 + errors << $! + false + end else - return true if tb === value + tb === value end end - - if raise_error - raise Exceptions::ValidationFailed, "Option #{key} must be one of: #{to_be.join(", ")}! You passed #{value.inspect}." + if passed + true else - false + message = "Property #{key} must be one of: #{to_be.map { |v| v.inspect }.join(", ")}! You passed #{value.inspect}." + unless errors.empty? + message << " Errors:\n#{errors.map { |m| "- #{m}" }.join("\n")}" + end + raise Exceptions::ValidationFailed, message end end diff --git a/spec/unit/property_spec.rb b/spec/unit/property_spec.rb index 2ff266f0ca..dc969ebde2 100644 --- a/spec/unit/property_spec.rb +++ b/spec/unit/property_spec.rb @@ -583,7 +583,7 @@ describe "Chef::Resource.property" do context "validation of defaults" do 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./ + /Default value 10 is invalid for property x of resource chef_resource_property_spec_(\d+). In Chef 13 this will become an error: Property x must be one of: String! You passed 10./ end context "With property :x, String, default: 10" do before do @@ -611,7 +611,7 @@ describe "Chef::Resource.property" do end 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./ + /Default value 1 is invalid for property x of resource chef_resource_property_spec_(\d+). In Chef 13 this will become an error: Property 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 @@ -729,7 +729,7 @@ describe "Chef::Resource.property" do with_property ':x, Integer, coerce: proc { |v| "#{v}#{next_index}" }, default: 10' do 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"./ + /Default value 10 is invalid for property x of resource chef_resource_property_spec_(\d+). In Chef 13 this will become an error: Property x must be one of: Integer! You passed "101"./ Chef::Config[:treat_deprecation_warnings_as_errors] = false expect(resource.x).to eq "102" end @@ -742,7 +742,7 @@ describe "Chef::Resource.property" do with_property ':x, Integer, coerce: proc { |v| "#{v}#{next_index}" }, default: lazy { 10 }' do 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"./ + /Default value 10 is invalid for property x of resource chef_resource_property_spec_(\d+). In Chef 13 this will become an error: Property x must be one of: Integer! You passed "101"./ Chef::Config[:treat_deprecation_warnings_as_errors] = false expect(resource.x).to eq "102" end @@ -1096,19 +1096,80 @@ describe "Chef::Resource.property" do /Cannot specify both name_property and name_attribute together on property x of resource chef_resource_property_spec_(\d+)./ end - it "property_types validate their defaults" do - expect { - module PropertySpecPropertyTypes - include Chef::Mixin::Properties - property_type(is: [:a, :b], default: :c) + context "property_type" do + it "property_types validate their defaults" do + expect { + module ::PropertySpecPropertyTypes + include Chef::Mixin::Properties + property_type(is: [:a, :b], default: :c) + end + }.to raise_error(Chef::Exceptions::DeprecatedFeatureError, /Default value :c is invalid for property <property type>./) + expect { + module ::PropertySpecPropertyTypes + include Chef::Mixin::Properties + property_type(is: [:a, :b], default: :b) + end + }.not_to raise_error + end + + context "With property_type ABType (is: [:a, :b]) and CDType (is: [:c, :d])" do + before :all do + module ::PropertySpecPropertyTypes + include Chef::Mixin::Properties + ABType = property_type(is: [:a, :b]) + CDType = property_type(is: [:c, :d]) + end + end + + with_property ":x, [PropertySpecPropertyTypes::ABType, nil, PropertySpecPropertyTypes::CDType]" do + it "The property can be set to nil without triggering a warning" do + expect(resource.x nil).to be_nil + expect(resource.x).to be_nil + end + it "The property can be set to :a" do + expect(resource.x :a).to eq(:a) + expect(resource.x).to eq(:a) + end + it "The property can be set to :c" do + expect(resource.x :c).to eq(:c) + expect(resource.x).to eq(:c) + end + it "The property cannot be set to :z" do + expect { resource.x :z }.to raise_error(Chef::Exceptions::ValidationFailed, /Property x must be one of/) + end + end + + with_property ":x, [nil, PropertySpecPropertyTypes::ABType, PropertySpecPropertyTypes::CDType]" do + it "The property can be set to nil without triggering a warning" do + expect(resource.x nil).to be_nil + expect(resource.x).to be_nil + end + it "The property can be set to :a" do + expect(resource.x :a).to eq(:a) + expect(resource.x).to eq(:a) + end + it "The property can be set to :c" do + expect(resource.x :c).to eq(:c) + expect(resource.x).to eq(:c) + end + it "The property cannot be set to :z" do + expect { resource.x :z }.to raise_error(Chef::Exceptions::ValidationFailed, /Property x must be one of/) + end + end + + with_property ":x, [PropertySpecPropertyTypes::ABType, nil], default: nil" do + it "The value defaults to nil" do + expect(resource.x).to be_nil + end end - }.to raise_error(Chef::Exceptions::DeprecatedFeatureError, /Default value :c is invalid for property <property type>./) - expect { - module PropertySpecPropertyTypes - include Chef::Mixin::Properties - property_type(is: [:a, :b], default: :b) + + with_property ":x, [PropertySpecPropertyTypes::ABType, nil], default: lazy { nil }" do + it "The value defaults to nil" do + expect(resource.x).to be_nil + end end - }.not_to raise_error + end + end context "with a custom property type" do |