Merge pull request #13 from yipit-cookbooks/bugfix-unwind

Bugfix for off-by-1 error in ResourceCollection#delete

lib/chef/rewind.rb

@@ -72,7 +72,13 @@ def delete_resource(resource_id)
 
       # assumes `resource_id` is the same as `Chef::Resource#to_s`
       @resources.delete_if {|r| r.to_s == resource_id }
+      resource_index_value = @resources_by_name[resource_id]
+      @resources_by_name.each do |k, v|
+        @resources_by_name[k] = v - 1 if v > resource_index_value
+      end
+
       @resources_by_name.delete resource_id
+
     end
   end
 end

spec/support/lib/chef/provider/cat.rb

@@ -0,0 +1,21 @@
+class Chef
+  class Provider
+    class Cat < Chef::Provider
+      class CatError < RuntimeError
+      end
+
+      def load_current_resource
+        true
+      end
+
+      def action_sell
+        true
+      end
+
+      def action_blowup
+        raise CatError, "CAT BLOWUP"
+      end
+
+    end
+  end
+end

spec/support/lib/chef/provider/zen_master.rb

@@ -0,0 +1,13 @@
+class Chef
+  class Provider
+    class ZenMaster < Chef::Provider
+      def load_current_resource
+        true
+      end
+
+      def action_change
+        true
+      end
+    end
+  end
+end

spec/support/lib/chef/resource/cat.rb

@@ -19,13 +19,11 @@
 class Chef
   class Resource
     class Cat < Chef::Resource
-
-      attr_accessor :action
-
       def initialize(name, run_context=nil)
         @resource_name = :cat
         super
-        @action = "sell"
+        @action = :nothing
+        @allowed_actions = [:nothing, :sell, :blowup]
       end
 
       def pretty_kitty(arg=nil)

spec/support/lib/chef/resource/zen_master.rb

@@ -27,12 +27,18 @@ class ZenMaster < Chef::Resource
       def initialize(name, run_context=nil)
         @resource_name = :zen_master
         super
+        @action = :nothing
+        @allowed_actions = [:change, :nothing]
       end
 
       def peace(tf)
         @peace = tf
       end
 
+      def updated_by_last_action?
+        true
+      end
+
       def something(arg=nil)
         set_if_args(@something, arg) do
           case arg

spec/unwind_recipe_spec.rb

@@ -2,17 +2,21 @@
 require 'chef/rewind'
 
 describe Chef::Recipe do
-  
+
   before(:each) do
     @cookbook_repo = File.expand_path(File.join(File.dirname(__FILE__), "..", "data", "cookbooks"))
     cl = Chef::CookbookLoader.new(@cookbook_repo)
     cl.load_cookbooks
     @cookbook_collection = Chef::CookbookCollection.new(cl)
     @node = Chef::Node.new
+    @node.name "latte"
+    @node.automatic[:platform] = "mac_os_x"
+    @node.automatic[:platform_version] = "10.5.1"
     @node.normal[:tags] = Array.new
     @events = Chef::EventDispatch::Dispatcher.new
     @run_context = Chef::RunContext.new(@node, @cookbook_collection, @events)
     @recipe = Chef::Recipe.new("hjk", "test", @run_context)
+    @runner = Chef::Runner.new(@run_context)
   end
 
 
@@ -21,15 +25,38 @@
       @recipe.zen_master "foobar" do
         peace false
       end
-      
+
       @recipe.unwind "zen_master[foobar]"
 
       resources = @run_context.resource_collection.all_resources
       resources.length.should == 0
     end
 
+    it "should define resource completely when unwind is called" do
+      @recipe.zen_master "foo" do
+        action :nothing
+        peace false
+      end
+      @recipe.cat "blanket" do
+      end
+      @recipe.zen_master "bar" do
+        action :nothing
+        peace false
+      end
+
+      @recipe.unwind "zen_master[foo]"
+
+      @recipe.zen_master "foobar" do
+        peace true
+        action :change
+        notifies :blowup, "cat[blanket]"
+      end
+
+      lambda { @runner.converge }.should raise_error(Chef::Provider::Cat::CatError)
+    end
+
     it "should throw an error when unwinding a nonexistent resource" do
-      lambda do 
+      lambda do
         @recipe.unwind "zen_master[foobar]"
       end.should raise_error(Chef::Exceptions::ResourceNotFound)
     end