[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 0.25.5-663-g71824ee

[Markus Roberts]

The following commit has been merged in the upstream branch:
commit b509032e559cb26e05863b2c290a543c6fa9d779
Author: Brice Figureau <brice-puppet at daysofwonder.com>
Date:   Fri Jul 16 10:22:27 2010 +0200

    Fix #4238 - if should match undef as ''
    
    The comparisons operator (and more particularly == and !=) were not treating
    the undef value as '', like case and selector did since #2818.
    
    This patch makes sure comparison operator uses AST leaf matching.
    
    Unfortunately, doing this introduces a behavior change compared to
    the previous versions:
    Numbers embedded in strings will now be matched as numbers in case and
    selector statements instead of string matching.
    
    Signed-off-by: Brice Figureau <brice-puppet at daysofwonder.com>

diff --git a/lib/puppet/parser/ast/comparison_operator.rb b/lib/puppet/parser/ast/comparison_operator.rb
index c8694bb..039c81d 100644
--- a/lib/puppet/parser/ast/comparison_operator.rb
+++ b/lib/puppet/parser/ast/comparison_operator.rb
@@ -16,17 +16,16 @@ class Puppet::Parser::AST
     def evaluate(scope)
       # evaluate the operands, should return a boolean value
       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
+      case @operator
+      when "==","!="
+        @rval.evaluate_match(lval, scope) ? @operator == '==' : @operator == '!='
+      else
+        rval = @rval.safeevaluate(scope)
+        rval = Puppet::Parser::Scope.number?(rval) || rval
+        lval = Puppet::Parser::Scope.number?(lval) || lval
 
-      # return result
-      unless @operator == '!='
         lval.send(@operator,rval)
-      else
-        lval != rval
       end
     end
 
diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb
index 3b9163d..49f4302 100644
--- a/lib/puppet/parser/ast/leaf.rb
+++ b/lib/puppet/parser/ast/leaf.rb
@@ -17,6 +17,9 @@ class Puppet::Parser::AST
       obj   = obj.downcase   if obj.respond_to?(:downcase)
       value = value.downcase if value.respond_to?(:downcase)
 
+      obj   = Puppet::Parser::Scope.number?(obj)   || obj
+      value = Puppet::Parser::Scope.number?(value) || value
+
       # "" == undef for case/selector/if
       obj == value or (obj == "" and value == :undef)
     end
diff --git a/spec/unit/parser/ast/comparison_operator_spec.rb b/spec/unit/parser/ast/comparison_operator_spec.rb
index 724b6c6..931f936 100755
--- a/spec/unit/parser/ast/comparison_operator_spec.rb
+++ b/spec/unit/parser/ast/comparison_operator_spec.rb
@@ -5,21 +5,22 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 describe Puppet::Parser::AST::ComparisonOperator do
   before :each do
     @scope = Puppet::Parser::Scope.new
-    @one = stub 'one', :safeevaluate => "1"
-    @two = stub 'two', :safeevaluate => "2"
+    @one = Puppet::Parser::AST::Leaf.new(:value => "1")
+    @two = Puppet::Parser::AST::Leaf.new(:value => "2")
+
+    @lval = Puppet::Parser::AST::Leaf.new(:value => "one")
+    @rval = Puppet::Parser::AST::Leaf.new(:value => "two")
   end
 
-  it "should evaluate both branches" do
-    lval = stub "lval"
-    lval.expects(:safeevaluate).with(@scope)
-    rval = stub "rval"
-    rval.expects(:safeevaluate).with(@scope)
+  it "should evaluate both values" do
+    @lval.expects(:safeevaluate).with(@scope)
+    @rval.expects(:safeevaluate).with(@scope)
 
-    operator = Puppet::Parser::AST::ComparisonOperator.new :lval => lval, :operator => "==", :rval => rval
+    operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @lval, :operator => "==", :rval => @rval
     operator.evaluate(@scope)
   end
 
-  it "should convert arguments strings to numbers if they are" do
+  it "should convert the arguments to numbers if they are numbers in string" do
     Puppet::Parser::Scope.expects(:number?).with("1").returns(1)
     Puppet::Parser::Scope.expects(:number?).with("2").returns(2)
 
@@ -27,25 +28,48 @@ describe Puppet::Parser::AST::ComparisonOperator do
     operator.evaluate(@scope)
   end
 
-  %w{< > <= >= ==}.each do |oper|
+  %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 = 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)
+  describe "with string comparison" do
+    it "should use matching" do
+      @rval.expects(:evaluate_match).with("one", @scope)
 
