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 - @leaf.expects(:safeevaluate).with(@scope) - - @leaf.evaluate_match("value", @scope) - end - - it "should match values by equality" do - @value.stubs(:==).returns(false) - @leaf.stubs(:safeevaluate).with(@scope).returns(@value) - @value.expects(:==).with("value") - - @leaf.evaluate_match("value", @scope) - end - - it "should downcase the evaluated value if wanted" do - @leaf.stubs(:safeevaluate).with(@scope).returns(@value) - @value.expects(:downcase).returns("value") - - @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("") - - @leaf.evaluate_match(:undef, @scope).should be_true - end - - it "should downcase the parameter value if wanted" do - parameter = stub 'parameter' - parameter.expects(:downcase).returns("value") - - @leaf.evaluate_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 + @evaluateable.expects(:safeevaluate).with(@scope) + + @evaluateable.evaluate_match("value", @scope) + end + + it "should match values by equality" do + @value.expects(:==).with("value").returns(true) + + @evaluateable.evaluate_match("value", @scope) + end + + it "should downcase the evaluated value if wanted" do + @value.expects(:downcase).returns("value") + + @evaluateable.evaluate_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) + + @evaluateable.evaluate_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) + + @evaluateable.evaluate_match("2", @scope) + end + + it "should match undef if value is an empty string" do + @evaluateable.value = '' + @evaluateable.evaluate_match(:undef, @scope).should be_true + end + + it "should downcase the parameter value if wanted" do + parameter = stub 'parameter' + parameter.expects(:downcase).returns("value") + + @evaluateable.evaluate_match(parameter, @scope) + end + + it "should not match '' if value is undef" do + @evaluateable.value = :undef + @evaluateable.evaluate_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.
