-1 We're looking for a minimal fix. That is, something restricted to lib/puppet/parser/ast/comparison_operator.rb and preferably restricted to tweaking the lines that introduced the bug:
+ case @operator + when "==","!=" + @rval.evaluate_match(lval, scope) ? @operator == '==' : @operator == '!=' + else On Mon, Jul 19, 2010 at 3:08 PM, Matt Robinson <[email protected]> wrote: > Ticket #4238 introduced a problem that a function couldn't compare to > another value until after it was evaluated, and AST::Function didn't have the > evaluate_match method. This change adds that method to AST::Function. > > To reduce code duplication we've moved evaluate_match into a module and > include it where needed, and tests are on a generic AST object instead > of the specific objects that include the module. > > The special casing necessary for doing comparisons between AST objects > feels messy and could probably be encapsulated better. I've created > ticket #4291 to remind us to refactor this at some point. > > Paired with: Nick Lewis > > Signed-off-by: Matt Robinson <[email protected]> > --- > lib/puppet/parser/ast.rb | 15 ++++++++ > lib/puppet/parser/ast/function.rb | 2 + > lib/puppet/parser/ast/leaf.rb | 15 +------- > spec/unit/parser/ast/leaf_spec.rb | 57 ----------------------------- > spec/unit/parser/ast_spec.rb | 71 > +++++++++++++++++++++++++++++++++++++ > 5 files changed, 89 insertions(+), 71 deletions(-) > > diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb > index 2773a24..7813be7 100644 > --- a/lib/puppet/parser/ast.rb > +++ b/lib/puppet/parser/ast.rb > @@ -8,6 +8,21 @@ require 'puppet/file_collection/lookup' > # Handles things like file name, line #, and also does the initialization > # for all of the parameters of all of the child objects. > class Puppet::Parser::AST > + module Puppet::Parser::AST::EvaluateMatch > + # evaluate ourselves, and match > + def evaluate_match(value, scope) > + obj = self.safeevaluate(scope) > + > + 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 > + end > # Do this so I don't have to type the full path in all of the subclasses > AST = Puppet::Parser::AST > > diff --git a/lib/puppet/parser/ast/function.rb > b/lib/puppet/parser/ast/function.rb > index 602016c..44944c3 100644 > --- a/lib/puppet/parser/ast/function.rb > +++ b/lib/puppet/parser/ast/function.rb > @@ -10,6 +10,8 @@ class Puppet::Parser::AST > > @settor = true > > + include AST::EvaluateMatch > + > def evaluate(scope) > > # Make sure it's a defined function > diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb > index 49f4302..950012f 100644 > --- a/lib/puppet/parser/ast/leaf.rb > +++ b/lib/puppet/parser/ast/leaf.rb > @@ -10,20 +10,6 @@ class Puppet::Parser::AST > @value > end > > - # evaluate ourselves, and match > - def evaluate_match(value, scope) > - obj = self.safeevaluate(scope) > - > - 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 > - > def match(value) > @value == value > end > @@ -31,6 +17,7 @@ class Puppet::Parser::AST > def to_s > @value.to_s unless @value.nil? > end > + include AST::EvaluateMatch > end > > # The boolean class. True or false. Converts the string it receives > diff --git a/spec/unit/parser/ast/leaf_spec.rb > b/spec/unit/parser/ast/leaf_spec.rb > index d21cbf5..6729cd2 100755 > --- a/spec/unit/parser/ast/leaf_spec.rb > +++ b/spec/unit/parser/ast/leaf_spec.rb > @@ -13,63 +13,6 @@ describe Puppet::Parser::AST::Leaf do > Puppet::Parser::AST::Leaf.new(:value => "value").should > respond_to(:evaluate_match) > end > > - describe "when evaluate_match is called" do > - it "should evaluate itself" do > - �[email protected](:safeevaluate).with(@scope) > - > - �[email protected]_match("value", @scope) > - end > - > - it "should match values by equality" do > - �[email protected](:==).returns(false) > - �[email protected](:safeevaluate).with(@scope).returns(@value) > - �[email protected](:==).with("value") > - > - �[email protected]_match("value", @scope) > - end > - > - it "should downcase the evaluated value if wanted" do > - �[email protected](:safeevaluate).with(@scope).returns(@value) > - �[email protected](:downcase).returns("value") > - > - �[email protected]_match("value", @scope) > - end > - > - it "should convert values to number" do > - �[email protected](:safeevaluate).with(@scope).returns(@value) > - Puppet::Parser::Scope.expects(:number?).with(@value).returns(2) > - Puppet::Parser::Scope.expects(:number?).with("23").returns(23) > - > - �[email protected]_match("23", @scope) > - end > - > - it "should compare 'numberized' values" do > - �[email protected](: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) > - > - �[email protected]_match("2", @scope) > - end > - > - it "should match undef if value is an empty string" do > - �[email protected](:safeevaluate).with(@scope).returns("") > - > - �[email protected]_match(:undef, @scope).should be_true > - end > - > - it "should downcase the parameter value if wanted" do > - parameter = stub 'parameter' > - parameter.expects(:downcase).returns("value") > - > - �[email protected]_match(parameter, @scope) > - end > - end > - > describe "when converting to string" do > it "should transform its value to string" do > value = stub 'value', :is_a? => true > diff --git a/spec/unit/parser/ast_spec.rb b/spec/unit/parser/ast_spec.rb > index b743cea..1285ad5 100644 > --- a/spec/unit/parser/ast_spec.rb > +++ b/spec/unit/parser/ast_spec.rb > @@ -39,3 +39,74 @@ describe Puppet::Parser::AST do > end > > end > + > +describe Puppet::Parser::AST::EvaluateMatch do > + before :each do > + �...@value = stub 'value' > + class Evaluateable < Puppet::Parser::AST > + attr_accessor :value > + include Puppet::Parser::AST::EvaluateMatch > + def safeevaluate(*options) > + return value > + end > + end > + �...@evaluateable = Evaluateable.new(:value => @value) > + �...@scope = stubs 'scope' > + end > + > + describe "when evaluate_match is called" do > + it "should evaluate itself" do > + �[email protected](:safeevaluate).with(@scope) > + > + �[email protected]_match("value", @scope) > + end > + > + it "should match values by equality" do > + �[email protected](:==).with("value").returns(true) > + > + �[email protected]_match("value", @scope) > + end > + > + it "should downcase the evaluated value if wanted" do > + �[email protected](:downcase).returns("value") > + > + �[email protected]_match("value", @scope) > + end > + > + it "should convert values to number" do > + Puppet::Parser::Scope.expects(:number?).with(@value).returns(2) > + Puppet::Parser::Scope.expects(:number?).with("23").returns(23) > + > + �[email protected]_match("23", @scope) > + end > + > + it "should compare 'numberized' values" do > + 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) > + > + �[email protected]_match("2", @scope) > + end > + > + it "should match undef if value is an empty string" do > + �[email protected] = '' > + �[email protected]_match(:undef, @scope).should be_true > + end > + > + it "should downcase the parameter value if wanted" do > + parameter = stub 'parameter' > + parameter.expects(:downcase).returns("value") > + > + �[email protected]_match(parameter, @scope) > + end > + > + it "should not match '' if value is undef" do > + �[email protected] = :undef > + �[email protected]_match('', @scope).should be_false > + end > + end > +end > -- > 1.7.1 > > -- > 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. > > -- ----------------------------------------------------------- The power of accurate observation is commonly called cynicism by those who have not got it. ~George Bernard Shaw ------------------------------------------------------------ -- 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.
