Merge branch 'maint-refactor_validate_slength'

2 files changed, 74 insertions, 63 deletions

diff --git a/lib/puppet/parser/functions/validate_slength.rb b/lib/puppet/parser/functions/validate_slength.rb
index 339a21d..7d534f3 100644
--- a/lib/puppet/parser/functions/validate_slength.rb
+++ b/lib/puppet/parser/functions/validate_slength.rb
@@ -21,47 +21,51 @@ module Puppet::Parser::Functions
 
     ENDHEREDOC
 
-    raise Puppet::ParseError, ("validate_slength(): Wrong number of arguments (#{args.length}; must be 2 or 3)") unless args.length == 2 or args.length == 3
+    raise Puppet::ParseError, "validate_slength(): Wrong number of arguments (#{args.length}; must be 2 or 3)" unless args.length == 2 or args.length == 3
 
-    unless (args[0].is_a?(String) or args[0].is_a?(Array))
-      raise Puppet::ParseError, ("validate_slength(): please pass a string, or an array of strings - what you passed didn't work for me at all - #{args[0].class}")
-    end
+    input, max_length, min_length = *args
 
     begin
-      max_length = args[1].to_i
-    rescue NoMethodError => e
-      raise Puppet::ParseError, ("validate_slength(): Couldn't convert whatever you passed as the max length parameter to an integer  - sorry: " + e.message )
+      max_length = Integer(max_length)
+      raise ArgumentError if max_length <= 0
+    rescue ArgumentError, TypeError
+      raise Puppet::ParseError, "validate_slength(): Expected second argument to be a positive Numeric, got #{max_length}:#{max_length.class}"
     end
 
-    unless args.length == 2
+    if min_length
       begin
-        min_length = Integer(args[2])
-      rescue StandardError => e
-        raise Puppet::ParseError, ("validate_slength(): Couldn't convert whatever you passed as the min length parameter to an integer  - sorry: " + e.message )
+        min_length = Integer(min_length)
+        raise ArgumentError if min_length < 0
+    rescue ArgumentError, TypeError
+        raise Puppet::ParseError, "validate_slength(): Expected third argument to be unset or a positive Numeric, got #{min_length}:#{min_length.class}"
       end
     else
       min_length = 0
     end
 
-    raise Puppet::ParseError, ("validate_slength(): please pass a positive number as max_length") unless max_length > 0
-    raise Puppet::ParseError, ("validate_slength(): please pass a positive number as min_length") unless min_length >= 0
-    raise Puppet::ParseError, ("validate_slength(): please pass a min length that is smaller than the maximum") unless min_length <= max_length
+    if min_length > max_length
+      raise Puppet::ParseError, "validate_slength(): Expected second argument to be larger than third argument"
+    end
 
-    case args[0]
-      when String
-        raise Puppet::ParseError, ("validate_slength(): #{args[0].inspect} is #{args[0].length} characters.  It should have been between #{min_length} and #{max_length} characters") unless args[0].length <= max_length and min_length <= args[0].length
-      when Array
-        args[0].each do |arg|
-          if arg.is_a?(String)
-            unless ( arg.is_a?(String) and arg.length <= max_length and min_length <= arg.length)
-              raise Puppet::ParseError, ("validate_slength(): #{arg.inspect} is #{arg.length} characters.  It should have been between #{min_length} and #{max_length} characters")
-            end
-          else
-            raise Puppet::ParseError, ("validate_slength(): #{arg.inspect} is not a string, it's a #{arg.class}")
-          end
+    validator = lambda do |str|
+      unless str.length <= max_length and str.length >= min_length
+        raise Puppet::ParseError, "validate_slength(): Expected length of #{input.inspect} to be between #{min_length} and #{max_length}, was #{input.length}"
+      end
+    end
+
+    case input
+    when String
+      validator.call(input)
+    when Array
+      input.each_with_index do |arg, pos|
+        if arg.is_a? String
+          validator.call(arg)
+        else
+          raise Puppet::ParseError, "validate_slength(): Expected element at array position #{pos} to be a String, got #{arg.class}"
         end
-      else
-        raise Puppet::ParseError, ("validate_slength(): please pass a string, or an array of strings - what you passed didn't work for me at all - #{args[0].class}")
+      end
+    else
+      raise Puppet::ParseError, "validate_slength(): Expected first argument to be a String or Array, got #{input.class}"
     end
   end
 end
diff --git a/spec/unit/puppet/parser/functions/validate_slength_spec.rb b/spec/unit/puppet/parser/functions/validate_slength_spec.rb
index 6ed1b0f..851835f 100755
--- a/spec/unit/puppet/parser/functions/validate_slength_spec.rb
+++ b/spec/unit/puppet/parser/functions/validate_slength_spec.rb
@@ -9,52 +9,59 @@ describe "the validate_slength function" do
     Puppet::Parser::Functions.function("validate_slength").should == "function_validate_slength"
   end
 
