Moving all of the tests for Puppet::Parser::Compile to

rspec, so I can refactor the class to more heavily rely
on a Node::Catalog instead of doing its own resource
container management.

5 files changed, 350 insertions, 84 deletions

diff --git a/lib/puppet/parser/compile.rb b/lib/puppet/parser/compile.rb
index e1e230d48..42d229867 100644
--- a/lib/puppet/parser/compile.rb
+++ b/lib/puppet/parser/compile.rb
@@ -68,10 +68,10 @@ class Puppet::Parser::Compile
 
         evaluate_generators()
 
-        fail_on_unevaluated()
-
         finish()
 
+        fail_on_unevaluated()
+
         if Puppet[:storeconfigs]
             store()
         end
@@ -200,8 +200,6 @@ class Puppet::Parser::Compile
 
     # Store a resource override.
     def store_override(override)
-        override.override = true
-
         # If possible, merge the override in immediately.
         if resource = @resource_table[override.ref]
             resource.merge(override)
@@ -383,7 +381,20 @@ class Puppet::Parser::Compile
     # Make sure all of our resources and such have done any last work
     # necessary.
     def finish
-        @resource_table.each { |name, resource| resource.finish if resource.respond_to?(:finish) }
+        @resource_table.each do |name, resource|
+            # Add in any resource overrides.
+            if overrides = resource_overrides(resource)
+                overrides.each do |over|
+                    resource.merge(over)
+                end
+
+                # Remove the overrides, so that the configuration knows there
+                # are none left.
+                overrides.clear
+            end
+
+            resource.finish if resource.respond_to?(:finish)
+        end
     end
 
     # Initialize the top-level scope, class, and resource.
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index f8701578c..67428d5f3 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -83,7 +83,6 @@ class Puppet::Parser::Resource
     # Do any finishing work on this object, called before evaluation or
     # before storage/translation.
     def finish
-        add_overrides()
         add_defaults()
         add_metaparams()
         add_scope_tags()
@@ -342,19 +341,6 @@ class Puppet::Parser::Resource
         end
     end
 
-    # Add any overrides for this object.
-    def add_overrides
-        if overrides = scope.compile.resource_overrides(self)
-            overrides.each do |over|
-                self.merge(over)
-            end
-
-            # Remove the overrides, so that the configuration knows there
-            # are none left.
-            overrides.clear
-        end
-    end
-
     def add_scope_tags
         if scope_resource = scope.resource
             tag(*scope_resource.tags)
diff --git a/lib/puppet/parser/resource/reference.rb b/lib/puppet/parser/resource/reference.rb
index 6e70d23b7..ea53b421a 100644
--- a/lib/puppet/parser/resource/reference.rb
+++ b/lib/puppet/parser/resource/reference.rb
@@ -49,7 +49,7 @@ class Puppet::Parser::Resource::Reference < Puppet::ResourceReference
             if tmp
                 @definedtype = tmp
             else
-                fail Puppet::ParseError, "Could not find resource '%s'" % self
+                fail Puppet::ParseError, "Could not find resource type '%s'" % self.type
             end
         end
 
diff --git a/spec/unit/parser/compile.rb b/spec/unit/parser/compile.rb
index 092bece0c..a72713360 100755
--- a/spec/unit/parser/compile.rb
+++ b/spec/unit/parser/compile.rb
@@ -2,12 +2,18 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
-describe Puppet::Parser::Compile, " when compiling" do
-    before do
-        @node = stub 'node', :name => 'mynode'
-        @parser = stub 'parser', :version => "1.0"
+module CompileTesting
+    def setup
+        @node = Puppet::Node.new "testnode"
+        @parser = Puppet::Parser::Parser.new :environment => "development"
+
+        @scope = stub 'scope', :resource => stub("scope_resource"), :source => mock("source")
         @compile = Puppet::Parser::Compile.new(@node, @parser)
     end
