cli/config/credentials: add test for save being idempotent

Test case for d3f6867e4d

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 3c78069240)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

cli/config/credentials/file_store.go

@@ -25,7 +25,8 @@ func NewFileStore(file store) Store {
 	return &fileStore{file: file}
 }
 
-// Erase removes the given credentials from the file store.
+// Erase removes the given credentials from the file store.This function is
+// idempotent and does not update the file if credentials did not change.
 func (c *fileStore) Erase(serverAddress string) error {
 	if _, exists := c.file.GetAuthConfigs()[serverAddress]; !exists {
 		// nothing to do; no credentials found for the given serverAddress

cli/config/credentials/file_store_test.go

@@ -10,9 +10,15 @@ import (
 
 type fakeStore struct {
 	configs map[string]types.AuthConfig
+	saveFn  func(*fakeStore) error
 }
 
 func (f *fakeStore) Save() error {
+	if f.saveFn != nil {
+		// Pass a reference to the fakeStore itself in case saveFn
+		// wants to access it.
+		return f.saveFn(f)
+	}
 	return nil
 }
 
@@ -21,7 +27,78 @@ func (f *fakeStore) GetAuthConfigs() map[string]types.AuthConfig {
 }
 
 func (f *fakeStore) GetFilename() string {
-	return "/tmp/docker-fakestore"
+	return "no-config.json"
+}
+
+// TestFileStoreIdempotent verifies that the config-file isn't updated
+// if nothing changed.
+func TestFileStoreIdempotent(t *testing.T) {
+	var saveCount, expectedSaveCount int
+
+	s := NewFileStore(&fakeStore{
+		configs: map[string]types.AuthConfig{},
+		saveFn: func(*fakeStore) error {
+			saveCount++
+			return nil
+		},
+	})
+	authOne := types.AuthConfig{
+		Auth:          "super_secret_token",
+		Email:         "foo@example.com",
+		ServerAddress: "https://example.com",
+	}
+	authTwo := types.AuthConfig{
+		Auth:          "also_super_secret_token",
+		Email:         "bar@example.com",
+		ServerAddress: "https://other.example.com",
+	}
+
+	expectedSaveCount = 1
+	t.Run("store new credentials", func(t *testing.T) {
+		assert.NilError(t, s.Store(authOne))
+		retrievedAuth, err := s.Get(authOne.ServerAddress)
+		assert.NilError(t, err)
+		assert.Check(t, is.Equal(retrievedAuth, authOne))
+		assert.Check(t, is.Equal(saveCount, expectedSaveCount))
+	})
+	t.Run("store same credentials is a no-op", func(t *testing.T) {
+		assert.NilError(t, s.Store(authOne))
+		retrievedAuth, err := s.Get(authOne.ServerAddress)
+		assert.NilError(t, err)
+		assert.Check(t, is.Equal(retrievedAuth, authOne))
+		assert.Check(t, is.Equal(saveCount, expectedSaveCount), "should not have saved if nothing changed")
+	})
+	t.Run("store other credentials", func(t *testing.T) {
+		expectedSaveCount++
+		assert.NilError(t, s.Store(authTwo))
+		retrievedAuth, err := s.Get(authTwo.ServerAddress)
+		assert.NilError(t, err)
+		assert.Check(t, is.Equal(retrievedAuth, authTwo))
+		assert.Check(t, is.Equal(saveCount, expectedSaveCount))
+	})
+	t.Run("erase credentials", func(t *testing.T) {
+		expectedSaveCount++
+		assert.NilError(t, s.Erase(authOne.ServerAddress))
+		retrievedAuth, err := s.Get(authOne.ServerAddress)
+		assert.NilError(t, err)
+		assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{}))
+		assert.Check(t, is.Equal(saveCount, expectedSaveCount))
+	})
+	t.Run("erase non-existing credentials is a no-op", func(t *testing.T) {
+		assert.NilError(t, s.Erase(authOne.ServerAddress))
+		retrievedAuth, err := s.Get(authOne.ServerAddress)
+		assert.NilError(t, err)
+		assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{}))
+		assert.Check(t, is.Equal(saveCount, expectedSaveCount), "should not have saved if nothing changed")
+	})
+	t.Run("erase other credentials", func(t *testing.T) {
+		expectedSaveCount++
+		assert.NilError(t, s.Erase(authTwo.ServerAddress))
+		retrievedAuth, err := s.Get(authTwo.ServerAddress)
+		assert.NilError(t, err)
+		assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{}))
+		assert.Check(t, is.Equal(saveCount, expectedSaveCount))
+	})
 }
 
 func TestFileStoreAddCredentials(t *testing.T) {