From 5aa4440b6fb8c9199ee549bd8fe0e4afb296c259 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 11 Sep 2007 12:38:56 -0500
Subject: Doing an intermediate commit so rick can look at the work I have done
 so far.

---
 spec/unit/node/facts.rb |  25 +++++++++++
 spec/unit/node/node.rb  | 116 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 141 insertions(+)
 create mode 100755 spec/unit/node/facts.rb
 create mode 100755 spec/unit/node/node.rb

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/facts.rb b/spec/unit/node/facts.rb
new file mode 100755
index 000000000..c7fc65f38
--- /dev/null
+++ b/spec/unit/node/facts.rb
@@ -0,0 +1,25 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/node/facts'
+
+describe Puppet::Node::Facts, " when indirecting" do
+    before do
+        Puppet[:fact_store] = "test_store"
+        @terminus_class = mock 'terminus_class'
+        @terminus = mock 'terminus'
+        @terminus_class.expects(:new).returns(@terminus)
+        Puppet::Indirector.expects(:terminus).with(:facts, :test_store).returns(@terminus_class)
+    end
+
+    it "should redirect to the specified fact store for retrieval" do
+        @terminus.expects(:get).with(:my_facts)
+        Puppet::Node::Facts.get(:my_facts)
+    end
+
+    it "should redirect to the specified fact store for storage" do
+        @terminus.expects(:put).with(:my_facts)
+        Puppet::Node::Facts.put(:my_facts)
+    end
+end
diff --git a/spec/unit/node/node.rb b/spec/unit/node/node.rb
new file mode 100755
index 000000000..a6cc1e301
--- /dev/null
+++ b/spec/unit/node/node.rb
@@ -0,0 +1,116 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+describe Puppet::Node, " when initializing" do
+    before do
+        @node = Puppet::Node.new("testnode")
+    end
+
+    it "should set the node name" do
+        @node.name.should == "testnode"
+    end
+
+    it "should default to an empty parameter hash" do
+        @node.parameters.should == {}
+    end
+
+    it "should default to an empty class array" do
+        @node.classes.should == []
+    end
+
+    it "should note its creation time" do
+        @node.time.should be_instance_of(Time)
+    end
+
+    it "should accept parameters passed in during initialization" do
+        params = {"a" => "b"}
+        @node = Puppet::Node.new("testing", :parameters => params)
+        @node.parameters.should == params
+    end
+
+    it "should accept classes passed in during initialization" do
+        classes = %w{one two}
+        @node = Puppet::Node.new("testing", :classes => classes)
+        @node.classes.should == classes
+    end
+
+    it "should always return classes as an array" do
+        @node = Puppet::Node.new("testing", :classes => "myclass")
+        @node.classes.should == ["myclass"]
+    end
+
+    it "should accept the environment during initialization" do
+        @node = Puppet::Node.new("testing", :environment => "myenv")
+        @node.environment.should == "myenv"
+    end
+
+    it "should accept names passed in" do
+        @node = Puppet::Node.new("testing", :names => ["myenv"])
+        @node.names.should == ["myenv"]
+    end
+end
+
+describe Puppet::Node, " when returning the environment" do
+    before do
+        @node = Puppet::Node.new("testnode")
+    end
+
+    it "should return the 'environment' fact if present and there is no explicit environment" do
+        @node.parameters = {"environment" => "myenv"}
+        @node.environment.should == "myenv"
+    end
+
+    it "should return the central environment if there is no environment fact nor explicit environment" do
+        Puppet.config.expects(:[]).with(:environment).returns(:centralenv)
+        @node.environment.should == :centralenv
+    end
+
+    it "should not use an explicit environment that is an empty string" do
+        @node.environment == ""
+        @node.environment.should be_nil
+    end
+
+    it "should not use an environment fact that is an empty string" do
+        @node.parameters = {"environment" => ""}
+        @node.environment.should be_nil
+    end
+
+    it "should not use an explicit environment that is an empty string" do
+        Puppet.config.expects(:[]).with(:environment).returns(nil)
+        @node.environment.should be_nil
+    end
+end
+
+describe Puppet::Node, " when merging facts" do
+    before do
+        @node = Puppet::Node.new("testnode")
+    end
+
+    it "should prefer parameters already set on the node over facts from the node" do
+        @node.parameters = {"one" => "a"}
+        @node.fact_merge("one" => "c")
+        @node.parameters["one"].should == "a"
+    end
+
+    it "should add passed parameters to the parameter list" do
+        @node.parameters = {"one" => "a"}
+        @node.fact_merge("two" => "b")
+        @node.parameters["two"].should == "b"
+    end
+end
+
+describe Puppet::Node, " when indirecting" do
+    before do
+        Puppet[:node_source] = :test_source
+        @terminus_class = mock 'terminus_class'
+        @terminus = mock 'terminus'
+        @terminus_class.expects(:new).returns(@terminus)
+        Puppet::Indirector.expects(:terminus).with(:node, :test_source).returns(@terminus_class)
+    end
+
+    it "should redirect to the specified node source" do
+        @terminus.expects(:get).with(:my_node)
+        Puppet::Node.get(:my_node)
+    end
+end
-- 
cgit 


From 65c1501504dd7e9166176661f9ed9f80300954db Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 11 Sep 2007 16:19:03 -0500
Subject: The Node handler is now obsolete.  Node searching is handled through
 the indirector.  I have not yet added the tests for the node handlers
 themselves, which is next.

