Allow an attribute with default boolean value to be set correctly #110
Conversation
lib/puppet/resource_api.rb
Outdated
| if options[:behaviour] != :read_only | ||
| if options.key? :default | ||
| if options[:behaviour] != :read_only && options.key?(:default) | ||
| if type.is_a? Puppet::Pops::Types::PBooleanType |
There was a problem hiding this comment.
What about compound types like Optional[Boolean], Array[Boolean] and Variant[String, Boolean]?
There was a problem hiding this comment.
Would it be enough to apply the workaround if and only if the default value is actually false?
lib/puppet/resource_api.rb
Outdated
| @@ -521,6 +524,15 @@ def self.mungify(type, value, error_msg_prefix) | |||
| # the correct types, e.g. "1" (a string) to 1 (an integer) | |||
| cleaned_value, error_msg = try_mungify(type, value, error_msg_prefix) | |||
| raise Puppet::ResourceError, error_msg if error_msg | |||
| elsif type.is_a? Puppet::Pops::Types::PBooleanType | |||
spec/puppet/resource_api_spec.rb
Outdated
| context 'when the default value is false' do | ||
| let(:type) { Puppet::Type.type(:default_bool) } | ||
| let(:default_value) { false } | ||
| let(:instance) { type.new(name: 'foo', ensure: 'present') } |
There was a problem hiding this comment.
lots of duplication here that can be pulled out to the parent context?
| @@ -1344,6 +1430,7 @@ def set(_context, changes) | |||
| context 'when flushing' do | |||
| before(:each) do | |||
| Puppet.debug = true | |||
| instance.my_provider.set(nil, nil) # reset the current_state | |||
There was a problem hiding this comment.
Huh, how are the changes above leaking into this test case?
There was a problem hiding this comment.
believe random seed order was changed when the new test went in and exposed the bug
There was a problem hiding this comment.
ah, ok! then please put this in a separate commit with a short note about that, to inform future-us of what went down here.
* ensure that the provider produces valid resources on `get` * run the `puppet apply` command in a let, instead of a hook * use the `wibble` resource to test the "optional boolean is absent" case * test the "variant uses string" case
|
Added some cleanups to the tests. After this has passed CI, we can merge and release? |
No description provided.