Fixing most of the broken tests in test/

This involves a bit of refactoring in the rest
of the code to make it all work, but most of the
changes are fixing or removing old tests.

Signed-off-by: Luke Kanies <luke@reductivelabs.com>

6 files changed, 231 insertions, 114 deletions

diff --git a/spec/integration/transaction.rb b/spec/integration/transaction.rb
index a387bf7c9..b87bfb3b5 100755
--- a/spec/integration/transaction.rb
+++ b/spec/integration/transaction.rb
@@ -4,10 +4,17 @@ require File.dirname(__FILE__) + '/../spec_helper'
 
 require 'puppet_spec/files'
 require 'puppet/transaction'
+require 'puppet_spec/files'
 
 describe Puppet::Transaction do
     include PuppetSpec::Files
 
+    def mk_catalog(*resources)
+        catalog = Puppet::Resource::Catalog.new(Puppet::Node.new("mynode"))
+        resources.each { |res| catalog.add_resource res }
+        catalog
+    end
+
     it "should not apply generated resources if the parent resource fails" do
         catalog = Puppet::Resource::Catalog.new
         resource = Puppet::Type.type(:file).new :path => "/foo/bar", :backup => false
@@ -19,7 +26,8 @@ describe Puppet::Transaction do
 
         transaction = Puppet::Transaction.new(catalog)
 
-        resource.expects(:retrieve).raises "this is a failure"
+        resource.expects(:evaluate).raises "this is a failure"
+        resource.stubs(:err)
 
         child_resource.expects(:retrieve).never
 
@@ -41,15 +49,13 @@ describe Puppet::Transaction do
 
     it "should apply exported resources" do
         catalog = Puppet::Resource::Catalog.new
-        resource = Puppet::Type.type(:file).new :path => "/foo/bar", :backup => false
+        path = tmpfile("exported_files")
+        resource = Puppet::Type.type(:file).new :path => path, :backup => false, :ensure => :file
         resource.exported = true
         catalog.add_resource resource
 
-        transaction = Puppet::Transaction.new(catalog)
-
-        resource.expects(:evaluate).never
-
-        transaction.evaluate
+        catalog.apply
+        FileTest.should be_exist(path)
     end
 
     it "should not apply virtual exported resources" do
@@ -66,119 +72,166 @@ describe Puppet::Transaction do
         transaction.evaluate
     end
 
-    it "should refresh resources that subscribe to changed resources" do
-        name = tmpfile("something")
+    # Verify that one component requiring another causes the contained
+    # resources in the requiring component to get refreshed.
+    it "should propagate events from a contained resource through its container to its dependent container's contained resources" do
+        transaction = nil
+        file = Puppet::Type.type(:file).new :path => tmpfile("event_propagation"), :ensure => :present
+        execfile = File.join(tmpdir("exec_event"), "exectestingness2")
+        exec = Puppet::Type.type(:exec).new :command => "touch #{execfile}", :path => ENV['PATH']
+        catalog = mk_catalog(file)
+
+        fcomp = Puppet::Type.type(:component).new(:name => "Foo[file]")
+        catalog.add_resource fcomp
+        catalog.add_edge(fcomp, file)
+
+        ecomp = Puppet::Type.type(:component).new(:name => "Foo[exec]")
+        catalog.add_resource ecomp
+        catalog.add_resource exec
+        catalog.add_edge(ecomp, exec)
+
+        ecomp[:subscribe] = Puppet::Resource.new(:foo, "file")
+        exec[:refreshonly] = true
+
+        exec.expects(:refresh)
+        catalog.apply
+    end
+
+    # Make sure that multiple subscriptions get triggered.
+    it "should propagate events to all dependent resources" do
+        path = tmpfile("path")
+        file1 = tmpfile("file1")
+        file2 = tmpfile("file2")
         file = Puppet::Type.type(:file).new(
-            :name => name,
+            :path => path,
             :ensure => "file"
         )