-  it "should raise a ParseError if there is less than 2 arguments" do
-    expect { scope.function_validate_slength([]) }.to(raise_error(Puppet::ParseError))
-    expect { scope.function_validate_slength(["asdf"]) }.to(raise_error(Puppet::ParseError))
-  end
+  describe "validating the input argument types" do
+    it "raises an error if there are less than two arguments" do
+      expect { scope.function_validate_slength([]) }.to raise_error Puppet::ParseError, /Wrong number of arguments/
+    end
 
-  it "should raise a ParseError if argument 2 doesn't convert to a fixnum" do
-    expect { scope.function_validate_slength(["moo",["2"]]) }.to(raise_error(Puppet::ParseError, /Couldn't convert whatever you passed/))
-  end
+    it "raises an error if there are more than three arguments" do
+      expect { scope.function_validate_slength(['input', 1, 2, 3]) }.to raise_error Puppet::ParseError, /Wrong number of arguments/
+    end
 
-  it "should raise a ParseError if argument 2 converted, but to 0, e.g. a string" do
-    expect { scope.function_validate_slength(["moo","monkey"]) }.to(raise_error(Puppet::ParseError, /please pass a positive number as max_length/))
-  end
+    it "raises an error if the first argument is not a string" do
+      expect { scope.function_validate_slength([Object.new, 2, 1]) }.to raise_error Puppet::ParseError, /Expected first argument.*got .*Object/
+    end
 
-  it "should raise a ParseError if argument 2 converted, but to 0" do
-    expect { scope.function_validate_slength(["moo","0"]) }.to(raise_error(Puppet::ParseError, /please pass a positive number as max_length/))
-  end
+    it "raises an error if the second argument cannot be cast to an Integer" do
+      expect { scope.function_validate_slength(['input', Object.new]) }.to raise_error Puppet::ParseError, /Expected second argument.*got .*Object/
+    end
 
-  it "should raise a ParseError if argument 3 doesn't convert to a fixnum" do
-    expect { scope.function_validate_slength(["moo",2,["3"]]) }.to(raise_error(Puppet::ParseError, /Couldn't convert whatever you passed/))
-  end
+    it "raises an error if the third argument cannot be cast to an Integer" do
+      expect { scope.function_validate_slength(['input', 1, Object.new]) }.to raise_error Puppet::ParseError, /Expected third argument.*got .*Object/
+    end
 
-  it "should raise a ParseError if argument 3 converted, but to 0, e.g. a string" do
-    expect { scope.function_validate_slength(["moo",2,"monkey"]) }.to(raise_error(Puppet::ParseError, /Couldn't convert whatever you passed/))
+    it "raises an error if the second argument is smaller than the third argument" do
+      expect { scope.function_validate_slength(['input', 1, 2]) }.to raise_error Puppet::ParseError, /Expected second argument to be larger than third argument/
+    end
   end
 
-  it "should fail if string greater then size" do
-    expect { scope.function_validate_slength(["test", 2]) }.to(raise_error(Puppet::ParseError, /It should have been between 0 and 2/))
-  end
+  describe "validating the input string length" do
+    describe "when the input is a string" do
+      it "fails validation if the string is larger than the max length" do
+        expect { scope.function_validate_slength(['input', 1]) }.to raise_error Puppet::ParseError, /Expected length .* between 0 and 1, was 5/
+      end
 
-  it "should fail if the min length is larger than the max length" do
-    expect { scope.function_validate_slength(["test", 10, 15]) }.to(raise_error(Puppet::ParseError, /pass a min length that is smaller than the max/))
-  end
+      it "fails validation if the string is less than the min length" do
+        expect { scope.function_validate_slength(['input', 10, 6]) }.to raise_error Puppet::ParseError, /Expected length .* between 6 and 10, was 5/
+      end
 
-  it "should fail if you pass an array of something other than strings" do
-    expect { scope.function_validate_slength([["moo",["moo"],Hash.new["moo" => 7]], 7]) }.to(raise_error(Puppet::ParseError, /is not a string, it's a/))
-  end
+      it "doesn't raise an error if the string is under the max length" do
+        scope.function_validate_slength(['input', 10])
+      end
 
-  it "should fail if you pass something other than a string or array" do
-    expect { scope.function_validate_slength([Hash.new["moo" => "7"],6]) }.to(raise_error(Puppet::ParseError, /please pass a string, or an array of strings/))
-  end
+      it "doesn't raise an error if the string is equal to the max length" do
+        scope.function_validate_slength(['input', 5])
+      end
 
-  it "should not fail if string is smaller or equal to size" do
-    expect { scope.function_validate_slength(["test", 5]) }.to_not(raise_error(Puppet::ParseError))
-  end
+      it "doesn't raise an error if the string is equal to the min length" do
+        scope.function_validate_slength(['input', 10, 5])
+      end
+    end
 
-  it "should not fail if array of string is are all smaller or equal to size" do
-    expect { scope.function_validate_slength([["moo","foo","bar"], 5]) }.to_not(raise_error(Puppet::ParseError))
+    describe "when the input is an array" do
+      it "fails validation if one of the array elements is not a string" do
+        expect { scope.function_validate_slength([["a", "b", Object.new], 2]) }.to raise_error Puppet::ParseError, /Expected element at array position 2 .*String, got .*Object/
+      end
+    end
   end
 end