---
 spec/unit/node/searching.rb | 79 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100755 spec/unit/node/searching.rb

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/searching.rb b/spec/unit/node/searching.rb
new file mode 100755
index 000000000..553822576
--- /dev/null
+++ b/spec/unit/node/searching.rb
@@ -0,0 +1,79 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+require 'puppet/node/searching'
+require 'puppet/node/facts'
+
+describe Puppet::Node::Searching, " when searching for nodes" do
+    before do
+        @searcher = Object.new
+        @searcher.extend(Puppet::Node::Searching)
+        @facts = Puppet::Node::Facts.new("foo", "hostname" => "yay", "domain" => "domain.com")
+        @node = Puppet::Node.new("foo")
+        Puppet::Node::Facts.stubs(:get).with("foo").returns(@facts)
+    end
+
+    it "should search for the node by its key first" do
+        names = []
+        @searcher.expects(:get).with do |name|
+            names << name
+            names == %w{foo}
+        end.returns(@node)
+        @searcher.search("foo").should equal(@node)
+    end
+
+    it "should return the first node found using the generated list of names" do
+        names = []
+        @searcher.expects(:get).with("foo").returns(nil)
+        @searcher.expects(:get).with("yay.domain.com").returns(@node)
+        @searcher.search("foo").should equal(@node)
+    end
+
+    it "should search for the rest of the names inversely by length" do
+        names = []
+        @facts.values["fqdn"] = "longer.than.the.normal.fqdn.com"
+        @searcher.stubs(:get).with do |name|
+            names << name
+        end
+        @searcher.search("foo")
+        # Strip off the key
+        names.shift
+
+        # And the 'default'
+        names.pop
+
+        length = 100
+        names.each do |name|
+            (name.length < length).should be_true
+            length = name.length
+        end
+    end
+
+    it "should attempt to find a default node if no names are found" do
+        names = []
+        @searcher.stubs(:get).with do |name|
+            names << name
+        end.returns(nil)
+        @searcher.search("foo")
+        names[-1].should == "default"
+    end
+
+    it "should cache the nodes" do
+        @searcher.expects(:get).with("foo").returns(@node)
+        @searcher.search("foo").should equal(@node)
+        @searcher.search("foo").should equal(@node)
+    end
+
+    it "should flush the node cache using the :filetimeout parameter" do
+        node2 = Puppet::Node.new("foo2")
+        Puppet[:filetimeout] = -1
+        # I couldn't get this to work with :expects
+        @searcher.stubs(:get).returns(@node, node2).then.raises(ArgumentError)
+        @searcher.search("foo").should equal(@node)
+        @searcher.search("foo").should equal(node2)
+    end
+
+    after do
+        Puppet.config.clear
+    end
+end
-- 
cgit 


From a6fe70054f4fb3efe4d558ffdd244917ca1c6f9c Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Wed, 12 Sep 2007 15:32:25 -0500
Subject: Another intermediate commit.  The node and fact classes are now
 functional and are used instead of the network handlers, which have been
 removed.  There are some failing tests as a result, but I want to get this
 code committed before I massage the rest of the system to make it work again.

---
 spec/unit/node/facts.rb | 25 +++++++++++++++++++------
 spec/unit/node/node.rb  | 20 ++++++++++++++------
 2 files changed, 33 insertions(+), 12 deletions(-)

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/facts.rb b/spec/unit/node/facts.rb
index c7fc65f38..c677c1d2e 100755
--- a/spec/unit/node/facts.rb
+++ b/spec/unit/node/facts.rb
@@ -6,11 +6,12 @@ require 'puppet/node/facts'
 
 describe Puppet::Node::Facts, " when indirecting" do
     before do
-        Puppet[:fact_store] = "test_store"
-        @terminus_class = mock 'terminus_class'
         @terminus = mock 'terminus'
-        @terminus_class.expects(:new).returns(@terminus)
-        Puppet::Indirector.expects(:terminus).with(:facts, :test_store).returns(@terminus_class)
+        Puppet::Indirector.terminus(:facts, Puppet[:fact_store].intern).stubs(:new).returns(@terminus)
+
+        # We have to clear the cache so that the facts ask for our terminus stub,
+        # instead of anything that might be cached.
+        Puppet::Indirector::Indirection.clear_cache
     end
 
     it "should redirect to the specified fact store for retrieval" do
@@ -19,7 +20,19 @@ describe Puppet::Node::Facts, " when indirecting" do
     end
 
     it "should redirect to the specified fact store for storage" do
-        @terminus.expects(:put).with(:my_facts)
-        Puppet::Node::Facts.put(:my_facts)
+        @terminus.expects(:post).with(:my_facts)
+        Puppet::Node::Facts.post(:my_facts)
+    end
+
+    after do
+        mocha_verify
+        Puppet::Indirector::Indirection.clear_cache
+    end
+end
+
+describe Puppet::Node::Facts, " when storing and retrieving" do
+    it "should add metadata to the facts" do
+        facts = Puppet::Node::Facts.new("me", "one" => "two", "three" => "four")
+        facts.values[:_timestamp].should be_instance_of(Time)
     end
 end
diff --git a/spec/unit/node/node.rb b/spec/unit/node/node.rb
index a6cc1e301..9342dc5ce 100755
--- a/spec/unit/node/node.rb
+++ b/spec/unit/node/node.rb
@@ -85,32 +85,40 @@ end
 describe Puppet::Node, " when merging facts" do
     before do
         @node = Puppet::Node.new("testnode")
+        Puppet::Node::Facts.stubs(:get).with(@node.name).returns(Puppet::Node::Facts.new(@node.name, "one" => "c", "two" => "b"))
     end
 
     it "should prefer parameters already set on the node over facts from the node" do
         @node.parameters = {"one" => "a"}
