Fixing a few testing bugs that have crept in, and fixing a self-reference problem when configuring, graphing, and setting graphdir manually.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2052 980ebf18-57e1-0310-9a29-db15c13687c0

12 files changed, 37 insertions, 24 deletions

diff --git a/bin/puppet b/bin/puppet
index ee60415d5..7062e4050 100755
--- a/bin/puppet
+++ b/bin/puppet
@@ -187,7 +187,7 @@ begin
     client.apply
 rescue => detail
     $stderr.puts detail
-    if Puppet[:debug]
+    if Puppet[:trace]
         puts detail.backtrace
     end
     exit(1)
diff --git a/lib/puppet/client/master.rb b/lib/puppet/client/master.rb
index 79eb746a5..e20203fd1 100644
--- a/lib/puppet/client/master.rb
+++ b/lib/puppet/client/master.rb
@@ -139,12 +139,8 @@ class Puppet::Client::MasterClient < Puppet::Client
 
     # Cache the config
     def cache(text)
-        Puppet.config.use(:puppet, :sslcertificates, :puppetd)
         Puppet.info "Caching configuration at %s" % self.cachefile
         confdir = File.dirname(Puppet[:localconfig])
-        #unless FileTest.exists?(confdir)
-        #    Puppet.recmkdir(confdir, 0770)
-        #end
         File.open(self.cachefile + ".tmp", "w", 0660) { |f|
             f.print text
         }
@@ -167,7 +163,6 @@ class Puppet::Client::MasterClient < Puppet::Client
     # Initialize and load storage
     def dostorage
         begin
-            Puppet::Storage.init
             Puppet::Storage.load
         rescue => detail
             if Puppet[:trace]
@@ -276,6 +271,7 @@ class Puppet::Client::MasterClient < Puppet::Client
     
     # Just so we can specify that we are "the" instance.
     def initialize(*args)
+        Puppet.config.use(:puppet, :sslcertificates, :puppetd)
         super
 
         @configtime = Time.now
diff --git a/lib/puppet/config.rb b/lib/puppet/config.rb
index bffcfbd48..23268e463 100644
--- a/lib/puppet/config.rb
+++ b/lib/puppet/config.rb
@@ -610,6 +610,7 @@ Generated on #{Time.now}.
             end
             trans = objects.evaluate
             trans.ignoretags = true
+            trans.configurator = true
             trans.evaluate
             if tags
                 Puppet[:tags] = tags
diff --git a/lib/puppet/provider/package/darwinport.rb b/lib/puppet/provider/package/darwinport.rb
index 36d87456f..adc722a66 100755
--- a/lib/puppet/provider/package/darwinport.rb
+++ b/lib/puppet/provider/package/darwinport.rb
@@ -46,7 +46,10 @@ Puppet::Type.type(:package).provide :darwinport do
         should = @model.should(:ensure)
 
         # Seems like you can always say 'upgrade'
-        port "upgrade", @model[:name]
+        output = port "upgrade", @model[:name]
+        if output =~ /^Error: No port/
+            raise Puppet::ExecutionFailure, "Could not find package %s" % @model[:name]
+        end
     end
 
     def query
diff --git a/lib/puppet/storage.rb b/lib/puppet/storage.rb
index 5f71c171b..e6c92cc65 100644
--- a/lib/puppet/storage.rb
+++ b/lib/puppet/storage.rb
@@ -45,9 +45,6 @@ module Puppet
         self.init
 
         def self.load
-            if Puppet[:statefile].nil?
-                raise Puppet::DevError, "Somehow the statefile is nil"
-            end
             Puppet.config.use(:puppet)
 
             unless File.exists?(Puppet[:statefile])
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index fe86c2595..b4d4c8512 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -7,7 +7,7 @@ require 'puppet/statechange'
 module Puppet
 class Transaction
     attr_accessor :component, :resources, :ignoreschedules, :ignoretags
-    attr_accessor :relgraph, :sorted_resources
+    attr_accessor :relgraph, :sorted_resources, :configurator
     
     attr_writer :tags
 
@@ -134,8 +134,10 @@ class Transaction
     # Do any necessary cleanup.  If we don't get rid of the graphs, the
     # contained resources might never get cleaned up.
     def cleanup
-        @generated.each do |resource|
-            resource.remove
+        if defined? @generated
+            @generated.each do |resource|
+                resource.remove
+            end
         end
         if defined? @relgraph
             @relgraph.clear
@@ -248,6 +250,8 @@ class Transaction
     def evaluate
         @count = 0
         
