On Jul 3, 2009, at 12:17 AM, Brice Figureau wrote:

>
> +1, one cosmetic comment below
>
> On Thu, 2009-07-02 at 16:59 -0500, Luke Kanies wrote:
>> This class is extracted from the Parser class,
>> and the main driver for it is to enable us to put mutexes
>> around some of the hashes to see if they're the source
>> of a race condition.
>>
>> Signed-off-by: Luke Kanies <[email protected]>
>> ---
>> lib/puppet/parser/loaded_code.rb |   85 ++++++++++++++++++++++++++++ 
>> ++
>> spec/unit/parser/loaded_code.rb  |  106 ++++++++++++++++++++++++++++ 
>> ++++++++++
>> 2 files changed, 191 insertions(+), 0 deletions(-)
>> create mode 100644 lib/puppet/parser/loaded_code.rb
>> create mode 100644 spec/unit/parser/loaded_code.rb
>>
>> diff --git a/lib/puppet/parser/loaded_code.rb b/lib/puppet/parser/ 
>> loaded_code.rb
>> new file mode 100644
>> index 0000000..7c918d4
>> --- /dev/null
>> +++ b/lib/puppet/parser/loaded_code.rb
>> @@ -0,0 +1,85 @@
>> +class Puppet::Parser::LoadedCode
>> +    def initialize
>> +        @hostclasses = {}
>> +        @definitions = {}
>> +        @nodes = {}
>> +    end
>> +
>> +    def add_hostclass(name, code)
>> +        @hostclasses[munge_name(name)] = code
>> +    end
>> +
>> +    def hostclass(name)
>> +        @hostclasses[munge_name(name)]
>> +    end
>> +
>> +    def add_node(name, code)
>> +        @nodes[munge_name(name)] = code
>> +    end
>> +
>> +    def node(name)
>> +        @nodes[munge_name(name)]
>> +    end
>> +
>> +    def nodes?
>> +        @nodes.length > 0
>> +    end
>> +
>> +    def add_definition(name, code)
>> +        @definitions[munge_name(name)] = code
>> +    end
>> +
>> +    def definition(name)
>> +        @definitions[munge_name(name)]
>> +    end
>> +
>> +    def find(namespace, name, type)
>> +        if r = find_fully_qualified(name, type)
>> +            return r
>> +        end
>> +
>> +        ary = namespace.split("::")
>> +
>> +        while ary.length > 0
>> +            tmp_namespace = ary.join("::")
>> +            if r = find_partially_qualified(tmp_namespace, name,  
>> type)
>> +                return r
>> +            end
>> +
>> +            # Delete the second to last object, which reduces our  
>> namespace by one.
>> +            ary.pop
>> +        end
>> +
>> +        send(type, name)
>> +    end
>> +
>> +    def find_node(name)
>> +        find("", name, :node)
>> +    end
>> +
>> +    def find_hostclass(namespace, name)
>> +        find(namespace, name, :hostclass)
>> +    end
>> +
>> +    def find_definition(namespace, name)
>> +        find(namespace, name, :definition)
>> +    end
>> +
>> +    private
>> +
>> +    def find_fully_qualified(name, type)
>> +        return nil unless name =~ /^::/
>> +
>> +        name = name.sub(/^::/, '')
>> +
>> +        send(type, name)
>> +    end
>> +
>> +    def find_partially_qualified(namespace, name, type)
>> +        send(type, [namespace, name].join("::"))
>> +    end
>> +
>> +    def munge_name(name)
>> +        name.to_s.downcase
>> +    end
>> +end
>> diff --git a/spec/unit/parser/loaded_code.rb b/spec/unit/parser/ 
>> loaded_code.rb
>> new file mode 100644
>> index 0000000..50f6b93
>> --- /dev/null
>> +++ b/spec/unit/parser/loaded_code.rb
>> @@ -0,0 +1,106 @@
>> +#!/usr/bin/env ruby
>> +
>> +require File.dirname(__FILE__) + '/../../spec_helper'
>> +
>> +require 'puppet/parser/loaded_code'
>> +
>> +describe Puppet::Parser::LoadedCode do
>> +    %w{hostclass node definition}.each do |data|
>> +        it "should have a method for adding a #{data}" do
>> +            Puppet::Parser::LoadedCode.new.should  
>> respond_to("add_" + data)
>> +        end
>> +
>> +        it "should be able to retrieve #{data} by name" do
>> +            loader = Puppet::Parser::LoadedCode.new
>> +            loader.send("add_" + data, "foo", "bar")
>> +            loader.send(data, "foo").should == "bar"
>> +        end
>> +
>> +        it "should retrieve #{data} insensitive to case" do
>> +            loader = Puppet::Parser::LoadedCode.new
>> +            loader.send("add_" + data, "Foo", "bar")
>> +            loader.send(data, "fOo").should == "bar"
>> +        end
>> +
>> +        it "should return nil when asked for a #{data} that has  
>> not been added" do
>> +            Puppet::Parser::LoadedCode.new.send(data,  
>> "foo").should be_nil
>> +        end
>> +    end
>> +
>> +    describe "when finding a qualified instance" do
>> +        it "should return any found instance if the instance name  
>> is fully qualified" do
>> +            loader = Puppet::Parser::LoadedCode.new
>> +            loader.add_node "foo::bar", "yay"
>> +            loader.find("namespace", "::foo::bar", :node).should  
>> == "yay"
>> +        end
>> +
>> +        it "should return nil if the instance name is fully  
>> qualified and no such instance exists" do
>> +            loader = Puppet::Parser::LoadedCode.new
>> +            loader.find("namespace", "::foo::bar", :node).should  
>> be_nil
>> +        end
>> +
>> +        it "should return the partially qualified object if it  
>> exists in the provided namespace" do
>> +            loader = Puppet::Parser::LoadedCode.new
>> +            loader.add_node "foo::bar::baz", "yay"
>> +            loader.find("foo", "bar::baz", :node).should == "yay"
>> +        end
>> +
>> +        it "should return the unqualified object if it exists in  
>> the provided namespace" do
>> +            loader = Puppet::Parser::LoadedCode.new
>> +            loader.add_node "foo::bar", "yay"
>> +            loader.find("foo", "bar", :node).should == "yay"
>> +        end
>> +
>> +        it "should return the unqualified object if it exists in  
>> the parent namespace" do
>> +            loader = Puppet::Parser::LoadedCode.new
>> +            loader.add_node "foo::bar", "yay"
>> +            loader.find("foo::bar::baz", "bar", :node).should ==  
>> "yay"
>> +        end
>> +
>> +        it "should should return the partially qualified object if  
>> it exists in the parent namespace" do
>> +            loader = Puppet::Parser::LoadedCode.new
>> +            loader.add_node "foo::bar::baz", "yay"
>> +            loader.find("foo::bar", "bar::baz", :node).should ==  
>> "yay"
>> +        end
>> +
>> +        it "should return the qualified object if it exists in the  
>> root namespace" do
>> +            loader = Puppet::Parser::LoadedCode.new
>> +            loader.add_node "foo::bar::baz", "yay"
>> +            loader.find("foo::bar", "foo::bar::baz", :node).should  
>> == "yay"
>> +        end
>> +
>> +        it "should return nil if the object cannot be found" do
>> +            loader = Puppet::Parser::LoadedCode.new
>> +            loader.add_node "foo::bar::baz", "yay"
>> +            loader.find("foo::bar", "eh", :node).should be_nil
>> +        end
>> +    end
>> +
>> +    it "should use the generic 'find' method with an empty  
>> namespace to find nodes" do
>> +        loader = Puppet::Parser::LoadedCode.new
>> +        loader.expects(:find).with("", "bar", :node)
>> +        loader.find_node("bar")
>> +    end
>> +
>> +    it "should use the generic 'find' method to find hostclasses" do
>> +        loader = Puppet::Parser::LoadedCode.new
>> +        loader.expects(:find).with("foo", "bar", :hostclass)
>> +        loader.find_hostclass("foo", "bar")
>> +    end
>> +
>> +    it "should use the generic 'find' method to find definitions" do
>> +        loader = Puppet::Parser::LoadedCode.new
>> +        loader.expects(:find).with("foo", "bar", :definition)
>> +        loader.find_definition("foo", "bar")
>> +    end
>> +
>> +    it "should indicate whether any nodes are defined" do
>> +        loader = Puppet::Parser::LoadedCode.new
>> +        loader.add_node("foo", "bar")
>> +        loader.should be_nodes
>> +    end
>> +
>> +    it "should indicate whether no nodes are defined" do
>> +        Puppet::Parser::LoadedCode.new.should_not be_nodes
>> +    end
>> +end
>
> Given the number of tests using
> loader = Puppet::Parser::LoadedCode.new
>
> You should add a before :each block to these specs.


'Course, this has already been merged, but...

I don't exactly have a standard for this.  Doing so would save some  
lines of code, but it's a trivial line of code and sometimes, for  
probably arbitrary reasons, I don't include the initialization in a  
'before' block.  Too much Jay Fields, I think.

It does look like every single test has a LoadedCode.new, though,  
which is a bit excessive.  I think when I first wrote this that wasn't  
the case, and when I changed it I didn't go back and fixed it.

Already been merged, though, so not really worth changing now.  Feel  
free to fix it in the commit series you're building on top of this.

-- 
I don't want the world, I just want your half.
---------------------------------------------------------------------
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
-~----------~----~----~----~------~----~------~--~---

Reply via email to