Rather than stating the logic as 'if !thing', the two checks done when initializing a new Puppet::FileBucket::File are now phrased as 'unless thing', which should lessen the likelihood of overlooking the '!'.
We also now provide a reason for the ArgumentError being raised, which should help users of Puppet::FileBucket::File quickly figure out what is the problem when these exceptions are raised. In addition to updating the tests to look for these new error messages, we update the existing tests to specify which type of exception, and what message it should have, when something is raised. Reviewed-by: Nick Lewis <[email protected]> Signed-off-by: Jacob Helwig <[email protected]> --- lib/puppet/file_bucket/file.rb | 6 +++--- spec/unit/file_bucket/file_spec.rb | 12 +++++++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/puppet/file_bucket/file.rb b/lib/puppet/file_bucket/file.rb index 08c0329..2a0558f 100644 --- a/lib/puppet/file_bucket/file.rb +++ b/lib/puppet/file_bucket/file.rb @@ -15,11 +15,11 @@ class Puppet::FileBucket::File attr :bucket_path def initialize( contents, options = {} ) - raise ArgumentError if !contents.is_a?(String) - @contents = contents + raise ArgumentError.new("contents must be a String, got a #{contents.class}") unless contents.is_a?(String) + @contents = contents @bucket_path = options.delete(:bucket_path) - raise ArgumentError if options != {} + raise ArgumentError.new("Unknown option(s): #{options.keys.join(', ')}") unless options.empty? end def checksum_type diff --git a/spec/unit/file_bucket/file_spec.rb b/spec/unit/file_bucket/file_spec.rb index c4444ae..ebf0243 100755 --- a/spec/unit/file_bucket/file_spec.rb +++ b/spec/unit/file_bucket/file_spec.rb @@ -26,11 +26,17 @@ describe Puppet::FileBucket::File do it "should raise an error if changing content" do x = Puppet::FileBucket::File.new("first") - proc { x.contents = "new" }.should raise_error + expect { x.contents = "new" }.to raise_error(NoMethodError, /undefined method .contents=/) end it "should require contents to be a string" do - proc { Puppet::FileBucket::File.new(5) }.should raise_error(ArgumentError) + expect { Puppet::FileBucket::File.new(5) }.to raise_error(ArgumentError, /contents must be a String, got a Fixnum$/) + end + + it "should complain about options other than :bucket_path" do + expect { + Puppet::FileBucket::File.new('5', :crazy_option => 'should not be passed') + }.to raise_error(ArgumentError, /Unknown option\(s\): crazy_option/) end it "should set the contents appropriately" do @@ -61,7 +67,7 @@ describe Puppet::FileBucket::File do it "should reject a url-ish name with an invalid checksum" do bucket = Puppet::FileBucket::File.new(@contents) - lambda { bucket.name = "sha1/4a8ec4fa5f01b4ab1a0ab8cbccb709f0/new/path" }.should raise_error + expect { bucket.name = "sha1/4a8ec4fa5f01b4ab1a0ab8cbccb709f0/new/path" }.to raise_error(NoMethodError, /undefined method .name=/) end it "should convert the contents to PSON" do -- 1.7.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.
