From 7d35ae8fed989ef3edb8a304f625786a04ee5faa Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 18 Mar 2008 18:14:52 -0500
Subject: Refactoring how the catalog creation handles errors. Previously, if
 there were an error creating a resource, the error would propagate leaving
 any previously created resources still in memory.

Now, resources are removed by default when an error
happens during instantiation, and the error propagates
to the caller so that they can log or whatever.

This also allows the Settings class to correctly and
separately handle the cases where we can't create the
catalog (which should never happen in normal usage but
was happening because of this error -- later catalogs
couldn't be created because earlier catalogs left
resources lying around) from those where we can't
apply the catalog.
---
 spec/unit/other/transbucket.rb | 37 ++++++++++++++++++++++---------------
 spec/unit/util/settings.rb     | 11 +++++++++++
 2 files changed, 33 insertions(+), 15 deletions(-)

(limited to 'spec')

diff --git a/spec/unit/other/transbucket.rb b/spec/unit/other/transbucket.rb
index 3904e39fe..4494f2abb 100755
--- a/spec/unit/other/transbucket.rb
+++ b/spec/unit/other/transbucket.rb
@@ -87,42 +87,54 @@ describe Puppet::TransBucket, " when generating a catalog" do
         @top.push(@topobj)
         @top.push(@middle)
 
-        @config = @top.to_catalog
-
         @users = %w{top middle bottom}
         @fakes = %w{Fake[bottom] Fake[middle] Fake[top]}
     end
 
+    after do
+        Puppet::Type.allclear
+    end
+
     it "should convert all transportable objects to RAL resources" do
+        @catalog = @top.to_catalog
         @users.each do |name|
-            @config.vertices.find { |r| r.class.name == :user and r.title == name }.should be_instance_of(Puppet::Type.type(:user))
+            @catalog.vertices.find { |r| r.class.name == :user and r.title == name }.should be_instance_of(Puppet::Type.type(:user))
         end
     end
 
+    it "should fail if any transportable resources fail to convert to RAL resources" do
+        @bottomobj.expects(:to_type).raises ArgumentError
+        lambda { @bottom.to_catalog }.should raise_error(ArgumentError)
+    end
+
     it "should convert all transportable buckets to RAL components" do
+        @catalog = @top.to_catalog
         @fakes.each do |name|
-            @config.vertices.find { |r| r.class.name == :component and r.title == name }.should be_instance_of(Puppet::Type.type(:component))
+            @catalog.vertices.find { |r| r.class.name == :component and r.title == name }.should be_instance_of(Puppet::Type.type(:component))
         end
     end
 
     it "should add all resources to the graph's resource table" do
-        @config.resource("fake[top]").should equal(@top)
+        @catalog = @top.to_catalog
+        @catalog.resource("fake[top]").should equal(@top)
     end
 
     it "should finalize all resources" do
-        @config.vertices.each do |vertex| vertex.should be_finalized end
+        @catalog = @top.to_catalog
+        @catalog.vertices.each do |vertex| vertex.should be_finalized end
     end
 
     it "should only call to_type on each resource once" do
-        @topobj.expects(:to_type)
-        @bottomobj.expects(:to_type)
-        Puppet::Type.allclear
+        # We just raise exceptions here because we're not interested in
+        # what happens with the result, only that the method only
+        # gets called once.
+        resource = @topobj.to_type
+        @topobj.expects(:to_type).once.returns resource
         @top.to_catalog
     end
 
     it "should set each TransObject's catalog before converting to a RAL resource" do
         @middleobj.expects(:catalog=).with { |c| c.is_a?(Puppet::Node::Catalog) }
-        Puppet::Type.allclear
         @top.to_catalog
     end
 
@@ -130,13 +142,8 @@ describe Puppet::TransBucket, " when generating a catalog" do
         # each bucket is seen twice in the loop, so we have to handle the case where the config
         # is set twice
         @bottom.expects(:catalog=).with { |c| c.is_a?(Puppet::Node::Catalog) }.at_least_once
-        Puppet::Type.allclear
         @top.to_catalog
     end
-
-    after do
-        Puppet::Type.allclear
-    end
 end
 
 describe Puppet::TransBucket, " when serializing" do
diff --git a/spec/unit/util/settings.rb b/spec/unit/util/settings.rb
index fbd638663..a6b358462 100755
--- a/spec/unit/util/settings.rb
+++ b/spec/unit/util/settings.rb
@@ -605,6 +605,17 @@ describe Puppet::Util::Settings, " when being used to manage the host machine" d
         lambda { trans.to_catalog }.should_not raise_error
     end
 
+    it "should do nothing if a catalog cannot be created" do
+        bucket = mock 'bucket'
+        catalog = mock 'catalog'
+
+        @settings.expects(:to_transportable).returns bucket
+        bucket.expects(:to_catalog).raises RuntimeError
+        catalog.expects(:apply).never
+
+        @settings.use(:mysection)
+    end
+
     it "should clear the catalog after applying" do
         bucket = mock 'bucket'
         catalog = mock 'catalog'
-- 
cgit