-        @node.fact_merge("one" => "c")
+        @node.fact_merge
         @node.parameters["one"].should == "a"
     end
 
     it "should add passed parameters to the parameter list" do
         @node.parameters = {"one" => "a"}
-        @node.fact_merge("two" => "b")
+        @node.fact_merge
         @node.parameters["two"].should == "b"
     end
+
+    it "should accept arbitrary parameters to merge into its parameters" do
+        @node.parameters = {"one" => "a"}
+        @node.merge "two" => "three"
+        @node.parameters["two"].should == "three"
+    end
 end
 
 describe Puppet::Node, " when indirecting" do
     before do
-        Puppet[:node_source] = :test_source
-        @terminus_class = mock 'terminus_class'
         @terminus = mock 'terminus'
-        @terminus_class.expects(:new).returns(@terminus)
-        Puppet::Indirector.expects(:terminus).with(:node, :test_source).returns(@terminus_class)
+        Puppet::Indirector.terminus(:node, Puppet[:node_source]).stubs(:new).returns(@terminus)
     end
 
     it "should redirect to the specified node source" do
         @terminus.expects(:get).with(:my_node)
         Puppet::Node.get(:my_node)
     end
+
+    after do
+        Puppet::Indirector::Indirection.clear_cache
+    end
 end
-- 
cgit 


From 3632926089cb27b93ff075c05ba21e2340a562ac Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Wed, 12 Sep 2007 18:40:54 -0500
Subject: Moving the resource container behaviour to the Configuration object,
 rather than the base PGraph class.  I expect I will just do away with PGraph,
 but for now, I am at least going to keep configuration-related code in that
 class.

---
 spec/unit/node/configuration.rb | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb
index 4429fe3a3..774a1550f 100755
--- a/spec/unit/node/configuration.rb
+++ b/spec/unit/node/configuration.rb
@@ -133,3 +133,26 @@ describe Puppet::Node::Configuration, " when extracting transobjects" do
         botarray.include?(:botres).should be_true
     end
 end
+
+describe Puppet::Node::Configuration, " functioning as a resource container" do
+    before do
+        @graph = Puppet::Node::Configuration.new("host")
+        @one = stub 'resource1', :ref => "Me[you]"
+        @two = stub 'resource2', :ref => "Me[him]"
+        @dupe = stub 'resource3', :ref => "Me[you]"
+    end
+
+    it "should make all vertices available by resource reference" do
+        @graph.add_resource(@one)
+        @graph.resource(@one.ref).should equal(@one)
+    end
+
+    it "should not allow two resources with the same resource reference" do
+        @graph.add_resource(@one)
+        proc { @graph.add_resource(@dupe) }.should raise_error(ArgumentError)
+    end
+
+    it "should not store objects that do not respond to :ref" do
+        proc { @graph.add_resource("thing") }.should raise_error(ArgumentError)
+    end
+end
-- 
cgit 


From f17f19dae941b17a56c1fc83ed3a89712b98c427 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Sat, 15 Sep 2007 22:17:20 -0600
Subject: The whole system now uses Configuration objects instead of ever
 converting the Transportable objects into a tree of components and then
 converting that into a graph.  This is a significant step, and drastically
 simplifies the model of how to use a configuration. The old code might have
 looked something like this:

    file = Puppet::Type.create :path => "/whatever", ...
    comp = Puppet::Type.create :name => :whatever
    comp.push file
    transaction = comp.evaluate
    transaction.evaluate

The new code looks like this:

    file = Puppet::Type.create :path => "/whatever", ...
    config = Puppet::Node::Configuration.new
    config.add_resource file
    config.apply

I did not really intend to do this much refactoring, but I
found I could not use a Configuration object to do work
without refactoring a lot of the system.  The primary problem
was that the Client::Master and the Config classes determined
how the transactions behaved; when I moved to using a Configuration,
this distinction was lost, which meant that configurations were
often needing to create other configurations, which resulted in
a whole lot of infinite recursion (e.g., Config objects that create
directories for Puppet use Configuration objects -- yes, I'm
s/Config/Settings/g soon -- and these Configuration objects would
need to create directories).

Not everything is fixed, but it's very close.  I am clearly over
the hump, though, so I wanted to get a commit in.
---
 spec/unit/node/configuration.rb | 147 ++++++++++++++++++++++++++++++++++++++--
 spec/unit/node/node.rb          |   4 ++
 2 files changed, 144 insertions(+), 7 deletions(-)

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb
index 774a1550f..3c30d9b3e 100755
--- a/spec/unit/node/configuration.rb
+++ b/spec/unit/node/configuration.rb
@@ -134,25 +134,158 @@ describe Puppet::Node::Configuration, " when extracting transobjects" do
     end
 end
 
-describe Puppet::Node::Configuration, " functioning as a resource container" do
+describe Puppet::Node::Configuration, " when functioning as a resource container" do
     before do
-        @graph = Puppet::Node::Configuration.new("host")
+        @config = Puppet::Node::Configuration.new("host")
         @one = stub 'resource1', :ref => "Me[you]"
         @two = stub 'resource2', :ref => "Me[him]"
         @dupe = stub 'resource3', :ref => "Me[you]"
     end
 
     it "should make all vertices available by resource reference" do
-        @graph.add_resource(@one)
-        @graph.resource(@one.ref).should equal(@one)
+        @config.add_resource(@one)
+        @config.resource(@one.ref).should equal(@one)
+        @config.vertices.find { |r| r.ref == @one.ref }.should equal(@one)
     end
 
     it "should not allow two resources with the same resource reference" do
