Skip to content

Commit 61baf2a

Browse files
thaJeztahthaJeztah
authored
Merge pull request #5570 from thaJeztah/credentials_coverage
cli/config/credentials: add test for save being idempotent
2 parents c34b80b + 3c78069 commit 61baf2a

3 files changed

Lines changed: 101 additions & 27 deletions

File tree

‎cli/config/credentials/file_store.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ func NewFileStore(file store) Store {
2828
return &fileStore{file: file}
2929
}
3030

31-
// Erase removes the given credentials from the file store.
31+
// Erase removes the given credentials from the file store.This function is
32+
// idempotent and does not update the file if credentials did not change.
3233
func (c *fileStore) Erase(serverAddress string) error {
3334
if _, exists := c.file.GetAuthConfigs()[serverAddress]; !exists {
3435
// nothing to do; no credentials found for the given serverAddress

‎cli/config/credentials/file_store_test.go‎

Lines changed: 83 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,15 @@ import (
1010

1111
type fakeStore struct {
1212
configs map[string]types.AuthConfig
13+
saveFn func(*fakeStore) error
1314
}
1415

1516
func (f *fakeStore) Save() error {
17+
if f.saveFn != nil {
18+
// Pass a reference to the fakeStore itself in case saveFn
19+
// wants to access it.
20+
return f.saveFn(f)
21+
}
1622
return nil
1723
}
1824

@@ -21,15 +27,82 @@ func (f *fakeStore) GetAuthConfigs() map[string]types.AuthConfig {
2127
}
2228

2329
func (f *fakeStore) GetFilename() string {
24-
return "/tmp/docker-fakestore"
30+
return "no-config.json"
2531
}
2632

27-
func newStore(auths map[string]types.AuthConfig) store {
28-
return &fakeStore{configs: auths}
33+
// TestFileStoreIdempotent verifies that the config-file isn't updated
34+
// if nothing changed.
35+
func TestFileStoreIdempotent(t *testing.T) {
36+
var saveCount, expectedSaveCount int
37+
38+
s := NewFileStore(&fakeStore{
39+
configs: map[string]types.AuthConfig{},
40+
saveFn: func(*fakeStore) error {
41+
saveCount++
42+
return nil
43+
},
44+
})
45+
authOne := types.AuthConfig{
46+
Auth: "super_secret_token",
47+
Email: "foo@example.com",
48+
ServerAddress: "https://example.com",
49+
}
50+
authTwo := types.AuthConfig{
51+
Auth: "also_super_secret_token",
52+
Email: "bar@example.com",
53+
ServerAddress: "https://other.example.com",
54+
}
55+
56+
expectedSaveCount = 1
57+
t.Run("store new credentials", func(t *testing.T) {
58+
assert.NilError(t, s.Store(authOne))
59+
retrievedAuth, err := s.Get(authOne.ServerAddress)
60+
assert.NilError(t, err)
61+
assert.Check(t, is.Equal(retrievedAuth, authOne))
62+
assert.Check(t, is.Equal(saveCount, expectedSaveCount))
63+
})
64+
t.Run("store same credentials is a no-op", func(t *testing.T) {
65+
assert.NilError(t, s.Store(authOne))
66+
retrievedAuth, err := s.Get(authOne.ServerAddress)
67+
assert.NilError(t, err)
68+
assert.Check(t, is.Equal(retrievedAuth, authOne))
69+
assert.Check(t, is.Equal(saveCount, expectedSaveCount), "should not have saved if nothing changed")
70+
})
71+
t.Run("store other credentials", func(t *testing.T) {
72+
expectedSaveCount++
73+
assert.NilError(t, s.Store(authTwo))
74+
retrievedAuth, err := s.Get(authTwo.ServerAddress)
75+
assert.NilError(t, err)
76+
assert.Check(t, is.Equal(retrievedAuth, authTwo))
77+
assert.Check(t, is.Equal(saveCount, expectedSaveCount))
78+
})
79+
t.Run("erase credentials", func(t *testing.T) {
80+
expectedSaveCount++
81+
assert.NilError(t, s.Erase(authOne.ServerAddress))
82+
retrievedAuth, err := s.Get(authOne.ServerAddress)
83+
assert.NilError(t, err)
84+
assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{}))
85+
assert.Check(t, is.Equal(saveCount, expectedSaveCount))
86+
})
87+
t.Run("erase non-existing credentials is a no-op", func(t *testing.T) {
88+
assert.NilError(t, s.Erase(authOne.ServerAddress))
89+
retrievedAuth, err := s.Get(authOne.ServerAddress)
90+
assert.NilError(t, err)
91+
assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{}))
92+
assert.Check(t, is.Equal(saveCount, expectedSaveCount), "should not have saved if nothing changed")
93+
})
94+
t.Run("erase other credentials", func(t *testing.T) {
95+
expectedSaveCount++
96+
assert.NilError(t, s.Erase(authTwo.ServerAddress))
97+
retrievedAuth, err := s.Get(authTwo.ServerAddress)
98+
assert.NilError(t, err)
99+
assert.Check(t, is.Equal(retrievedAuth, types.AuthConfig{}))
100+
assert.Check(t, is.Equal(saveCount, expectedSaveCount))
101+
})
29102
}
30103

