Tests for Git revision property, cleanup

lib/puppet/provider/vcsrepo/git.rb

@@ -21,12 +21,8 @@ Puppet::Type.type(:vcsrepo).provide(:git) do
   end
   
   def revision
-    current = Dir.chdir(@resource.value(:path)) do
-      git('rev-parse', 'HEAD')
-    end
-    canonical = Dir.chdir(@resource.value(:path)) do
-      git('rev-parse', @resource.value(:revision))
-    end
+    current   = at_path { git('rev-parse', 'HEAD') }
+    canonical = at_path { git('rev-parse', @resource.value(:revision)) }
     if current == canonical
       @resource.value(:revision)
     else
@@ -64,8 +60,14 @@ Puppet::Type.type(:vcsrepo).provide(:git) do
     end
   end
 
-  def at_path(&block)
-    Dir.chdir(@resource.value(:path), &block)
+  # Note: We don't rely on Dir.chdir's behavior of automatically returning the
+  # value of the last statement -- for easier stubbing.
+  def at_path(&block) #:nodoc:
+    value = nil
+    Dir.chdir(@resource.value(:path)) do
+      value = yield
+    end
+    value
   end
 
 end

lib/puppet/provider/vcsrepo/svn.rb

@@ -60,7 +60,9 @@ Puppet::Type.type(:vcsrepo).provide(:svn) do
     end
   end
 
-  def at_path(&block)
+  # Note: We don't rely on Dir.chdir's behavior of automatically returning the
+  # value of the last statement -- for easier stubbing.
+  def at_path(&block) #:nodoc:
     value = nil
     Dir.chdir(@resource.value(:path)) do
       value = yield

spec/unit/puppet/provider/vcsrepo/git_spec.rb

@@ -10,7 +10,7 @@ describe provider_class do
     @path = '/tmp/vcsrepo'
   end
 
-  context 'when creating' do
+  describe 'when creating' do
     context "when a source is given" do
       context "and when a revision is given" do
         it "should execute 'git clone' and 'git reset'" do
@@ -44,7 +44,7 @@ describe provider_class do
     end
   end
 
-  context 'when destroying' do
+  describe 'when destroying' do
     it "it should remove the directory" do
       @resource.expects(:value).with(:path).returns(@path).at_least_once
       FileUtils.expects(:rm_rf).with(@path)
@@ -52,7 +52,7 @@ describe provider_class do
     end
   end
 
-  context "when checking existence" do
+  describe "when checking existence" do
     it "should check for the directory" do
       @resource.expects(:value).with(:path).returns(@path)
       File.expects(:directory?).with(@path)
@@ -60,6 +60,61 @@ describe provider_class do
     end
   end
 
-  context "when checking 'revision' property"
+  describe "when checking the revision property" do
+    context "when given a non-SHA ref as the resource revision" do
+      context "when its SHA is not different than the curent SHA" do
+        it "should return the ref" do
+          @resource.expects(:value).with(:path).returns(@path).at_least_once
+          @provider.expects(:git).with('rev-parse', 'HEAD').returns('currentsha')
+          @resource.expects(:value).with(:revision).returns('a-tag').at_least_once
+          @provider.expects(:git).with('rev-parse', 'a-tag').returns('currentsha')
+          Dir.expects(:chdir).with(@path).twice.yields
+          @provider.revision.should == 'a-tag'
+        end
+      end
+      context "when its SHA is different than the current SHA" do
+        it "should return the current SHA" do
+          @resource.expects(:value).with(:path).returns(@path).at_least_once
+          @provider.expects(:git).with('rev-parse', 'HEAD').returns('currentsha')
+          @resource.expects(:value).with(:revision).returns('a-tag').at_least_once
+          @provider.expects(:git).with('rev-parse', 'a-tag').returns('othersha')
+          Dir.expects(:chdir).with(@path).twice.yields
+          @provider.revision.should == 'currentsha'
+        end          
+      end
+    end
+    context "when given a SHA ref as the resource revision" do
+      context "when it is the same as the current SHA" do
+        it "should return it" do
+          @resource.expects(:value).with(:path).returns(@path).at_least_once
+          @provider.expects(:git).with('rev-parse', 'HEAD').returns('currentsha')
+          @resource.expects(:value).with(:revision).returns('currentsha').at_least_once
+          @provider.expects(:git).with('rev-parse', 'currentsha').returns('currentsha')
+          Dir.expects(:chdir).with(@path).twice.yields
+          @provider.revision.should == 'currentsha'
+        end
+      end
+      context "when it is not the same as the current SHA" do
+        it "should return the current SHA" do
+          @resource.expects(:value).with(:path).returns(@path).at_least_once
+          @provider.expects(:git).with('rev-parse', 'HEAD').returns('currentsha')
+          @resource.expects(:value).with(:revision).returns('othersha').at_least_once
+          @provider.expects(:git).with('rev-parse', 'othersha').returns('othersha')
+          Dir.expects(:chdir).with(@path).twice.yields
+          @provider.revision.should == 'currentsha'
+        end
+      end
+    end
+  end
+  
+  describe "when setting the revision property" do
+    it "should use 'git fetch' and 'git reset'" do
+      @resource.expects(:value).with(:path).returns(@path).at_least_once
+      @provider.expects('git').with('fetch', 'origin')
+      Dir.expects(:chdir).with(@path).at_least_once.yields
+      @provider.expects('git').with('reset', '--hard', 'carcar')
+      @provider.revision = 'carcar'
+    end
+  end
 
 end