-        @graph.add_resource(@one)
-        proc { @graph.add_resource(@dupe) }.should raise_error(ArgumentError)
+        @config.add_resource(@one)
+        proc { @config.add_resource(@dupe) }.should raise_error(ArgumentError)
     end
 
     it "should not store objects that do not respond to :ref" do
-        proc { @graph.add_resource("thing") }.should raise_error(ArgumentError)
+        proc { @config.add_resource("thing") }.should raise_error(ArgumentError)
     end
+
+    it "should remove all resources when asked" do
+        @config.add_resource @one
+        @config.add_resource @two
+        @one.expects :remove
+        @two.expects :remove
+        @config.clear(true)
+    end
+
+    it "should support a mechanism for finishing resources" do
+        @one.expects :finish
+        @two.expects :finish
+        @config.add_resource @one
+        @config.add_resource @two
+
+        @config.finalize
+    end
+    
+    it "should optionally support an initialization block and should finalize after such blocks" do
+        @one.expects :finish
+        @two.expects :finish
+        config = Puppet::Node::Configuration.new("host") do |conf|
+            conf.add_resource @one
+            conf.add_resource @two
+        end
+    end
+end
+
+module ApplyingConfigurations
+    def setup
+        @config = Puppet::Node::Configuration.new("host")
+
+        @config.retrieval_duration = Time.now
+        @transaction = mock 'transaction'
+        Puppet::Transaction.stubs(:new).returns(@transaction)
+        @transaction.stubs(:evaluate)
+        @transaction.stubs(:cleanup)
+        @transaction.stubs(:addtimes)
+    end
+end
+
+describe Puppet::Node::Configuration, " when applying" do
+    include ApplyingConfigurations
+
+    it "should create and evaluate a transaction" do
+        @transaction.expects(:evaluate)
+        @config.apply
+    end
+
+    it "should provide the configuration time to the transaction" do
+        @transaction.expects(:addtimes).with do |arg|
+            arg[:config_retrieval].should be_instance_of(Time)
+            true
+        end
+        @config.apply
+    end
+
+    it "should clean up the transaction" do
+        @transaction.expects :cleanup
+        @config.apply
+    end
+    
+    it "should return the transaction" do
+        @config.apply.should equal(@transaction)
+    end
+
+    it "should yield the transaction if a block is provided" do
+        pending "the code works but is not tested"
+    end
+    
+    it "should default to not being a host configuration" do
+        @config.host_config.should be_nil
+    end
+end
+
+describe Puppet::Node::Configuration, " when applying host configurations" do
+    include ApplyingConfigurations
+
+    # super() doesn't work in the setup method for some reason
+    before do
+        @config.host_config = true
+    end
+
+    it "should send a report if reporting is enabled" do
+        Puppet[:report] = true
+        @transaction.expects :send_report
+        @config.apply
+    end
+
+    it "should send a report if report summaries are enabled" do
+        Puppet[:summarize] = true
+        @transaction.expects :send_report
+        @config.apply
+    end
+
+    it "should initialize the state database before applying a configuration" do
+        Puppet::Util::Storage.expects(:load)
+
+        # Short-circuit the apply, so we know we're loading before the transaction
+        Puppet::Transaction.expects(:new).raises ArgumentError
+        proc { @config.apply }.should raise_error(ArgumentError)
+    end
+
+    it "should sync the state database after applying" do
+        Puppet::Util::Storage.expects(:store)
+        @config.apply
+    end
+
+    after { Puppet.config.clear }
+end
+
+describe Puppet::Node::Configuration, " when applying non-host configurations" do
+    include ApplyingConfigurations
+
+    before do
+        @config.host_config = false
+    end
+    
+    it "should never send reports" do
+        Puppet[:report] = true
+        Puppet[:summarize] = true
+        @transaction.expects(:send_report).never
+        @config.apply
+    end
+
+    it "should never modify the state database" do
+        Puppet::Util::Storage.expects(:load).never
+        Puppet::Util::Storage.expects(:store).never
+        @config.apply
+    end
+
+    after { Puppet.config.clear }
 end
diff --git a/spec/unit/node/node.rb b/spec/unit/node/node.rb
index 9342dc5ce..2f63c253d 100755
--- a/spec/unit/node/node.rb
+++ b/spec/unit/node/node.rb
@@ -11,6 +11,10 @@ describe Puppet::Node, " when initializing" do
         @node.name.should == "testnode"
     end
 
+    it "should not allow nil node names" do
+        proc { Puppet::Node.new(nil) }.should raise_error(ArgumentError)
+    end
+
     it "should default to an empty parameter hash" do
         @node.parameters.should == {}
     end
-- 
cgit 


From b3c8cdb67d9a423a1d14764f1e58f677d7ef8d41 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Sat, 15 Sep 2007 22:25:54 -0600
Subject: Configurations now set a "configuration" instance variable in
 resources that are inside a configuration, so the resources can interact with
 the configuration to get things like relationships.

---
 spec/unit/node/configuration.rb | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb
index 3c30d9b3e..071afc041 100755
--- a/spec/unit/node/configuration.rb
+++ b/spec/unit/node/configuration.rb
@@ -137,9 +137,9 @@ end
 describe Puppet::Node::Configuration, " when functioning as a resource container" do
     before do
         @config = Puppet::Node::Configuration.new("host")
-        @one = stub 'resource1', :ref => "Me[you]"
-        @two = stub 'resource2', :ref => "Me[him]"
-        @dupe = stub 'resource3', :ref => "Me[you]"
+        @one = stub 'resource1', :ref => "Me[you]", :configuration= => nil
+        @two = stub 'resource2', :ref => "Me[him]", :configuration= => nil
+        @dupe = stub 'resource3', :ref => "Me[you]", :configuration= => nil
     end
 
     it "should make all vertices available by resource reference" do