31104
func TestFileStoreAddCredentials(t *testing.T) {
32-
f := newStore(make(map[string]types.AuthConfig))
105+
f := &fakeStore{configs: map[string]types.AuthConfig{}}
33106

34107
s := NewFileStore(f)
35108
auth := types.AuthConfig{
@@ -47,13 +120,13 @@ func TestFileStoreAddCredentials(t *testing.T) {
47120
}
48121

49122
func TestFileStoreGet(t *testing.T) {
50-
f := newStore(map[string]types.AuthConfig{
123+
f := &fakeStore{configs: map[string]types.AuthConfig{
51124
"https://example.com": {
52125
Auth: "super_secret_token",
53126
Email: "foo@example.com",
54127
ServerAddress: "https://example.com",
55128
},
56-
})
129+
}}
57130

58131
s := NewFileStore(f)
59132
a, err := s.Get("https://example.com")
@@ -71,7 +144,7 @@ func TestFileStoreGet(t *testing.T) {
71144
func TestFileStoreGetAll(t *testing.T) {
72145
s1 := "https://example.com"
73146
s2 := "https://example2.example.com"
74-
f := newStore(map[string]types.AuthConfig{
147+
f := &fakeStore{configs: map[string]types.AuthConfig{
75148
s1: {
76149
Auth: "super_secret_token",
77150
Email: "foo@example.com",
@@ -82,7 +155,7 @@ func TestFileStoreGetAll(t *testing.T) {
82155
Email: "foo@example2.com",
83156
ServerAddress: "https://example2.example.com",
84157
},
85-
})
158+
}}
86159

87160
s := NewFileStore(f)
88161
as, err := s.GetAll()
@@ -107,13 +180,13 @@ func TestFileStoreGetAll(t *testing.T) {
107180
}
108181

109182
func TestFileStoreErase(t *testing.T) {
110-
f := newStore(map[string]types.AuthConfig{
183+
f := &fakeStore{configs: map[string]types.AuthConfig{
111184
"https://example.com": {
112185
Auth: "super_secret_token",
113186
Email: "foo@example.com",
114187
ServerAddress: "https://example.com",
115188
},
116-
})
189+
}}
117190

118191
s := NewFileStore(f)
119192
err := s.Erase("https://example.com")

‎cli/config/credentials/native_store_test.go‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func mockCommandFn(args ...string) client.Program {
9191
}
9292

9393
func TestNativeStoreAddCredentials(t *testing.T) {
94-
f := newStore(make(map[string]types.AuthConfig))
94+
f := &fakeStore{configs: map[string]types.AuthConfig{}}
9595
s := &nativeStore{
9696
programFunc: mockCommandFn,
9797
fileStore: NewFileStore(f),
@@ -116,7 +116,7 @@ func TestNativeStoreAddCredentials(t *testing.T) {
116116
}
117117

118118
func TestNativeStoreAddInvalidCredentials(t *testing.T) {
119-
f := newStore(make(map[string]types.AuthConfig))
119+
f := &fakeStore{configs: map[string]types.AuthConfig{}}
120120
s := &nativeStore{
121121
programFunc: mockCommandFn,
122122
fileStore: NewFileStore(f),
@@ -132,11 +132,11 @@ func TestNativeStoreAddInvalidCredentials(t *testing.T) {
132132
}
133133

134134
func TestNativeStoreGet(t *testing.T) {
135-
f := newStore(map[string]types.AuthConfig{
135+
f := &fakeStore{configs: map[string]types.AuthConfig{
136136
validServerAddress: {
137137
Email: "foo@example.com",
138138
},
139-
})
139+
}}
140140
s := &nativeStore{
141141
programFunc: mockCommandFn,
142142
fileStore: NewFileStore(f),
@@ -154,11 +154,11 @@ func TestNativeStoreGet(t *testing.T) {
154154
}
155155

156156
func TestNativeStoreGetIdentityToken(t *testing.T) {
157-
f := newStore(map[string]types.AuthConfig{
157+
f := &fakeStore{configs: map[string]types.AuthConfig{
158158
validServerAddress2: {
159159
Email: "foo@example2.com",
160160
},
161-
})
161+
}}
162162

163163
s := &nativeStore{
164164
programFunc: mockCommandFn,
@@ -176,11 +176,11 @@ func TestNativeStoreGetIdentityToken(t *testing.T) {
176176
}
177177

178178
func TestNativeStoreGetAll(t *testing.T) {
179-
f := newStore(map[string]types.AuthConfig{
179+
f := &fakeStore{configs: map[string]types.AuthConfig{
180180
validServerAddress: {
181181
Email: "foo@example.com",
182182
},
183-
})
183+
}}
184184

185185
s := &nativeStore{
186186
programFunc: mockCommandFn,
@@ -217,11 +217,11 @@ func TestNativeStoreGetAll(t *testing.T) {
217217
}
218218

219219
func TestNativeStoreGetMissingCredentials(t *testing.T) {
220-
f := newStore(map[string]types.AuthConfig{
220+
f := &fakeStore{configs: map[string]types.AuthConfig{
221221
validServerAddress: {
222222
Email: "foo@example.com",
223223
},
224-
})
224+
}}
225225

226226
s := &nativeStore{
227227
programFunc: mockCommandFn,
@@ -232,11 +232,11 @@ func TestNativeStoreGetMissingCredentials(t *testing.T) {
232232
}
233233

234234
func TestNativeStoreGetInvalidAddress(t *testing.T) {
235-
f := newStore(map[string]types.AuthConfig{
235+
f := &fakeStore{configs: map[string]types.AuthConfig{
236236
validServerAddress: {
237237
Email: "foo@example.com",
238238
},
239-
})
239+
}}
240240

241241
s := &nativeStore{
242242
programFunc: mockCommandFn,
@@ -247,11 +247,11 @@ func TestNativeStoreGetInvalidAddress(t *testing.T) {
247247
}
248248

249249
func TestNativeStoreErase(t *testing.T) {
250-
f := newStore(map[string]types.AuthConfig{
250+
f := &fakeStore{configs: map[string]types.AuthConfig{
251251
validServerAddress: {
252252
Email: "foo@example.com",
253253
},
254-
})
254+
}}
255255

256256
s := &nativeStore{
257257
programFunc: mockCommandFn,
@@ -263,11 +263,11 @@ func TestNativeStoreErase(t *testing.T) {
263263
}
264264

265265
func TestNativeStoreEraseInvalidAddress(t *testing.T) {
266-
f := newStore(map[string]types.AuthConfig{
266+
f := &fakeStore{configs: map[string]types.AuthConfig{
267267
validServerAddress: {
268268
Email: "foo@example.com",
269269
},
270-
})
270+
}}
271271

272272
s := &nativeStore{
273273
programFunc: mockCommandFn,

0 commit comments

Comments
 (0)