I'll leave someone else to comment on the code, but I really like the change
toward better errors.
One comment below.
On Jan 11, 2011, at 6:02 PM, Dan Bode wrote:
> Previously, when a class could not be found, it was displaying the same error
> message as when a resource type could not be found. This resulted in
> confusing error message: Invalid resource type class, when really it should
> display the name of the class that could not be found.
>
> My patch changes the error message to:
>
> Could not find declared class #{title}
>
> Signed-off-by: Dan Bode <[email protected]>
> ---
> lib/puppet/resource.rb | 10 ++++++++--
> spec/unit/resource_spec.rb | 8 ++++++++
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
> index 4f0d507..7d440e1 100644
> --- a/lib/puppet/resource.rb
> +++ b/lib/puppet/resource.rb
> @@ -201,8 +201,14 @@ class Puppet::Resource
> tag(self.title) if valid_tag?(self.title)
>
> @reference = Reference.new(@type,@title) # for serialization
> compatibility with 0.25.x
> -
> - raise ArgumentError, "Invalid resource type #{type}" if strict? and !
> resource_type
> + if strict? and ! resource_type
> + puts 'in'
This is probably unintentionally left in; extraneous debugging or something.
> + if @type == 'Class'
> + raise ArgumentError, "Could not find declared class #{title}"
> + else
> + raise ArgumentError, "Invalid resource type #{type}"
> + end
> + end
> end
>
> def ref
> diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
> index e65e8a1..11ff904 100755
> --- a/spec/unit/resource_spec.rb
> +++ b/spec/unit/resource_spec.rb
> @@ -98,6 +98,14 @@ describe Puppet::Resource do
> lambda { Puppet::Resource.new("foo") }.should raise_error(ArgumentError)
> end
>
> + it 'should fail if strict is set and type does not exist' do
> + lambda { Puppet::Resource.new('foo', 'title', {:strict=>true}) }.should
> raise_error(ArgumentError, 'Invalid resource type foo')
> + end
> +
> + it 'should fail if strict is set and class does not exist' do
> + lambda { Puppet::Resource.new('Class', 'foo', {:strict=>true}) }.should
> raise_error(ArgumentError, 'Could not find declared class foo')
> + end
> +
> it "should fail if the title is a hash and the type is not a valid resource
> reference string" do
> lambda { Puppet::Resource.new({:type => "foo", :title => "bar"}) }.should
> raise_error(ArgumentError,
> 'Puppet::Resource.new does not take a hash as the first argument. Did
> you mean ("foo", "bar") ?'
> --
> 1.5.5.6
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Developers" group.
> To post to this group, send email to [email protected].
> To unsubscribe from this group, send email to
> [email protected].
> For more options, visit this group at
> http://groups.google.com/group/puppet-dev?hl=en.
>
--
When a man sits with a pretty girl for an hour, it seems like a
minute. But let him sit on a hot stove for a minute, and it's longer
than any hour. That's relativity. --Albert Einstein
---------------------------------------------------------------------
Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
