Skip to content

Commit 02d55ff

Browse files
committed
use t.Run to run test cases with better case handling (no need to count the case number now)
1 parent c78892e commit 02d55ff

File tree

1 file changed

+66
-84
lines changed

1 file changed

+66
-84
lines changed

modules/lfs/http_client_test.go

Lines changed: 66 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -185,94 +185,84 @@ func TestHTTPClientDownload(t *testing.T) {
185185

186186
cases := []struct {
187187
endpoint string
188-
expectederror string
188+
expectedError string
189189
}{
190-
// case 0
191190
{
192191
endpoint: "https://status-not-ok.io",
193-
expectederror: io.ErrUnexpectedEOF.Error(),
192+
expectedError: io.ErrUnexpectedEOF.Error(),
194193
},
195-
// case 1
196194
{
197195
endpoint: "https://invalid-json-response.io",
198-
expectederror: "invalid json",
196+
expectedError: "invalid json",
199197
},
200-
// case 2
201198
{
202199
endpoint: "https://valid-batch-request-download.io",
203-
expectederror: "",
200+
expectedError: "",
204201
},
205-
// case 3
206202
{
207203
endpoint: "https://response-no-objects.io",
208-
expectederror: "",
204+
expectedError: "",
209205
},
210-
// case 4
211206
{
212207
endpoint: "https://unknown-transfer-adapter.io",
213-
expectederror: "TransferAdapter not found: ",
208+
expectedError: "TransferAdapter not found: ",
214209
},
215-
// case 5
216210
{
217211
endpoint: "https://error-in-response-objects.io",
218-
expectederror: "Object not found",
212+
expectedError: "Object not found",
219213
},
220-
// case 6
221214
{
222215
endpoint: "https://empty-actions-map.io",
223-
expectederror: "missing action 'download'",
216+
expectedError: "missing action 'download'",
224217
},
225-
// case 7
226218
{
227219
endpoint: "https://download-actions-map.io",
228-
expectederror: "",
220+
expectedError: "",
229221
},
230-
// case 8
231222
{
232223
endpoint: "https://upload-actions-map.io",
233-
expectederror: "missing action 'download'",
224+
expectedError: "missing action 'download'",
234225
},
235-
// case 9
236226
{
237227
endpoint: "https://verify-actions-map.io",
238-
expectederror: "missing action 'download'",
228+
expectedError: "missing action 'download'",
239229
},
240-
// case 10
241230
{
242231
endpoint: "https://unknown-actions-map.io",
243-
expectederror: "missing action 'download'",
232+
expectedError: "missing action 'download'",
244233
},
245-
// case 11
246234
{
247235
endpoint: "https://legacy-batch-request-download.io",
248-
expectederror: "",
236+
expectedError: "",
249237
},
250238
}
251239

252-
defer test.MockVariableValue(&setting.LFSClient.BatchOperationConcurrency, 1)()
253-
for n, c := range cases {
254-
client := &HTTPClient{
255-
client: hc,
256-
endpoint: c.endpoint,
257-
transfers: map[string]TransferAdapter{
258-
"dummy": dummy,
259-
},
260-
}
240+
defer test.MockVariableValue(&setting.LFSClient.BatchOperationConcurrency, 3)()
241+
for _, c := range cases {
242+
t.Run(c.endpoint, func(t *testing.T) {
243+
client := &HTTPClient{
244+
client: hc,
245+
endpoint: c.endpoint,
246+
transfers: map[string]TransferAdapter{
247+
"dummy": dummy,
248+
},
249+
}
261250

262-
err := client.Download(context.Background(), []Pointer{p}, func(p Pointer, content io.ReadCloser, objectError error) error {
263-
if objectError != nil {
264-
return objectError
251+
err := client.Download(context.Background(), []Pointer{p}, func(p Pointer, content io.ReadCloser, objectError error) error {
252+
if objectError != nil {
253+
return objectError
254+
}
255+
b, err := io.ReadAll(content)
256+
assert.NoError(t, err)
257+
assert.Equal(t, []byte("dummy"), b)
258+
return nil
259+
})
260+
if c.expectedError != "" {
261+
assert.ErrorContains(t, err, c.expectedError)
262+
} else {
263+
assert.NoError(t, err)
265264
}
266-
b, err := io.ReadAll(content)
267-
assert.NoError(t, err)
268-
assert.Equal(t, []byte("dummy"), b)
269-
return nil
270265
})
271-
if len(c.expectederror) > 0 {
272-
assert.True(t, strings.Contains(err.Error(), c.expectederror), "case %d: '%s' should contain '%s'", n, err.Error(), c.expectederror)
273-
} else {
274-
assert.NoError(t, err, "case %d", n)
275-
}
276266
}
277267
}
278268

@@ -299,81 +289,73 @@ func TestHTTPClientUpload(t *testing.T) {
299289

300290
cases := []struct {
301291
endpoint string
302-
expectederror string
292+
expectedError string
303293
}{
304-
// case 0
305294
{
306295
endpoint: "https://status-not-ok.io",
307-
expectederror: io.ErrUnexpectedEOF.Error(),
296+
expectedError: io.ErrUnexpectedEOF.Error(),
308297
},
309-
// case 1
310298
{
311299
endpoint: "https://invalid-json-response.io",
312-
expectederror: "invalid json",
300+
expectedError: "invalid json",
313301
},
314-
// case 2
315302
{
316303
endpoint: "https://valid-batch-request-upload.io",
317-
expectederror: "",
304+
expectedError: "",
318305
},
319-
// case 3
320306
{
321307
endpoint: "https://response-no-objects.io",
322-
expectederror: "",
308+
expectedError: "",
323309
},
324-
// case 4
325310
{
326311
endpoint: "https://unknown-transfer-adapter.io",
327-
expectederror: "TransferAdapter not found: ",
312+
expectedError: "TransferAdapter not found: ",
328313
},
329-
// case 5
330314
{
331315
endpoint: "https://error-in-response-objects.io",
332-
expectederror: "Object not found",
316+
expectedError: "Object not found",
333317
},
334-
// case 6
335318
{
336319
endpoint: "https://empty-actions-map.io",
337-
expectederror: "",
320+
expectedError: "",
338321
},
339-
// case 7
340322
{
341323
endpoint: "https://download-actions-map.io",
342-
expectederror: "missing action 'upload'",
324+
expectedError: "missing action 'upload'",
343325
},
344-
// case 8
345326
{
346327
endpoint: "https://upload-actions-map.io",
347-
expectederror: "",
328+
expectedError: "",
348329
},
349-
// case 9
350330
{
351331
endpoint: "https://verify-actions-map.io",
352-
expectederror: "missing action 'upload'",
332+
expectedError: "missing action 'upload'",
353333
},
354-
// case 10
355334
{
356335
endpoint: "https://unknown-actions-map.io",
357-
expectederror: "missing action 'upload'",
336+
expectedError: "missing action 'upload'",
358337
},
359338
}
360339

361-
for n, c := range cases {
362-
client := &HTTPClient{
363-
client: hc,
364-
endpoint: c.endpoint,
365-
transfers: map[string]TransferAdapter{
366-
"dummy": dummy,
367-
},
368-
}
340+
defer test.MockVariableValue(&setting.LFSClient.BatchOperationConcurrency, 3)()
341+
for _, c := range cases {
342+
t.Run(c.endpoint, func(t *testing.T) {
343+
client := &HTTPClient{
344+
client: hc,
345+
endpoint: c.endpoint,
346+
transfers: map[string]TransferAdapter{
347+
"dummy": dummy,
348+
},
349+
}
369350

370-
err := client.Upload(context.Background(), []Pointer{p}, func(p Pointer, objectError error) (io.ReadCloser, error) {
371-
return io.NopCloser(new(bytes.Buffer)), objectError
351+
err := client.Upload(context.Background(), []Pointer{p}, func(p Pointer, objectError error) (io.ReadCloser, error) {
352+
return io.NopCloser(new(bytes.Buffer)), objectError
353+
})
354+
if c.expectedError != "" {
355+
assert.ErrorContains(t, err, c.expectedError)
356+
} else {
357+
assert.NoError(t, err)
358+
}
372359
})
373-
if len(c.expectederror) > 0 {
374-
assert.True(t, strings.Contains(err.Error(), c.expectederror), "case %d: '%s' should contain '%s'", n, err.Error(), c.expectederror)
375-
} else {
376-
assert.NoError(t, err, "case %d", n)
377-
}
378360
}
379361
}

0 commit comments

Comments
 (0)