+        graph(@resources, :resources)
+        
         # Start logging.
         Puppet::Log.newdestination(@report)
         
@@ -342,11 +346,14 @@ class Transaction
 
     # Produce the graph files if requested.
     def graph(gr, name)
+        # We don't want to graph the configuration process.
+        return if self.configurator
+        
         return unless Puppet[:graph]
 
         file = File.join(Puppet[:graphdir], "%s.dot" % name.to_s)
         File.open(file, "w") { |f|
-            f.puts gr.to_dot
+            f.puts gr.to_dot("name" => name.to_s.capitalize)
         }
     end
 
@@ -359,8 +366,6 @@ class Transaction
             @resources = resources.to_graph
         end
 
-        graph(@resources, :resources)
-
         @resourcemetrics = {
             :total => @resources.vertices.length,
             :out_of_sync => 0,    # The number of resources that had changes
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index e55f9f5ae..a81424b45 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -357,7 +357,7 @@ class Type < Puppet::Element
 
     # convert to a string
     def to_s
-        self.title
+        self.ref
     end
 
     # Convert to a transportable object
diff --git a/lib/puppet/type/component.rb b/lib/puppet/type/component.rb
index 167e8963f..cafd1bbe7 100644
--- a/lib/puppet/type/component.rb
+++ b/lib/puppet/type/component.rb
@@ -147,7 +147,11 @@ module Puppet
         end
 
         def to_s
-            return "component(%s)" % self.title
+            if self.title =~ /\[/
+                return self.title
+            else
+                return "component(%s)" % self.title
+            end
         end
 	end
 end
diff --git a/lib/puppet/type/exec.rb b/lib/puppet/type/exec.rb
index 1be9c030c..7ab54ade6 100755
--- a/lib/puppet/type/exec.rb
+++ b/lib/puppet/type/exec.rb
@@ -593,10 +593,6 @@ module Puppet
             return output, status
         end
 
-        def to_s
-            "exec(%s)" % self.name
-        end
-
         def validatecmd(cmd)
             # if we're not fully qualified, require a path
             if cmd !~ /^\//
diff --git a/lib/puppet/util/posix.rb b/lib/puppet/util/posix.rb
index 37ec298bb..9131b394d 100755
--- a/lib/puppet/util/posix.rb
+++ b/lib/puppet/util/posix.rb
@@ -8,6 +8,10 @@ module Puppet::Util::POSIX
         end
         prefix = "get" + space.to_s
         if id.is_a?(Integer)
+            if id > 1000000
+                Puppet.err "Tried to get %s field for silly id %s" % [field, id]
+                return nil
+            end
             method = (prefix + idfield(space).to_s).intern
         else
             method = (prefix + "nam").intern
diff --git a/test/language/interpreter.rb b/test/language/interpreter.rb
index 774c317b2..0b7703a67 100755
--- a/test/language/interpreter.rb
+++ b/test/language/interpreter.rb
@@ -967,6 +967,7 @@ class TestInterpreter < Test::Unit::TestCase
             end
         end
         
+        mapper = mk_node_mapper
         # Make sure it gives the right response
         assert_equal("bpple\napple1 apple2 apple3\n",
             %x{#{mapper} apple})
@@ -976,8 +977,6 @@ class TestInterpreter < Test::Unit::TestCase
             assert_nil(interp.nodesearch_external("apple"),
                 "Interp#nodesearch_external defaulted to a non-nil response")
         }
-        
-        mapper = mk_node_mapper
         assert_nothing_raised { Puppet[:external_nodes] = mapper }
         
         node = nil
diff --git a/test/other/config.rb b/test/other/config.rb
index ca8fff4e7..3eb6b77a6 100755
--- a/test/other/config.rb
+++ b/test/other/config.rb
@@ -339,6 +339,10 @@ yay = /a/path
     end
 
     def test_usesection
+        # We want to make sure that config processes do not result in graphing.
+        Puppet[:graphdir] = tempfile()
+        Puppet[:graph] = true
+        Dir.mkdir(Puppet[:graphdir])
         c = mkconfig
 
         dir = tempfile()
@@ -365,6 +369,10 @@ yay = /a/path
         assert_nothing_raised("Could not reuse a section") {
             c.use(section)
         }
+        
+        # Make sure it didn't graph anything, which is the only real way
+        # to test that the transaction was marked as a configurator.
+        assert(Dir.entries(Puppet[:graphdir]).reject { |f| f =~ /^\.\.?$/ }.empty?, "Graphed config process")
 
         assert(FileTest.directory?(dir), "Did not create directory")
         assert(FileTest.exists?(otherfile), "Did not create file")