-        exec = Puppet::Type.type(:exec).new(
-            :name => "echo true",
-            :path => "/usr/bin:/bin",
+        exec1 = Puppet::Type.type(:exec).new(
+            :path => ENV["PATH"],
+            :command => "touch %s" % file1,
             :refreshonly => true,
-            :subscribe => Puppet::Resource::Reference.new(file.class.name, file.name)
+            :subscribe => Puppet::Resource.new(:file, path)
+        )
+        exec2 = Puppet::Type.type(:exec).new(
+            :path => ENV["PATH"],
+            :command => "touch %s" % file2,
+            :refreshonly => true,
+            :subscribe => Puppet::Resource.new(:file, path)
         )
 
-        catalog = Puppet::Resource::Catalog.new
-        catalog.add_resource file, exec
+        catalog = mk_catalog(file, exec1, exec2)
+        catalog.apply
+        FileTest.should be_exist(file1)
+        FileTest.should be_exist(file2)
+    end
 
-        exec.expects(:refresh)
+    it "should not let one failed refresh result in other refreshes failing" do
+        path = tmpfile("path")
+        newfile = tmpfile("file")
+        file = Puppet::Type.type(:file).new(
+            :path => path,
+            :ensure => "file"
+        )
+        exec1 = Puppet::Type.type(:exec).new(
+            :path => ENV["PATH"],
+            :command => "touch /this/cannot/possibly/exist",
+            :logoutput => true,
+            :refreshonly => true,
+            :subscribe => file,
+            :title => "one"
+        )
+        exec2 = Puppet::Type.type(:exec).new(
+            :path => ENV["PATH"],
+            :command => "touch %s" % newfile,
+            :logoutput => true,
+            :refreshonly => true,
+            :subscribe => [file, exec1],
+            :title => "two"
+        )
+
+        exec1.stubs(:err)
 
+        catalog = mk_catalog(file, exec1, exec2)
         catalog.apply
+        FileTest.should be_exists(newfile)
     end
 
-    it "should not refresh resources that only require changed resources" do
-        name = tmpfile("something")
+    it "should still trigger skipped resources" do
+        catalog = mk_catalog()
+        catalog.add_resource(*Puppet::Type.type(:schedule).mkdefaultschedules)
+
+        Puppet[:ignoreschedules] = false
         file = Puppet::Type.type(:file).new(
-            :name => name,
-            :ensure => "file"
+            :name => tmpfile("file"),
+            :ensure => "file",
+            :backup => false
         )
+
+        fname = tmpfile("exec")
         exec = Puppet::Type.type(:exec).new(
-            :name => "echo true",
+            :name => "touch #{fname}",
             :path => "/usr/bin:/bin",
-            :refreshonly => true,
-            :require => Puppet::Resource::Reference.new(file.class.name, file.name)
+            :schedule => "monthly",
+            :subscribe => Puppet::Resource.new("file", file.name)
         )
 
+        catalog.add_resource(file, exec)
 
-        catalog = Puppet::Resource::Catalog.new
-        catalog.add_resource file
-        catalog.add_resource exec
+        # Run it once
+        catalog.apply
+        FileTest.should be_exists(fname)
 
-        exec.expects(:refresh).never
+        exec.should_not be_scheduled
 
-        trans = catalog.apply
+        # Now remove it, so it can get created again
+        File.unlink(fname)
 
-        trans.events.length.should == 1
-    end
+        file[:content] = "some content"
+
+        catalog.apply
+        FileTest.should be_exists(fname)
 
-    it "should cascade events such that multiple refreshes result" do
-        files = []
+        # Now remove it, so it can get created again
+        File.unlink(fname)
 
-        4.times { |i|
-            files << Puppet::Type.type(:file).new(
-                :name => tmpfile("something"),
-                :ensure => "file"
-            )
-        }
+        # And tag our exec
+        exec.tag("testrun")
 
-        fname = tmpfile("something")
-        exec = Puppet::Type.type(:exec).new(
-            :name => "touch %s" % fname,
-            :path => "/usr/bin:/bin",
-            :refreshonly => true
-        )
+        # And our file, so it runs
+        file.tag("norun")
 
-        exec[:subscribe] = files.collect { |f|
-            Puppet::Resource::Reference.new(:file, f.name)
-        }
+        Puppet[:tags] = "norun"
 
-        catalog = Puppet::Resource::Catalog.new
-        catalog.add_resource(exec, *files)
+        file[:content] = "totally different content"
 
         catalog.apply
-        FileTest.should be_exist(fname)
+        FileTest.should be_exists(fname)
     end
 
-    # Make sure refreshing happens mid-transaction, rather than at the end.
-    it "should refresh resources as they're encountered rather than all at the end" do
-        file = tmpfile("something")
-
-        exec1 = Puppet::Type.type(:exec).new(
-            :title => "one",
-            :name => "echo one >> %s" % file,
-            :path => "/usr/bin:/bin"
+    it "should not attempt to evaluate resources with failed dependencies" do
+        exec = Puppet::Type.type(:exec).new(
+            :command => "/bin/mkdir /this/path/cannot/possibly/exit",
+            :title => "mkdir"
         )
 
-        exec2 = Puppet::Type.type(:exec).new(
-            :title => "two",
-            :name => "echo two >> %s" % file,
-            :path => "/usr/bin:/bin",
-            :refreshonly => true,
-            :subscribe => exec1
+        file1 = Puppet::Type.type(:file).new(
+            :title => "file1",
+            :path => tmpfile("file1"),
+            :require => exec,
+            :ensure => :file
         )
 
-        exec3 = Puppet::Type.type(:exec).new(
-            :title => "three",
-            :name => "echo three >> %s" % file,
-            :path => "/usr/bin:/bin",
-            :require => exec2
+        file2 = Puppet::Type.type(:file).new(
+            :title => "file2",
+            :path => tmpfile("file2"),
+            :require => file1,
+            :ensure => :file
         )
-        execs = [exec1, exec2, exec3]
-
-        catalog = Puppet::Resource::Catalog.new
-        catalog.add_resource(exec1,exec2,exec3)
-
-        trans = Puppet::Transaction.new(catalog)
-        execs.each { |e| catalog.should be_vertex(e) }
-        trans.prepare
-        execs.each { |e| catalog.should be_vertex(e) }
-        reverse = trans.relationship_graph.reversal
-        execs.each { |e| reverse.should be_vertex(e) }
 
+        catalog = mk_catalog(exec, file1, file2)
         catalog.apply
 
-        FileTest.should be_exist(file)
-        File.read(file).should == "one\ntwo\nthree\n"
+        FileTest.should_not be_exists(file1[:path])
+        FileTest.should_not be_exists(file2[:path])
     end
 end
diff --git a/spec/unit/parser/ast/resource.rb b/spec/unit/parser/ast/resource.rb
index 391f4c770..ef65f4ccd 100755
--- a/spec/unit/parser/ast/resource.rb
+++ b/spec/unit/parser/ast/resource.rb
@@ -7,7 +7,7 @@ describe Puppet::Parser::AST::Resource do
 
     before :each do
         @title = stub_everything 'title'
-        @compiler = stub_everything 'compiler'
+        @compiler = stub_everything 'compiler', :environment => Puppet::Node::Environment.new
         @scope = Puppet::Parser::Scope.new(:compiler => @compiler)
         @scope.stubs(:resource).returns(stub_everything)
         @resource = ast::Resource.new(:title => @title, :type => "Resource", :params => ast::ASTArray.new(:children => []) )
@@ -99,4 +99,38 @@ describe Puppet::Parser::AST::Resource do
         result[0].should be_virtual
         result[0].should be_exported
     end
+
+    # Related to #806, make sure resources always look up the full path to the resource.
+    describe "when generating qualified resources" do
+        before do
+            @scope = Puppet::Parser::Scope.new :compiler => Puppet::Parser::Compiler.new(Puppet::Node.new("mynode"))
+            @parser = Puppet::Parser::Parser.new(Puppet::Node::Environment.new)
+            @parser.newdefine "one"
+            @parser.newdefine "one::two"
+            @parser.newdefine "three"
+            @twoscope = @scope.newscope(:namespace => "one")
+            @twoscope.resource = @scope.resource
+        end
+
+        def resource(type, params = nil)
+            params ||= Puppet::Parser::AST::ASTArray.new(:children => [])
+            Puppet::Parser::AST::Resource.new(:type => type, :title => Puppet::Parser::AST::String.new(:value => "myresource"), :params => params)
+        end
+
+        it "should be able to generate resources with fully qualified type information" do
+            resource("two").evaluate(@twoscope)[0].type.should == "One::Two"
+        end
+
+        it "should be able to generate resources with unqualified type information" do
+            resource("one").evaluate(@twoscope)[0].type.should == "One"
+        end
+
+        it "should correctly generate resources that can look up builtin types" do
+            resource("file").evaluate(@twoscope)[0].type.should == "File"
+        end
+
+        it "should fail for resource types that do not exist" do
+            lambda { resource("nosuchtype").evaluate(@twoscope) }.should raise_error(Puppet::ParseError)
+        end
+    end
 end
diff --git a/spec/unit/parser/collector.rb b/spec/unit/parser/collector.rb
index 9c2d722e5..78d47c63f 100755
--- a/spec/unit/parser/collector.rb
+++ b/spec/unit/parser/collector.rb
@@ -174,7 +174,7 @@ describe Puppet::Parser::Collector, "when collecting virtual and catalog resourc
         @collector.evaluate.should == [one]
     end
 
-    it "should create a resource with overriden parameters" do
+    it "should create a resource with overridden parameters" do
         one = stub_everything 'one', :type => "Mytype", :virtual? => true, :title => "test"
         param = stub 'param'
         @compiler.stubs(:add_override)
@@ -182,7 +182,7 @@ describe Puppet::Parser::Collector, "when collecting virtual and catalog resourc
         @compiler.expects(:resources).returns([one])
 
         @collector.add_override(:params => param )
-        Puppet::Parser::Resource.expects(:new).with { |h|
+        Puppet::Parser::Resource.expects(:new).with { |type, title, h|
             h[:params] == param
         }
 
@@ -214,7 +214,7 @@ describe Puppet::Parser::Collector, "when collecting virtual and catalog resourc
         @compiler.expects(:resources).at_least(2).returns([one])
 
         @collector.add_override(:params => param )
-        Puppet::Parser::Resource.expects(:new).once.with { |h|
+        Puppet::Parser::Resource.expects(:new).once.with { |type, title, h|
             h[:params] == param
         }
 
@@ -375,7 +375,7 @@ describe Puppet::Parser::Collector, "when collecting exported resources" do
         @compiler.stubs(:add_resource)
 
         @collector.add_override(:params => param )
-        Puppet::Parser::Resource.expects(:new).once.with { |h|
+        Puppet::Parser::Resource.expects(:new).once.with { |type, title, h|
             h[:params] == param
         }
 
diff --git a/spec/unit/parser/resource.rb b/spec/unit/parser/resource.rb
index 0c70c817e..ee0c1e089 100755
--- a/spec/unit/parser/resource.rb
+++ b/spec/unit/parser/resource.rb
@@ -16,12 +16,8 @@ describe Puppet::Parser::Resource do
     end
 
     def mkresource(args = {})
-        args[:source] ||= "source"
-        args[:scope] ||= stub('scope', :source => mock('source'))
-
-        {:source => "source", :scope => "scope"}.each do |param, value|
-            args[param] ||= value
-        end
+        args[:source] ||= @source
+        args[:scope] ||= @scope
 
         params = args[:params] || {:one => "yay", :three => "rah"}
         if args[:params] == :none
@@ -101,18 +97,18 @@ describe Puppet::Parser::Resource do
     end
 
     it "should be able to use the indexing operator to access parameters" do
-        resource = Puppet::Parser::Resource.new("resource", "testing", :source => "source", :scope => "scope")
+        resource = Puppet::Parser::Resource.new("resource", "testing", :source => "source", :scope => @scope)
         resource["foo"] = "bar"
         resource["foo"].should == "bar"
     end
 
     it "should return the title when asked for a parameter named 'title'" do
-        Puppet::Parser::Resource.new("resource", "testing", :source => "source", :scope => "scope")[:title].should == "testing"
+        Puppet::Parser::Resource.new("resource", "testing", :source => @source, :scope => @scope)[:title].should == "testing"
     end
 
     describe "when initializing" do
         before do
-            @arguments = {:scope => stub('scope', :source => mock('source'))}
+            @arguments = {:scope => @scope}
         end
 
         it "should fail unless #{name.to_s} is specified" do
@@ -132,17 +128,6 @@ describe Puppet::Parser::Resource do
         end
     end
 
-    describe "when refering to a resource with name canonicalization" do
-        before do
-            @arguments = {:scope => stub('scope', :source => mock('source'))}
-        end
-
-        it "should canonicalize its own name" do
-            res = Puppet::Parser::Resource.new("file", "/path/", @arguments)
-            res.ref.should == "File[/path]"
-        end
-    end
-
     describe "when evaluating" do
         it "should evaluate the associated AST definition" do
             definition = newdefine "mydefine"
@@ -294,7 +279,7 @@ describe Puppet::Parser::Resource do
     describe "when being tagged" do
         before do
             @scope_resource = stub 'scope_resource', :tags => %w{srone srtwo}
-            @scope = stub 'scope', :resource => @scope_resource
+            @scope.stubs(:resource).returns @scope_resource
             @resource = Puppet::Parser::Resource.new("file", "yay", :scope => @scope, :source => mock('source'))
         end
 
@@ -448,8 +433,7 @@ describe Puppet::Parser::Resource do
 
     describe "when being converted to a resource" do
         before do
-            @source = stub 'scope', :name => "myscope"
-            @parser_resource = mkresource :source => @source, :params => {:foo => "bar", :fee => "fum"}
+            @parser_resource = mkresource :scope => @scope, :params => {:foo => "bar", :fee => "fum"}
         end
 
         it "should create an instance of Puppet::Resource" do
@@ -537,7 +521,7 @@ describe Puppet::Parser::Resource do
 
     describe "when validating" do
         it "should check each parameter" do
-            resource = Puppet::Parser::Resource.new :foo, "bar", :scope => stub("scope"), :source => stub("source")
+            resource = Puppet::Parser::Resource.new :foo, "bar", :scope => @scope, :source => stub("source")
             resource[:one] = :two
             resource[:three] = :four
             resource.expects(:validate_parameter).with(:one)
@@ -546,7 +530,7 @@ describe Puppet::Parser::Resource do
         end
 
         it "should raise a parse error when there's a failure" do
-            resource = Puppet::Parser::Resource.new :foo, "bar", :scope => stub("scope"), :source => stub("source")
+            resource = Puppet::Parser::Resource.new :foo, "bar", :scope => @scope, :source => stub("source")
             resource[:one] = :two
             resource.expects(:validate_parameter).with(:one).raises ArgumentError
             lambda { resource.send(:validate) }.should raise_error(Puppet::ParseError)
@@ -556,7 +540,7 @@ describe Puppet::Parser::Resource do
     describe "when setting parameters" do
         before do
             @source = newclass "foobar"
-            @resource = Puppet::Parser::Resource.new :foo, "bar", :scope => stub("scope"), :source => @source
+            @resource = Puppet::Parser::Resource.new :foo, "bar", :scope => @scope, :source => @source
         end
 
         it "should accept Param instances and add them to the parameter list" do
diff --git a/spec/unit/resource.rb b/spec/unit/resource.rb
index c97cd634f..834a5b078 100755
--- a/spec/unit/resource.rb
+++ b/spec/unit/resource.rb
@@ -160,6 +160,21 @@ describe Puppet::Resource do
         resource.resource_type.should equal(type)
     end
 
+    it "should use its namespaces to set its type name when looking up defined resource types" do
+        type = Puppet::Resource::Type.new(:definition, "foo::bar")
+        Puppet::Node::Environment.new.known_resource_types.add type
+        resource = Puppet::Resource.new("bar", "/my/file", :namespaces => ["foo"])
+        resource.type.should == "Foo::Bar"
+    end
+
+    it "should look up its resource type when set manually" do
+        type = Puppet::Resource::Type.new(:definition, "foo::bar")
+        Puppet::Node::Environment.new.known_resource_types.add type
+        resource = Puppet::Resource.new("foo", "/my/file", :namespaces => ["foo"])
+        resource.type = "bar"
+        resource.type.should == "Foo::Bar"
+    end
+
     it "should use its namespaces when looking up host classes" do
         resource = Puppet::Resource.new("class", "bar", :namespaces => ["foo"])
         type = Puppet::Resource::Type.new(:hostclass, "foo::bar")
@@ -168,6 +183,13 @@ describe Puppet::Resource do
         resource.resource_type.should equal(type)
     end
 
+    it "should consider a class whose name is an empty string to be the main class" do
+        type = Puppet::Resource::Type.new(:hostclass, "")
+        Puppet::Node::Environment.new.known_resource_types.add type
+
+        resource = Puppet::Resource.new("class", "").type.should == :main
+    end
+
     it "should return nil when looking up resource types that don't exist" do
         Puppet::Resource.new("foobar", "bar").resource_type.should be_nil
     end
@@ -217,6 +239,17 @@ describe Puppet::Resource do
         Puppet::Resource.new("file", "/foo").should_not == Puppet::Resource.new("file", "/f")
     end
 
+    describe "when refering to a resource with name canonicalization" do
+        before do
+        end
+
+        it "should canonicalize its own name" do
+            res = Puppet::Resource.new("file", "/path/")
+            res.title.should == "/path"
+            res.ref.should == "File[/path]"
+        end
+    end
+
     describe "when managing parameters" do
         before do
             @resource = Puppet::Resource.new("file", "/my/file")
diff --git a/spec/unit/type/resources.rb b/spec/unit/type/resources.rb
index 480b6c00d..3e65aec14 100644
--- a/spec/unit/type/resources.rb
+++ b/spec/unit/type/resources.rb
@@ -52,6 +52,19 @@ describe resources do
                 @resources.generate.collect { |r| r.ref }.should_not include(@host1.ref)
             end
 
+            it "should not include the skipped users" do
+                res = Puppet::Type.type(:resources).new :name => :user, :purge => true
+                res.catalog = Puppet::Resource::Catalog.new
+
+                users = [
+                    Puppet::Type.type(:user).new(:name => "root")
+                ]
+                Puppet::Type.type(:user).expects(:instances).returns users
+                list = res.generate
+
+                names = list.collect { |r| r[:name] }
+                names.should_not be_include("root")
+            end
 
             describe "when generating a purgeable resource" do
                 it "should be included in the generated resources" do