+end
+
+describe Puppet::Parser::Compile, " when compiling" do
+    include CompileTesting
 
     def compile_methods
         [:set_node_parameters, :evaluate_main, :evaluate_ast_node, :evaluate_node_classes, :evaluate_generators, :fail_on_unevaluated,
@@ -37,7 +43,7 @@ describe Puppet::Parser::Compile, " when compiling" do
         @node.stubs(:classes).returns(classes)
 
         @compile.expects(:evaluate_classes).with(classes, @compile.topscope)
-        @compile.send :evaluate_node_classes
+        @compile.class.publicize_methods(:evaluate_node_classes) { @compile.evaluate_node_classes }
     end
 
     it "should enable ast_nodes if the parser has any nodes" do
@@ -49,15 +55,143 @@ describe Puppet::Parser::Compile, " when compiling" do
         @parser.expects(:nodes).returns({})
         @compile.ast_nodes?.should be_false
     end
+
+    it "should evaluate the main class if it exists" do
+        compile_stub(:evaluate_main)
+        main_class = mock 'main_class'
+        main_class.expects(:evaluate_code).with { |r| r.is_a?(Puppet::Parser::Resource) }
+        @compile.topscope.expects(:source=).with(main_class)
+        @parser.stubs(:findclass).with("", "").returns(main_class)
+
+        @compile.compile
+    end
+
+    it "should evaluate any node classes" do
+        @node.stubs(:classes).returns(%w{one two three four})
+        @compile.expects(:evaluate_classes).with(%w{one two three four}, @compile.topscope)
+        @compile.send(:evaluate_node_classes)
+    end
+
+    it "should evaluate all added collections" do
+        colls = []
+        # And when the collections fail to evaluate.
+        colls << mock("coll1-false")
+        colls << mock("coll2-false")
+        colls.each { |c| c.expects(:evaluate).returns(false) }
+
+        @compile.add_collection(colls[0])
+        @compile.add_collection(colls[1])
+
+        compile_stub(:evaluate_generators)
+        @compile.compile
+    end
+
+    it "should ignore builtin resources" do
+        resource = stub 'builtin', :ref => "File[testing]", :builtin? => true
+
+        scope = stub 'scope', :resource => stub("scope_resource")
+        @compile.store_resource(scope, resource)
+        resource.expects(:evaluate).never
+        
+        @compile.compile
+    end
+
+    it "should evaluate unevaluated resources" do
+        resource = stub 'notevaluated', :ref => "File[testing]", :builtin? => false, :evaluated? => false
+        scope = stub 'scope', :resource => stub("scope_resource")
+        @compile.store_resource(scope, resource)
+
+        # We have to now mark the resource as evaluated
+        resource.expects(:evaluate).with { |*whatever| resource.stubs(:evaluated?).returns true }
+        
+        @compile.compile
+    end
+
+    it "should not evaluate already-evaluated resources" do
+        resource = stub 'already_evaluated', :ref => "File[testing]", :builtin? => false, :evaluated? => true
+        scope = stub 'scope', :resource => stub("scope_resource")
+        @compile.store_resource(scope, resource)
+        resource.expects(:evaluate).never
+        
+        @compile.compile
+    end
+
+    it "should evaluate unevaluated resources created by evaluating other resources" do
+        resource = stub 'notevaluated', :ref => "File[testing]", :builtin? => false, :evaluated? => false
+        scope = stub 'scope', :resource => stub("scope_resource")
+        @compile.store_resource(scope, resource)
+
+        resource2 = stub 'created', :ref => "File[other]", :builtin? => false, :evaluated? => false
+
+        # We have to now mark the resource as evaluated
+        resource.expects(:evaluate).with { |*whatever| resource.stubs(:evaluated?).returns(true); @compile.store_resource(scope, resource2) }
+        resource2.expects(:evaluate).with { |*whatever| resource2.stubs(:evaluated?).returns(true) }
+
+        
+        @compile.compile
+    end
+
+    it "should call finish() on all resources" do
+        # Add a resource that does respond to :finish
+        yep = stub "finisher", :ref => "File[finish]"
+        yep.expects(:respond_to?).with(:finish).returns(true)
+        yep.expects(:finish)
+
+        @compile.store_resource(@scope, yep)
+
+        # And one that does not
+        dnf = stub "dnf", :ref => "File[dnf]"
+        dnf.expects(:respond_to?).with(:finish).returns(false)
+
+        @compile.store_resource(@scope, dnf)
+
+        @compile.send(:finish)
+    end
+
+    it "should add resources that do not conflict with existing resources" do
+        resource = stub "noconflict", :ref => "File[yay]"
+        @compile.store_resource(@scope, resource)
+
+        @compile.catalog.should be_vertex(resource)
+    end
+
+    it "should not fail when conflicting resources are non-isormphic" do
+        type = stub 'faketype', :isomorphic? => false, :name => "mytype"
+        Puppet::Type.stubs(:type).with("mytype").returns(type)
+
+        resource1 = stub "iso1conflict", :ref => "Mytype[yay]", :type => "mytype"
+        resource2 = stub "iso2conflict", :ref => "Mytype[yay]", :type => "mytype"
+
+        @compile.store_resource(@scope, resource1)
+        lambda { @compile.store_resource(@scope, resource2) }.should_not raise_error
+    end
+
+    it "should fail to add resources that conflict with existing resources" do
+        type = stub 'faketype', :isomorphic? => true, :name => "mytype"
+        Puppet::Type.stubs(:type).with("mytype").returns(type)
+
+        resource1 = stub "iso1conflict", :ref => "Mytype[yay]", :type => "mytype", :file => "eh", :line => 0
+        resource2 = stub "iso2conflict", :ref => "Mytype[yay]", :type => "mytype", :file => "eh", :line => 0
+
+        @compile.store_resource(@scope, resource1)
+        lambda { @compile.store_resource(@scope, resource2) }.should raise_error(Puppet::ParseError)
+    end
+
+    it "should have a method for looking up resources" do
+        resource = stub 'resource', :ref => "Yay[foo]"
+        @compile.store_resource(@scope, resource)
+        @compile.findresource("Yay[foo]").should equal(resource)
+    end
+
+    it "should be able to look resources up by type and title" do
+        resource = stub 'resource', :ref => "Yay[foo]"
+        @compile.store_resource(@scope, resource)
+        @compile.findresource("Yay", "foo").should equal(resource)
+    end
 end
 
 describe Puppet::Parser::Compile, " when evaluating classes" do
-    before do
-        @node = stub 'node', :name => 'mynode'
-        @parser = stub 'parser', :version => "1.0"
-        @scope = stub 'scope', :source => mock("source")
-        @compile = Puppet::Parser::Compile.new(@node, @parser)
-    end
+    include CompileTesting
 
     it "should fail if there's no source listed for the scope" do
         scope = stub 'scope', :source => nil
@@ -72,12 +206,7 @@ describe Puppet::Parser::Compile, " when evaluating classes" do
 end
 
 describe Puppet::Parser::Compile, " when evaluating collections" do
-    before do
-        @node = stub 'node', :name => 'mynode'
-        @parser = stub 'parser', :version => "1.0"
-        @scope = stub 'scope', :source => mock("source")
-        @compile = Puppet::Parser::Compile.new(@node, @parser)
-    end
+    include CompileTesting
 
     it "should evaluate each collection" do
         2.times { |i|
@@ -93,16 +222,40 @@ describe Puppet::Parser::Compile, " when evaluating collections" do
 
         @compile.class.publicize_methods(:evaluate_collections) { @compile.evaluate_collections }
     end
+
+    it "should not fail when there are unevaluated resource collections that do not refer to specific resources" do
+        coll = stub 'coll', :evaluate => false
+        coll.expects(:resources).returns(nil)
+
+        @compile.add_collection(coll)
+
+        lambda { @compile.compile }.should_not raise_error
+    end
+
+    it "should fail when there are unevaluated resource collections that refer to a specific resource" do
+        coll = stub 'coll', :evaluate => false
+        coll.expects(:resources).returns(:something)
+
+        @compile.add_collection(coll)
+
+        lambda { @compile.compile }.should raise_error(Puppet::ParseError)
+    end
+
+    it "should fail when there are unevaluated resource collections that refer to multiple specific resources" do
+        coll = stub 'coll', :evaluate => false
+        coll.expects(:resources).returns([:one, :two])
+
+        @compile.add_collection(coll)
+
+        lambda { @compile.compile }.should raise_error(Puppet::ParseError)
+    end
 end
 
 
 describe Puppet::Parser::Compile, " when evaluating found classes" do
-    before do
-        @node = stub 'node', :name => 'mynode'
-        @parser = stub 'parser', :version => "1.0"
-        @scope = stub 'scope', :source => mock("source")
-        @compile = Puppet::Parser::Compile.new(@node, @parser)
+    include CompileTesting
 
+    before do
         @class = stub 'class', :classname => "my::class"
         @scope.stubs(:findclass).with("myclass").returns(@class)
 
@@ -181,11 +334,7 @@ describe Puppet::Parser::Compile, " when evaluating found classes" do
 end
 
 describe Puppet::Parser::Compile, " when evaluating AST nodes with no AST nodes present" do
-    before do
-        @node = stub 'node', :name => "foo"
-        @parser = stub 'parser', :version => "1.0", :nodes => {}
-        @compile = Puppet::Parser::Compile.new(@node, @parser)
-    end
+    include CompileTesting
 
     it "should do nothing" do
         @compile.expects(:ast_nodes?).returns(false)
@@ -197,11 +346,9 @@ describe Puppet::Parser::Compile, " when evaluating AST nodes with no AST nodes
 end
 
 describe Puppet::Parser::Compile, " when evaluating AST nodes with AST nodes present" do
-    before do
-        @node = stub 'node', :name => "foo"
-        @parser = stub 'parser', :version => "1.0", :nodes => {}
-        @compile = Puppet::Parser::Compile.new(@node, @parser)
+    include CompileTesting
 
+    before do
         @nodes = mock 'node_hash'
         @compile.stubs(:ast_nodes?).returns(true)
         @compile.parser.stubs(:nodes).returns(@nodes)
@@ -279,3 +426,158 @@ describe Puppet::Parser::Compile, " when evaluating AST nodes with AST nodes pre
         @compile.topscope.should == :my_node_scope
     end
 end
+
+describe Puppet::Parser::Compile, " when initializing" do
+    include CompileTesting
+
+    it "should set its node attribute" do
+        @compile.node.should equal(@node)
+    end
+
+    it "should set its parser attribute" do
+        @compile.parser.should equal(@parser)
+    end
+
+    it "should detect when ast nodes are absent" do
+        @compile.ast_nodes?.should be_false
+    end
+
+    it "should detect when ast nodes are present" do
+        @parser.nodes["testing"] = "yay"
+        @compile.ast_nodes?.should be_true
+    end
+end
+
+describe Puppet::Parser::Compile do
+    include CompileTesting
+
+    it "should be able to store references to class scopes" do
+        lambda { @compile.class_set "myname", "myscope" }.should_not raise_error
+    end
+
+    it "should be able to retrieve class scopes by name" do
+        @compile.class_set "myname", "myscope"
+        @compile.class_scope("myname").should == "myscope"
+    end
+
+    it "should be able to retrieve class scopes by object" do
+        klass = mock 'ast_class'
+        klass.expects(:classname).returns("myname")
+        @compile.class_set "myname", "myscope"
+        @compile.class_scope(klass).should == "myscope"
+    end
+
+    it "should be able to return a class list containing all set classes" do
+        @compile.class_set "", "empty"
+        @compile.class_set "one", "yep"
+        @compile.class_set "two", "nope"
+
+        @compile.classlist.sort.should == %w{one two}.sort
+    end
+end
+
+describe Puppet::Parser::Compile, "when managing scopes" do
+    include CompileTesting
+
+    it "should create a top scope" do
+        @compile.topscope.should be_instance_of(Puppet::Parser::Scope)
+    end
+
+    it "should be able to create new scopes" do
+        @compile.newscope(@compile.topscope).should be_instance_of(Puppet::Parser::Scope)
+    end
+
+    it "should correctly set the level of newly created scopes" do
+        @compile.newscope(@compile.topscope, :level => 5).level.should == 5
+    end
+
+    it "should set the parent scope of the new scope to be the passed-in parent" do
+        scope = mock 'scope'
+        newscope = @compile.newscope(scope)
+
+        @compile.parent(newscope).should equal(scope)
+    end
+end
+
+describe Puppet::Parser::Compile, "when storing compiled resources" do
+    include CompileTesting
+
+    it "should store the resources" do
+        Puppet.features.expects(:rails?).returns(true)
+        Puppet::Rails.expects(:connect)
+
+        resource_table = mock 'resources'
+        resource_table.expects(:values).returns(:resources)
+        @compile.instance_variable_set("@resource_table", resource_table)
+        @compile.expects(:store_to_active_record).with(@node, :resources)
+        @compile.send(:store)
+    end
+
+    it "should store to active_record" do
+        @node.expects(:name).returns("myname")
+        Puppet::Rails::Host.stubs(:transaction).yields
+        Puppet::Rails::Host.expects(:store).with(@node, :resources)
+        @compile.send(:store_to_active_record, @node, :resources)
+    end
+end
+
+describe Puppet::Parser::Compile, "when managing resource overrides" do
+    include CompileTesting
+
+    before do
+        @override = stub 'override', :ref => "My[ref]"
+        @resource = stub 'resource', :ref => "My[ref]", :builtin? => true
+    end
+
+    it "should be able to store overrides" do
+        lambda { @compile.store_override(@override) }.should_not raise_error
+    end
+
+    it "should apply overrides to the appropriate resources" do
+        @compile.store_resource(@scope, @resource)
+        @resource.expects(:merge).with(@override)
+
+        @compile.store_override(@override)
+
+        @compile.compile
+    end
+
+    it "should accept overrides before the related resource has been created" do
+        @resource.expects(:merge).with(@override)
+
+        # First store the override
+        @compile.store_override(@override)
+
+        # Then the resource
+        @compile.store_resource(@scope, @resource)
+
+        # And compile, so they get resolved
+        @compile.compile
+    end
+
+    it "should fail if the compile is finished and resource overrides have not been applied" do
+        @compile.store_override(@override)
+
+        lambda { @compile.compile }.should raise_error(Puppet::ParseError)
+    end
+end
+
+# #620 - Nodes and classes should conflict, else classes don't get evaluated
+describe Puppet::Parser::Compile, "when evaluating nodes and classes with the same name (#620)" do
+    include CompileTesting
+
+    before do
+        @node = stub :nodescope? => true
+        @class = stub :nodescope? => false
+    end
+
+    it "should fail if a node already exists with the same name as the class being evaluated" do
+        @compile.class_set("one", @node)
+        lambda { @compile.class_set("one", @class) }.should raise_error(Puppet::ParseError)
+    end
+
+    it "should fail if a class already exists with the same name as the node being evaluated" do
+        @compile.class_set("one", @class)
+        lambda { @compile.class_set("one", @node) }.should raise_error(Puppet::ParseError)
+    end
+end
diff --git a/test/language/resource.rb b/test/language/resource.rb
index 3aa9dcf6a..f129023eb 100755
--- a/test/language/resource.rb
+++ b/test/language/resource.rb
@@ -106,7 +106,6 @@ class TestResource < PuppetTest::TestCase
 
     def test_finish
         res = mkresource
-        res.expects(:add_overrides)
         res.expects(:add_defaults)
         res.expects(:add_metaparams)
         res.expects(:validate)
@@ -275,38 +274,6 @@ class TestResource < PuppetTest::TestCase
         res.evaluate
     end
 
-    def test_add_overrides
-        # Try it with nil
-        res = mkresource
-        res.scope = mock('scope')
-        config = mock("config")
-        res.scope.expects(:compile).returns(config)
-        config.expects(:resource_overrides).with(res).returns(nil)
-        res.expects(:merge).never
-        res.send(:add_overrides)
-
-        # And an empty array
-        res = mkresource
-        res.scope = mock('scope')
-        config = mock("config")
-        res.scope.expects(:compile).returns(config)
-        config.expects(:resource_overrides).with(res).returns([])
-        res.expects(:merge).never
-        res.send(:add_overrides)
-
-        # And with some overrides
-        res = mkresource
-        res.scope = mock('scope')
-        config = mock("config")
-        res.scope.expects(:compile).returns(config)
-        returns = %w{a b}
-        config.expects(:resource_overrides).with(res).returns(returns)
-        res.expects(:merge).with("a")
-        res.expects(:merge).with("b")
-        res.send(:add_overrides)
-        assert(returns.empty?, "Did not clear overrides")
-    end
-
     def test_proxymethods
         res = Parser::Resource.new :type => "evaltest", :title => "yay",
             :source => mock("source"), :scope => mkscope