@@ -182,6 +182,11 @@ describe Puppet::Node::Configuration, " when functioning as a resource container
             conf.add_resource @two
         end
     end
+
+    it "should inform the resource that it is the resource's configuration" do
+        @one.expects(:configuration=).with(@config)
+        @config.add_resource @one
+    end
 end
 
 module ApplyingConfigurations
@@ -223,7 +228,9 @@ describe Puppet::Node::Configuration, " when applying" do
     end
 
     it "should yield the transaction if a block is provided" do
-        pending "the code works but is not tested"
+        @config.apply do |trans|
+            trans.should equal(@transaction)
+        end
     end
     
     it "should default to not being a host configuration" do
-- 
cgit 


From 9fa2628a844c75b8f554f283dfece01667f20594 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Mon, 17 Sep 2007 15:21:44 -0700
Subject: This is basically another intermediate commit.  I feel like I've gone
 too far down the rabbit hole to turn back now, but the code is clearly
 getting more centralized around the Configuration class, which is the goal.

Things are currently a bit muddy between recursion, dynamic resource
generation, transactions, and the configuration, and I don't expect
to be able to clear it up much until we rewrite all of the tests
for the Transaction class, since that is when we'll actually be
setting its behaviour.

At this point, Files (which are currently the only resources that
generate other resources) are responsible for adding their edges
to the relationship graph.  This puts them knowing more than I would
like about how the relationship graph works, but it'll have to do for now.

There are still failing tests, but files seem to work again.  Now to
go through the rest of the tests and make them work.
---
 spec/unit/node/configuration.rb | 153 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 153 insertions(+)

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb
index 071afc041..ecf311948 100755
--- a/spec/unit/node/configuration.rb
+++ b/spec/unit/node/configuration.rb
@@ -142,6 +142,12 @@ describe Puppet::Node::Configuration, " when functioning as a resource container
         @dupe = stub 'resource3', :ref => "Me[you]", :configuration= => nil
     end
 
+    it "should provide a method to add one or more resources" do
+        @config.add_resource @one, @two
+        @config.resource(@one.ref).should equal(@one)
+        @config.resource(@two.ref).should equal(@two)
+    end
+
     it "should make all vertices available by resource reference" do
         @config.add_resource(@one)
         @config.resource(@one.ref).should equal(@one)
@@ -187,6 +193,24 @@ describe Puppet::Node::Configuration, " when functioning as a resource container
         @one.expects(:configuration=).with(@config)
         @config.add_resource @one
     end
+
+    it "should be able to find resources by reference" do
+        @config.add_resource @one
+        @config.resource(@one.ref).should equal(@one)
+    end
+
+    it "should be able to find resources by reference or by type/title tuple" do
+        @config.add_resource @one
+        @config.resource("me", "you").should equal(@one)
+    end
+
+    it "should have a mechanism for removing resources" do
+        @config.add_resource @one
+        @one.expects :remove
+        @config.remove_resource(@one)
+        @config.resource(@one.ref).should be_nil
+        @config.vertex?(@one).should be_false
+    end
 end
 
 module ApplyingConfigurations
@@ -296,3 +320,132 @@ describe Puppet::Node::Configuration, " when applying non-host configurations" d
 
     after { Puppet.config.clear }
 end
