+1, although it might be worth always defaulting to strings if the two
arguments aren't of the same type.
On Nov 19, 2008, at 3:13 AM, Brice Figureau wrote:
>
> I also piggy-backed the change to cleanup a little bit the spec test.
>
> Signed-off-by: Brice Figureau <[EMAIL PROTECTED]>
> ---
> lib/puppet/parser/ast/comparison_operator.rb | 4 ++
> spec/unit/parser/ast/comparison_operator.rb | 60 +++++++++++++++++
> ++++----
> 2 files changed, 54 insertions(+), 10 deletions(-)
>
> diff --git a/lib/puppet/parser/ast/comparison_operator.rb b/lib/
> puppet/parser/ast/comparison_operator.rb
> index 63aa36c..3af86ef 100644
> --- a/lib/puppet/parser/ast/comparison_operator.rb
> +++ b/lib/puppet/parser/ast/comparison_operator.rb
> @@ -18,6 +18,10 @@ class Puppet::Parser::AST
> lval = @lval.safeevaluate(scope)
> rval = @rval.safeevaluate(scope)
>
> + # convert to number if operands are number
> + lval = Puppet::Parser::Scope.number?(lval) || lval
> + rval = Puppet::Parser::Scope.number?(rval) || rval
> +
> # return result
> unless @operator == '!='
> lval.send(@operator,rval)
> diff --git a/spec/unit/parser/ast/comparison_operator.rb b/spec/unit/
> parser/ast/comparison_operator.rb
> index dbea349..80e8307 100755
> --- a/spec/unit/parser/ast/comparison_operator.rb
> +++ b/spec/unit/parser/ast/comparison_operator.rb
> @@ -5,8 +5,8 @@ require File.dirname(__FILE__) + '/../../../
> spec_helper'
> describe Puppet::Parser::AST::ComparisonOperator do
> before :each do
> @scope = Puppet::Parser::Scope.new()
> - @one = Puppet::Parser::AST::FlatString.new( :value => 1 )
> - @two = Puppet::Parser::AST::FlatString.new( :value => 2 )
> + @one = stub 'one', :safeevaluate => "1"
> + @two = stub 'two', :safeevaluate => "2"
> end
>
> it "should evaluate both branches" do
> @@ -19,34 +19,74 @@ describe Puppet::Parser::AST::ComparisonOperator
> do
> operator.evaluate(@scope)
> end
>
> + it "should convert arguments strings to numbers if they are" do
> + Puppet::Parser::Scope.expects(:number?).with("1").returns(1)
> + Puppet::Parser::Scope.expects(:number?).with("2").returns(2)
> +
> + operator =
> Puppet::Parser::AST::ComparisonOperator.new :lval => @one, :operator
> => "==", :rval => @two
> + operator.evaluate(@scope)
> + end
> +
> + %w{< > <= >= ==}.each do |oper|
> + it "should use string comparison #{oper} if operands are
> strings" do
> + lval = stub 'one', :safeevaluate => "one"
> + rval = stub 'two', :safeevaluate => "two"
> +
> Puppet::Parser::Scope.stubs(:number?).with("one").returns(nil)
> +
> Puppet::Parser::Scope.stubs(:number?).with("two").returns(nil)
> +
> + operator =
> Puppet::Parser::AST::ComparisonOperator.new :lval => lval, :operator
> => oper, :rval => rval
> + operator.evaluate(@scope).should ==
> "one".send(oper,"two")
> + end
> + end
> +
> + it "should fail with arguments of different types" do
> + lval = stub 'one', :safeevaluate => "one"
> + rval = stub 'two', :safeevaluate => "2"
> +
> Puppet::Parser::Scope.stubs(:number?).with("one").returns(nil)
> + Puppet::Parser::Scope.stubs(:number?).with("2").returns(2)
> +
> + operator =
> Puppet::Parser::AST::ComparisonOperator.new :lval => lval, :operator
> => ">", :rval => rval
> + lambda { operator.evaluate(@scope) }.should
> raise_error(ArgumentError)
> + end
> +
> it "should fail for an unknown operator" do
> lambda { operator =
> Puppet::Parser::AST::ComparisonOperator.new :lval => @one, :operator
> => "or", :rval => @two }.should raise_error
> end
>
> %w{< > <= >= ==}.each do |oper|
> it "should return the result of using '#{oper}' to compare
> the left and right sides" do
> - one = stub 'one', :safeevaluate => "1"
> - two = stub 'two', :safeevaluate => "2"
> - operator =
> Puppet::Parser::AST::ComparisonOperator.new :lval => one, :operator
> => oper, :rval => two
> + operator =
> Puppet::Parser::AST::ComparisonOperator.new :lval => @one, :operator
> => oper, :rval => @two
> +
> operator.evaluate(@scope).should == 1.send(oper,2)
> end
> end
>
> it "should return the result of using '!=' to compare the left
> and right sides" do
> - one = stub 'one', :safeevaluate => "1"
> - two = stub 'two', :safeevaluate => "2"
> - operator =
> Puppet::Parser::AST::ComparisonOperator.new :lval => one, :operator
> => '!=', :rval => two
> + operator =
> Puppet::Parser::AST::ComparisonOperator.new :lval => @one, :operator
> => '!=', :rval => @two
> +
> operator.evaluate(@scope).should == true
> end
>
> it "should work for variables too" do
> - @scope.expects(:lookupvar).with("one").returns(1)
> - @scope.expects(:lookupvar).with("two").returns(2)
> one = Puppet::Parser::AST::Variable.new( :value => "one" )
> two = Puppet::Parser::AST::Variable.new( :value => "two" )
> +
> + @scope.expects(:lookupvar).with("one").returns(1)
> + @scope.expects(:lookupvar).with("two").returns(2)
>
> operator = Puppet::Parser::AST::ComparisonOperator.new :lval
> => one, :operator => "<", :rval => two
> operator.evaluate(@scope).should == true
> end
>
> + # see ticket #1759
> + %w{< > <= >=}.each do |oper|
> + it "should return the correct result of using '#{oper}' to
> compare 10 and 9" do
> + ten = stub 'one', :safeevaluate => "10"
> + nine = stub 'two', :safeevaluate => "9"
> + operator =
> Puppet::Parser::AST::ComparisonOperator.new :lval => ten, :operator
> => oper, :rval => nine
> +
> + operator.evaluate(@scope).should == 10.send(oper,9)
> + end
> + end
> +
> end
> --
> 1.5.6.5
>
>
> >
--
There are three social classes in America: upper middle class, middle
class, and lower middle class. --Judith Martin
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
