dotgit: add CheckAndSetReference tests

Author: strib

plumbing/storer/reference.go

@@ -16,6 +16,10 @@ var ErrMaxResolveRecursion = errors.New("max. recursion level reached")
 // ReferenceStorer is a generic storage of references.
 type ReferenceStorer interface {
 	SetReference(*plumbing.Reference) error
+	// CheckAndSetReference sets the reference `new`, but if `old` is
+	// not `nil`, it first checks that the current stored value for
+	// `old.Name()` matches the given reference value in `old`.  If
+	// not, it returns an error and doesn't update `new`.
 	CheckAndSetReference(new, old *plumbing.Reference) error
 	Reference(plumbing.ReferenceName) (*plumbing.Reference, error)
 	IterReferences() (ReferenceIter, error)

remote_test.go

@@ -308,8 +308,8 @@ func (s *RemoteSuite) doTestFetchNoErrAlreadyUpToDate(c *C, url string) {
 	c.Assert(err, Equals, NoErrAlreadyUpToDate)
 }
 
-func (s *RemoteSuite) TestFetchFastForward(c *C) {
-	r := newRemote(memory.NewStorage(), &config.RemoteConfig{
+func (s *RemoteSuite) testFetchFastForward(c *C, sto storage.Storer) {
+	r := newRemote(sto, &config.RemoteConfig{
 		URLs: []string{s.GetBasicLocalRepositoryURL()},
 	})
 
@@ -350,6 +350,23 @@ func (s *RemoteSuite) TestFetchFastForward(c *C) {
 	})
 }
 
+func (s *RemoteSuite) TestFetchFastForwardMem(c *C) {
+	s.testFetchFastForward(c, memory.NewStorage())
+}
+
+func (s *RemoteSuite) TestFetchFastForwardFS(c *C) {
+	dir, err := ioutil.TempDir("", "fetch")
+	c.Assert(err, IsNil)
+
+	defer os.RemoveAll(dir) // clean up
+
+	fss, err := filesystem.NewStorage(osfs.New(dir))
+	c.Assert(err, IsNil)
+
+	// This exercises `storage.filesystem.Storage.CheckAndSetReference()`.
+	s.testFetchFastForward(c, fss)
+}
+
 func (s *RemoteSuite) TestString(c *C) {
 	r := newRemote(nil, &config.RemoteConfig{
 		Name: "foo",

storage/filesystem/internal/dotgit/dotgit_test.go

@@ -55,10 +55,11 @@ func (s *SuiteDotGit) TestSetRefs(c *C) {
 	fs := osfs.New(tmp)
 	dir := New(fs)
 
-	err = dir.SetRef(plumbing.NewReferenceFromStrings(
+	firstFoo := plumbing.NewReferenceFromStrings(
 		"refs/heads/foo",
 		"e8d3ffab552895c19b9fcf7aa264d277cde33881",
-	), nil)
+	)
+	err = dir.SetRef(firstFoo, nil)
 
 	c.Assert(err, IsNil)
 
@@ -105,6 +106,20 @@ func (s *SuiteDotGit) TestSetRefs(c *C) {
 	c.Assert(ref, NotNil)
 	c.Assert(ref.Hash().String(), Equals, "e8d3ffab552895c19b9fcf7aa264d277cde33881")
 
+	// Check that SetRef with a non-nil `old` works.
+	err = dir.SetRef(plumbing.NewReferenceFromStrings(
+		"refs/heads/foo",
+		"6ecf0ef2c2dffb796033e5a02219af86ec6584e5",
+	), firstFoo)
+	c.Assert(err, IsNil)
+
+	// `firstFoo` is no longer the right `old` reference, so this
+	// should fail.
+	err = dir.SetRef(plumbing.NewReferenceFromStrings(
+		"refs/heads/foo",
+		"6ecf0ef2c2dffb796033e5a02219af86ec6584e5",
+	), firstFoo)
+	c.Assert(err, NotNil)
 }
 
 func (s *SuiteDotGit) TestRefsFromPackedRefs(c *C) {
@@ -192,7 +207,7 @@ func (s *SuiteDotGit) TestRemoveRefFromReferenceFileAndPackedRefs(c *C) {
 	err := dir.SetRef(plumbing.NewReferenceFromStrings(
 		"refs/remotes/origin/branch",
 		"e8d3ffab552895c19b9fcf7aa264d277cde33881",
-	))
+	), nil)
 
 	// Make sure it only appears once in the refs list.
 	refs, err := dir.Refs()

storage/memory/storage.go

@@ -204,16 +204,17 @@ func (r ReferenceStorage) SetReference(ref *plumbing.Reference) error {
 }
 
 func (r ReferenceStorage) CheckAndSetReference(ref, old *plumbing.Reference) error {
-	if ref != nil {
-		if old != nil {
-			tmp := r[ref.Name()]
-			if tmp != nil && tmp.Hash() != old.Hash() {
-				return ErrRefHasChanged
-			}
-		}
-		r[ref.Name()] = ref
+	if ref == nil {
+		return nil
 	}
 
+	if old != nil {
+		tmp := r[ref.Name()]
+		if tmp != nil && tmp.Hash() != old.Hash() {
+			return ErrRefHasChanged
+		}
+	}
+	r[ref.Name()] = ref
 	return nil
 }