-    operator = Puppet::Parser::AST::ComparisonOperator.new :lval => lval, :operator => ">", :rval => rval
+      operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @lval, :operator => "==", :rval => @rval
+      operator.evaluate(@scope)
+    end
+
+    it "should return true for :undef to '' equality" do
+      astundef = Puppet::Parser::AST::Leaf.new(:value => :undef)
+      empty = Puppet::Parser::AST::Leaf.new(:value => '')
+
+      operator = Puppet::Parser::AST::ComparisonOperator.new :lval => astundef, :operator => "==", :rval => empty
+      operator.evaluate(@scope).should be_true
+    end
+
+    [true, false].each do |result|
+      it "should return #{(result).inspect} with '==' when matching return #{result.inspect}" do
+        @rval.expects(:evaluate_match).with("one", @scope).returns result
+
+        operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @lval, :operator => "==", :rval => @rval
+        operator.evaluate(@scope).should == result
+      end
+
+      it "should return #{(!result).inspect} with '!=' when matching return #{result.inspect}" do
+        @rval.expects(:evaluate_match).with("one", @scope).returns result
+
+        operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @lval, :operator => "!=", :rval => @rval
+        operator.evaluate(@scope).should == !result
+      end
+    end
+  end
+
+  it "should fail with arguments of different types" do
+    operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @one, :operator => ">", :rval => @rval
     lambda { operator.evaluate(@scope) }.should raise_error(ArgumentError)
   end
 
@@ -81,8 +105,8 @@ describe Puppet::Parser::AST::ComparisonOperator do
   # 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"
+      ten = Puppet::Parser::AST::Leaf.new(:value => "10")
+      nine = Puppet::Parser::AST::Leaf.new(:value => "9")
       operator = Puppet::Parser::AST::ComparisonOperator.new :lval => ten, :operator => oper, :rval => nine
 
       operator.evaluate(@scope).should == 10.send(oper,9)
diff --git a/spec/unit/parser/ast/leaf_spec.rb b/spec/unit/parser/ast/leaf_spec.rb
index 379cbfd..d21cbf5 100755
--- a/spec/unit/parser/ast/leaf_spec.rb
+++ b/spec/unit/parser/ast/leaf_spec.rb
@@ -35,6 +35,27 @@ describe Puppet::Parser::AST::Leaf do
       @leaf.evaluate_match("value", @scope)
     end
 
+    it "should convert values to number" do
+      @leaf.stubs(:safeevaluate).with(@scope).returns(@value)
+      Puppet::Parser::Scope.expects(:number?).with(@value).returns(2)
+      Puppet::Parser::Scope.expects(:number?).with("23").returns(23)
+
+      @leaf.evaluate_match("23", @scope)
+    end
+
+    it "should compare 'numberized' values" do
+      @leaf.stubs(:safeevaluate).with(@scope).returns(@value)
+      two = stub_everything 'two'
+      one = stub_everything 'one'
+
+      Puppet::Parser::Scope.stubs(:number?).with(@value).returns(one)
+      Puppet::Parser::Scope.stubs(:number?).with("2").returns(two)
+
+      one.expects(:==).with(two)
+
+      @leaf.evaluate_match("2", @scope)
+    end
+
     it "should match undef if value is an empty string" do
       @leaf.stubs(:safeevaluate).with(@scope).returns("")
 

-- 
Puppet packaging for Debian