+
+describe Puppet::Node::Configuration, " when creating a relationship graph" do
+    before do
+        @config = Puppet::Node::Configuration.new("host")
+        @compone = Puppet::Type::Component.create :name => "one"
+        @comptwo = Puppet::Type::Component.create :name => "two", :require => ["class", "one"]
+        @file = Puppet::Type.type(:file)
+        @one = @file.create :path => "/one"
+        @two = @file.create :path => "/two"
+        @config.add_edge! @compone, @one
+        @config.add_edge! @comptwo, @two
+
+        @three = @file.create :path => "/three"
+        @four = @file.create :path => "/four", :require => ["file", "/three"]
+        @config.add_resource @compone, @comptwo, @one, @two, @three, @four
+        @relationships = @config.relationship_graph
+    end
+
+    it "should be able to create a resource graph" do
+        @relationships.should be_instance_of(Puppet::Node::Configuration)
+    end
+
+    it "should not have any components" do
+        @relationships.vertices.find { |r| r.instance_of?(Puppet::Type::Component) }.should be_nil
+    end
+
+    it "should have all non-component resources from the configuration" do
+        # The failures print out too much info, so i just do a class comparison
+        @relationships.resource(@one.ref).should be_instance_of(@one.class)
+        @relationships.resource(@three.ref).should be_instance_of(@three.class)
+    end
+
+    it "should have all resource relationships set as edges" do
+        @relationships.edge?(@three, @four).should be_true
+    end
+
+    it "should copy component relationships to all contained resources" do
+        @relationships.edge?(@one, @two).should be_true
+    end
+
+    it "should get removed when the configuration is cleaned up" do
+        @relationships.expects(:clear).with(false)
+        @config.clear
+        @config.instance_variable_get("@relationship_graph").should be_nil
+    end
+
+    it "should create a new relationship graph after clearing the old one" do
+        @relationships.expects(:clear).with(false)
+        @config.clear
+        @config.relationship_graph.should be_instance_of(Puppet::Node::Configuration)
+    end
+
+    it "should look up resources in the relationship graph if not found in the main configuration" do
+        five = stub 'five', :ref => "File[five]", :configuration= => nil
+        @relationships.add_resource five
+        @config.resource(five.ref).should equal(five)
+    end
+
+    it "should provide a method to create additional resources that also registers the resource" do
+        args = {:name => "/yay", :ensure => :file}
+        resource = stub 'file', :ref => "File[/yay]", :configuration= => @config
+        Puppet::Type.type(:file).expects(:create).with(args).returns(resource)
+        @config.create_resource :file, args
+        @config.resource("File[/yay]").should equal(resource)
+    end
+
+    it "should provide a mechanism for creating implicit resources" do
+        args = {:name => "/yay", :ensure => :file}
+        resource = stub 'file', :ref => "File[/yay]", :configuration= => @config
+        Puppet::Type.type(:file).expects(:create).with(args).returns(resource)
+        resource.expects(:implicit=).with(true)
+        @config.create_implicit_resource :file, args
+        @config.resource("File[/yay]").should equal(resource)
+    end
+
+    it "should remove resources created mid-transaction" do
+        args = {:name => "/yay", :ensure => :file}
+        resource = stub 'file', :ref => "File[/yay]", :configuration= => @config
+        @transaction = mock 'transaction'
+        Puppet::Transaction.stubs(:new).returns(@transaction)
+        @transaction.stubs(:evaluate)
+        @transaction.stubs(:cleanup)
+        @transaction.stubs(:addtimes)
+        Puppet::Type.type(:file).expects(:create).with(args).returns(resource)
+        resource.expects :remove
+        @config.apply do |trans|
+            @config.create_resource :file, args
+            @config.resource("File[/yay]").should equal(resource)
+        end
+        @config.resource("File[/yay]").should be_nil
+    end
+
+    after do
+        Puppet::Type.allclear
+    end
+end
+
+describe Puppet::Node::Configuration, " when writing dot files" do
+    before do
+        @config = Puppet::Node::Configuration.new("host")
+        @name = :test
+        @file = File.join(Puppet[:graphdir], @name.to_s + ".dot")
+    end
+    it "should only write when it is a host configuration" do
+        File.expects(:open).with(@file).never
+        @config.host_config = false
+        Puppet[:graph] = true
+        @config.write_graph(@name)
+    end
+
+    it "should only write when graphing is enabled" do
+        File.expects(:open).with(@file).never
+        @config.host_config = true
+        Puppet[:graph] = false
+        @config.write_graph(@name)
+    end
+
+    it "should write a dot file based on the passed name" do
+        File.expects(:open).with(@file, "w").yields(stub("file", :puts => nil))
+        @config.expects(:to_dot).with("name" => @name.to_s.capitalize)
+        @config.host_config = true
+        Puppet[:graph] = true
+        @config.write_graph(@name)
+    end
+
+    after do
+        Puppet.config.clear
+    end
+end
-- 
cgit 


From 46d69068fa7b2f3448294c5d3da21c69cef73d2f Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Thu, 20 Sep 2007 12:57:57 -0500
Subject: An intermediate commit so I can start working on a different branch. 
 The file recursion code actually works for the first time in a painful while,
 but there are still some quirks and design issues to resolve, particularly
 around creating implicit resources that then fail (i.e., the behaviour of the
 create_implicit_resource method in Configuration).

---
 spec/unit/node/configuration.rb | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb
index ecf311948..eb4eaa418 100755
--- a/spec/unit/node/configuration.rb
+++ b/spec/unit/node/configuration.rb
@@ -334,22 +334,28 @@ describe Puppet::Node::Configuration, " when creating a relationship graph" do
 
         @three = @file.create :path => "/three"
         @four = @file.create :path => "/four", :require => ["file", "/three"]
-        @config.add_resource @compone, @comptwo, @one, @two, @three, @four
+        @five = @file.create :path => "/five"
+        @config.add_resource @compone, @comptwo, @one, @two, @three, @four, @five
         @relationships = @config.relationship_graph
     end
 
-    it "should be able to create a resource graph" do
+    it "should be able to create a relationship graph" do
         @relationships.should be_instance_of(Puppet::Node::Configuration)
     end
 
+    it "should copy its host_config setting to the relationship graph" do
+        config = Puppet::Node::Configuration.new
+        config.host_config = true
+        config.relationship_graph.host_config.should be_true
+    end
+
     it "should not have any components" do
         @relationships.vertices.find { |r| r.instance_of?(Puppet::Type::Component) }.should be_nil
     end
 
     it "should have all non-component resources from the configuration" do
         # The failures print out too much info, so i just do a class comparison
-        @relationships.resource(@one.ref).should be_instance_of(@one.class)
-        @relationships.resource(@three.ref).should be_instance_of(@three.class)
+        @relationships.vertex?(@five).should be_true
     end
 
     it "should have all resource relationships set as edges" do
@@ -412,6 +418,11 @@ describe Puppet::Node::Configuration, " when creating a relationship graph" do
         @config.resource("File[/yay]").should be_nil
     end
 
+    it "should remove resources from the relationship graph if it exists" do
+        @config.remove_resource(@one)
+        @config.relationship_graph.vertex?(@one).should be_false
+    end
+
     after do
         Puppet::Type.allclear
     end
-- 
cgit 


From 8212f88ce3ad2ddbc7e1e713111d9478171c42b8 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Thu, 20 Sep 2007 15:24:20 -0500
Subject: Fixing all existing spec tests so that they now pass given the
 redesign that Rick implemented. This was mostly a question of fixing the
 method names and the mocks.

---
 spec/unit/node/facts.rb     | 11 ++++++-----
 spec/unit/node/node.rb      |  8 ++++----
 spec/unit/node/searching.rb | 16 ++++++++--------
 3 files changed, 18 insertions(+), 17 deletions(-)

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/facts.rb b/spec/unit/node/facts.rb
index c677c1d2e..61f05a2b2 100755
--- a/spec/unit/node/facts.rb
+++ b/spec/unit/node/facts.rb
@@ -7,21 +7,22 @@ require 'puppet/node/facts'
 describe Puppet::Node::Facts, " when indirecting" do
     before do
         @terminus = mock 'terminus'
