Finally writing unit tests for Transaction#trigger, and drastically simplifying the method in the process.

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

2 files changed, 47 insertions, 59 deletions

diff --git a/lib/puppet/pgraph.rb b/lib/puppet/pgraph.rb
index 5ae42045f..393f88449 100644
--- a/lib/puppet/pgraph.rb
+++ b/lib/puppet/pgraph.rb
@@ -96,13 +96,7 @@ class Puppet::PGraph < GRATR::Digraph
             # This vertex in the graph.
             adjacent(source, :direction => :out, :type => :edges).find_all do |edge|
                 edge.match?(event.event)
-            end.each { |edge|
-                target = edge.target
-                if target.respond_to?(:ref)
-                    source.info "Scheduling %s of %s" %
-                        [edge.callback, target.ref]
-                end
-            }
+            end
         end.flatten
     end
     
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 10d17471d..34717d7de 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -224,7 +224,7 @@ class Transaction
         # the parent resource in so it will override the source in the events,
         # since eval_generated children can't have direct relationships.
         @relgraph.matching_edges(events, resource).each do |edge|
-            @targets[edge.target] << edge
+            set_trigger(edge)
         end
 
         # And return the events for collection
@@ -332,7 +332,11 @@ class Transaction
     # this should only be called by a Puppet::Type::Component resource now
     # and it should only receive an array
     def initialize(resources)
-        @resources = resources.to_graph
+        if resources.is_a?(Puppet::PGraph)
+            @resources = resources
+        else
+            @resources = resources.to_graph
+        end
 
         @resourcemetrics = {
             :total => @resources.vertices.length,
@@ -510,6 +514,15 @@ class Transaction
     def scheduled?(resource)
         self.ignoreschedules or resource.scheduled?
     end
+
+    # Set an edge to be triggered when we evaluate its target.
+    def set_trigger(edge)
+        return unless edge.callback
+        if edge.target.respond_to?(:ref)
+            edge.source.info "Scheduling %s of %s" % [edge.callback, edge.target.ref]
+        end
+        @targets[edge.target] << edge
+    end
     
     # Should this resource be skipped?
     def skip?(resource)
@@ -555,70 +568,51 @@ class Transaction
     
     # Are there any edges that target this resource?
     def targeted?(resource)
-        @targets[resource]
+        # The default value is a new array so we have to test the length of it.
+        @targets.include?(resource) and @targets[resource].length > 0
     end
 
     # Trigger any subscriptions to a child.  This does an upwardly recursive
     # search -- it triggers the passed resource, but also the resource's parent
     # and so on up the tree.
-    def trigger(child)
-        obj = child
+    def trigger(resource)
+        return nil unless targeted?(resource)
         callbacks = Hash.new { |hash, key| hash[key] = [] }
-        sources = Hash.new { |hash, key| hash[key] = [] }
 
         trigged = []
-        while obj
-            if @targets.include?(obj)
-                callbacks.clear
-                sources.clear
-                @targets[obj].each do |edge|
-                    # Some edges don't have callbacks
-                    next unless edge.callback
-                    
-                    # Collect all of the subs for each callback
-                    callbacks[edge.callback] << edge
-
-                    # And collect the sources for logging
-                    sources[edge.source] << edge.callback
-                end
-
-                sources.each do |source, callbacklist|
-                    obj.debug "%s[%s] results in triggering %s" %
-                        [source.class.name, source.name, callbacklist.join(", ")]
-                end
+        @targets[resource].each do |edge|
+            # Collect all of the subs for each callback
+            callbacks[edge.callback] << edge
+        end
 
-                callbacks.each do |callback, subs|
-                    message = "Triggering '%s' from %s dependencies" %
-                        [callback, subs.length]
-                    obj.notice message
-                    # At this point, just log failures, don't try to react
-                    # to them in any way.
-                    begin
-                        obj.send(callback)
-                        @resourcemetrics[:restarted] += 1
-                    rescue => detail
-                        obj.err "Failed to call %s on %s: %s" %
-                            [callback, obj, detail]
-
-                        @resourcemetrics[:failed_restarts] += 1
-
-                        if Puppet[:trace]
-                            puts detail.backtrace
-                        end
-                    end
+        callbacks.each do |callback, subs|
+            message = "Triggering '%s' from %s dependencies" %
+                [callback, subs.length]
+            resource.notice message
+            # At this point, just log failures, don't try to react
+            # to them in any way.
+            begin
+                resource.send(callback)
+                @resourcemetrics[:restarted] += 1
+            rescue => detail
+                resource.err "Failed to call %s on %s: %s" %
+                    [callback, resource, detail]
 
-                    # And then add an event for it.
-                    trigged << Puppet::Event.new(
-                        :event => :triggered,
-                        :transaction => self,
-                        :source => obj
-                    )
+                @resourcemetrics[:failed_restarts] += 1
 
-                    triggered(obj, callback)
+                if Puppet[:trace]
+                    puts detail.backtrace
                 end
             end
 
-            obj = obj.parent
+            # And then add an event for it.
+            trigged << Puppet::Event.new(
+                :event => :triggered,
+                :transaction => self,
+                :source => resource
+            )
+
+            triggered(resource, callback)
         end
 
         if trigged.empty?