-        Puppet::Indirector.terminus(:facts, Puppet[:fact_store].intern).stubs(:new).returns(@terminus)
+        Puppet::Node::Facts.stubs(:indirection).returns(@terminus)
 
         # We have to clear the cache so that the facts ask for our terminus stub,
         # instead of anything that might be cached.
         Puppet::Indirector::Indirection.clear_cache
+        @facts = Puppet::Node::Facts.new("me", "one" => "two")
     end
 
     it "should redirect to the specified fact store for retrieval" do
-        @terminus.expects(:get).with(:my_facts)
-        Puppet::Node::Facts.get(:my_facts)
+        @terminus.expects(:find).with(:my_facts)
+        Puppet::Node::Facts.find(:my_facts)
     end
 
     it "should redirect to the specified fact store for storage" do
-        @terminus.expects(:post).with(:my_facts)
-        Puppet::Node::Facts.post(:my_facts)
+        @terminus.expects(:save).with(@facts)
+        @facts.save
     end
 
     after do
diff --git a/spec/unit/node/node.rb b/spec/unit/node/node.rb
index 9342dc5ce..899d81ac7 100755
--- a/spec/unit/node/node.rb
+++ b/spec/unit/node/node.rb
@@ -85,7 +85,7 @@ end
 describe Puppet::Node, " when merging facts" do
     before do
         @node = Puppet::Node.new("testnode")
-        Puppet::Node::Facts.stubs(:get).with(@node.name).returns(Puppet::Node::Facts.new(@node.name, "one" => "c", "two" => "b"))
+        Puppet::Node::Facts.stubs(:find).with(@node.name).returns(Puppet::Node::Facts.new(@node.name, "one" => "c", "two" => "b"))
     end
 
     it "should prefer parameters already set on the node over facts from the node" do
@@ -110,12 +110,12 @@ end
 describe Puppet::Node, " when indirecting" do
     before do
         @terminus = mock 'terminus'
-        Puppet::Indirector.terminus(:node, Puppet[:node_source]).stubs(:new).returns(@terminus)
+        Puppet::Node.stubs(:indirection).returns(@terminus)
     end
 
     it "should redirect to the specified node source" do
-        @terminus.expects(:get).with(:my_node)
-        Puppet::Node.get(:my_node)
+        @terminus.expects(:find).with(:my_node.to_s)
+        Puppet::Node.find(:my_node.to_s)
     end
 
     after do
diff --git a/spec/unit/node/searching.rb b/spec/unit/node/searching.rb
index 553822576..b7105050a 100755
--- a/spec/unit/node/searching.rb
+++ b/spec/unit/node/searching.rb
@@ -10,12 +10,12 @@ describe Puppet::Node::Searching, " when searching for nodes" do
         @searcher.extend(Puppet::Node::Searching)
         @facts = Puppet::Node::Facts.new("foo", "hostname" => "yay", "domain" => "domain.com")
         @node = Puppet::Node.new("foo")
-        Puppet::Node::Facts.stubs(:get).with("foo").returns(@facts)
+        Puppet::Node::Facts.stubs(:find).with("foo").returns(@facts)
     end
 
     it "should search for the node by its key first" do
         names = []
-        @searcher.expects(:get).with do |name|
+        @searcher.expects(:find).with do |name|
             names << name
             names == %w{foo}
         end.returns(@node)
@@ -24,15 +24,15 @@ describe Puppet::Node::Searching, " when searching for nodes" do
 
     it "should return the first node found using the generated list of names" do
         names = []
-        @searcher.expects(:get).with("foo").returns(nil)
-        @searcher.expects(:get).with("yay.domain.com").returns(@node)
+        @searcher.expects(:find).with("foo").returns(nil)
+        @searcher.expects(:find).with("yay.domain.com").returns(@node)
         @searcher.search("foo").should equal(@node)
     end
 
     it "should search for the rest of the names inversely by length" do
         names = []
         @facts.values["fqdn"] = "longer.than.the.normal.fqdn.com"
-        @searcher.stubs(:get).with do |name|
+        @searcher.stubs(:find).with do |name|
             names << name
         end
         @searcher.search("foo")
@@ -51,7 +51,7 @@ describe Puppet::Node::Searching, " when searching for nodes" do
 
     it "should attempt to find a default node if no names are found" do
         names = []
-        @searcher.stubs(:get).with do |name|
+        @searcher.stubs(:find).with do |name|
             names << name
         end.returns(nil)
         @searcher.search("foo")
@@ -59,7 +59,7 @@ describe Puppet::Node::Searching, " when searching for nodes" do
     end
 
     it "should cache the nodes" do
-        @searcher.expects(:get).with("foo").returns(@node)
+        @searcher.expects(:find).with("foo").returns(@node)
         @searcher.search("foo").should equal(@node)
         @searcher.search("foo").should equal(@node)
     end
@@ -68,7 +68,7 @@ describe Puppet::Node::Searching, " when searching for nodes" do
         node2 = Puppet::Node.new("foo2")
         Puppet[:filetimeout] = -1
         # I couldn't get this to work with :expects
-        @searcher.stubs(:get).returns(@node, node2).then.raises(ArgumentError)
+        @searcher.stubs(:find).returns(@node, node2).then.raises(ArgumentError)
         @searcher.search("foo").should equal(@node)
         @searcher.search("foo").should equal(node2)
     end
-- 
cgit 


From 86dde63473d29c45d8698ce4edd53c820a621362 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Sat, 22 Sep 2007 14:25:17 -0500
Subject: All tests now pass in this configuration branch, which means it's
 time to merge it back into the indirection branch.

Considering that this work was what drove me to create the
indirection branch in the first place, i should now be able to
merge both back in the master branch.
---
 spec/unit/node/configuration.rb | 10 ++++++++++
 1 file changed, 10 insertions(+)

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb
index eb4eaa418..b37cc754d 100755
--- a/spec/unit/node/configuration.rb
+++ b/spec/unit/node/configuration.rb
@@ -260,6 +260,16 @@ describe Puppet::Node::Configuration, " when applying" do
     it "should default to not being a host configuration" do
         @config.host_config.should be_nil
     end
+
+    it "should pass supplied tags on to the transaction" do
+        @transaction.expects(:tags=).with(%w{one two})
+        @config.apply(:tags => %w{one two})
+    end
+
+    it "should set ignoreschedules on the transaction if specified in apply()" do
+        @transaction.expects(:ignoreschedules=).with(true)
+        @config.apply(:ignoreschedules => true)
+    end
 end
 
 describe Puppet::Node::Configuration, " when applying host configurations" do
-- 
cgit 


From 3a18348fdbea39f56857b03c8f531bd5a2a8105d Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Sat, 22 Sep 2007 17:54:46 -0500
Subject: Renaming the 'Puppet::Util::Config' class to
 'Puppet::Util::Settings'.  This is to clear up confusion caused by the fact
 that we now have a 'Configuration' class to model host configurations, or any
 set of resources as a "configuration".

---
 spec/unit/node/configuration.rb | 6 +++---
 spec/unit/node/node.rb          | 4 ++--
 spec/unit/node/searching.rb     | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb
index b37cc754d..8ba55f50c 100755
--- a/spec/unit/node/configuration.rb
+++ b/spec/unit/node/configuration.rb
@@ -305,7 +305,7 @@ describe Puppet::Node::Configuration, " when applying host configurations" do
         @config.apply
     end
 
-    after { Puppet.config.clear }
+    after { Puppet.settings.clear }
 end
 
 describe Puppet::Node::Configuration, " when applying non-host configurations" do
@@ -328,7 +328,7 @@ describe Puppet::Node::Configuration, " when applying non-host configurations" d
         @config.apply
     end
 
-    after { Puppet.config.clear }
+    after { Puppet.settings.clear }
 end
 
 describe Puppet::Node::Configuration, " when creating a relationship graph" do
@@ -467,6 +467,6 @@ describe Puppet::Node::Configuration, " when writing dot files" do
     end
 
     after do
-        Puppet.config.clear
+        Puppet.settings.clear
     end
 end
diff --git a/spec/unit/node/node.rb b/spec/unit/node/node.rb
index 53f362da6..3146f6e7e 100755
--- a/spec/unit/node/node.rb
+++ b/spec/unit/node/node.rb
@@ -66,7 +66,7 @@ describe Puppet::Node, " when returning the environment" do
     end
 
     it "should return the central environment if there is no environment fact nor explicit environment" do
-        Puppet.config.expects(:[]).with(:environment).returns(:centralenv)
+        Puppet.settings.expects(:[]).with(:environment).returns(:centralenv)
         @node.environment.should == :centralenv
     end
 
@@ -81,7 +81,7 @@ describe Puppet::Node, " when returning the environment" do
     end
 
     it "should not use an explicit environment that is an empty string" do
-        Puppet.config.expects(:[]).with(:environment).returns(nil)
+        Puppet.settings.expects(:[]).with(:environment).returns(nil)
         @node.environment.should be_nil
     end
 end
diff --git a/spec/unit/node/searching.rb b/spec/unit/node/searching.rb
index b7105050a..e747996e4 100755
--- a/spec/unit/node/searching.rb
+++ b/spec/unit/node/searching.rb
@@ -74,6 +74,6 @@ describe Puppet::Node::Searching, " when searching for nodes" do
     end
 
     after do
-        Puppet.config.clear
+        Puppet.settings.clear
     end
 end
-- 
cgit 


From cdc8ea6e81c1b5eba5ea784bb7079c4c1f3965a4 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Sun, 23 Sep 2007 19:04:31 -0500
Subject: Taking a first stab at moving configuration compiling into the
 indirection system.  There are still quite a few unanswered questions, the
 two most notable being embodied in unimplemented tests in the Configuration
 Code terminus.

This also requires changing the behaviour in a few places.
In particular, 'puppet' and the 'module_puppet' cfengine
module need to store a Node object in memory with the appropriate
classes, since that's now the only way to communicate with
the compiler.  That integration work has not yet been done,
partially because the old configuration handler (which the
soon-to-be-deprecated master handler now uses) still exists.
---
 spec/unit/node/node.rb | 7 +++++++
 1 file changed, 7 insertions(+)

(limited to 'spec/unit/node')

diff --git a/spec/unit/node/node.rb b/spec/unit/node/node.rb
index 3146f6e7e..fe5d2be8b 100755
--- a/spec/unit/node/node.rb
+++ b/spec/unit/node/node.rb
@@ -126,3 +126,10 @@ describe Puppet::Node, " when indirecting" do
         Puppet::Indirector::Indirection.clear_cache
     end
 end
+
+describe Puppet::Node do
+    # LAK:NOTE This is used to keep track of when a given node has connected,
+    # so we can report on nodes that do not appear to connecting to the
+    # central server.
+    it "should provide a method for noting that the node has connected"
+end
-- 
cgit