Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit ca9ec99

Browse files
burmudarvovakulikov
authored andcommitted
github test: make the sorting more stable in test (#58038)
* make the sorting more stable in this test This is diffcult to catch in the act but here is one data point where the sorting order isn't stable https://buildkite.com/sourcegraph/sourcegraph/builds/249376#018b89e8-b855-4663-939a-dc8c39847dfb * bazel configure * std lib slices only in 1.21 * gofmt * use less instead of compare * refactor to just use stablefunc * cmp module is only 1.21 * fix ordering
1 parent 49ea2c7 commit ca9ec99

File tree

1 file changed

+37
-32
lines changed

1 file changed

+37
-32
lines changed

internal/authz/providers/github/github_test.go

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ func mockClientFunc(mockClient client) func() (client, error) {
4040
}
4141
}
4242

43+
func stableSortRepoID(v []extsvc.RepoID) {
44+
slices.SortStableFunc(v, func(a, b extsvc.RepoID) bool { return strings.Compare(string(a), string(b)) < 1 })
45+
}
46+
4347
// newMockClientWithTokenMock is used to keep the behaviour of WithToken function mocking
4448
// which is lost during moving the client interface to mockgen usage
4549
func newMockClientWithTokenMock() *MockClient {
@@ -156,8 +160,8 @@ func TestProvider_FetchUserPerms(t *testing.T) {
156160
switch page {
157161
case 1:
158162
return []*github.Repository{
159-
{ID: "MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE="}, // existing repo
160163
{ID: "MDEwOlJlcG9zaXRvcnkyNDQ1MTc1234="},
164+
{ID: "MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE="}, // existing repo
161165
}, true, 1, nil
162166
case 2:
163167
return []*github.Repository{
@@ -202,11 +206,12 @@ func TestProvider_FetchUserPerms(t *testing.T) {
202206

203207
wantRepoIDs := []extsvc.RepoID{
204208
"MDEwOlJlcG9zaXRvcnkyNDI2NTEwMDA=",
205-
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=",
206209
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1MzY=",
210+
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=",
207211
}
208-
slices.Sort(wantRepoIDs)
209-
slices.Sort(repoIDs.Exacts)
212+
213+
stableSortRepoID(wantRepoIDs)
214+
stableSortRepoID(repoIDs.Exacts)
210215
if diff := cmp.Diff(wantRepoIDs, repoIDs.Exacts); diff != "" {
211216
t.Fatalf("RepoIDs mismatch (-want +got):\n%s", diff)
212217
}
@@ -251,12 +256,12 @@ func TestProvider_FetchUserPerms(t *testing.T) {
251256
}
252257

253258
wantRepoIDs := []extsvc.RepoID{
254-
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=",
255-
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1MzY=",
256259
"MDEwOlJlcG9zaXRvcnkyNDI2NTEwMDA=",
260+
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1MzY=",
261+
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=",
257262
}
258-
slices.Sort(wantRepoIDs)
259-
slices.Sort(repoIDs.Exacts)
263+
stableSortRepoID(wantRepoIDs)
264+
stableSortRepoID(repoIDs.Exacts)
260265
if diff := cmp.Diff(wantRepoIDs, repoIDs.Exacts); diff != "" {
261266
t.Fatalf("RepoIDs mismatch (-want +got):\n%s", diff)
262267
}
@@ -284,15 +289,15 @@ func TestProvider_FetchUserPerms(t *testing.T) {
284289
}
285290

286291
wantRepoIDs := []extsvc.RepoID{
287-
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=",
288-
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1MzY=",
292+
"MDEwOlJlcG9zaXRvcnkyNDI2NTE5678=",
289293
"MDEwOlJlcG9zaXRvcnkyNDI2NTEwMDA=",
290294
"MDEwOlJlcG9zaXRvcnkyNDI2NTadmin=",
291295
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1234=",
292-
"MDEwOlJlcG9zaXRvcnkyNDI2NTE5678=",
296+
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1MzY=",
297+
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=",
293298
}
294-
slices.Sort(wantRepoIDs)
295-
slices.Sort(repoIDs.Exacts)
299+
stableSortRepoID(wantRepoIDs)
300+
stableSortRepoID(repoIDs.Exacts)
296301
if diff := cmp.Diff(wantRepoIDs, repoIDs.Exacts); diff != "" {
297302
t.Fatalf("RepoIDs mismatch (-want +got):\n%s", diff)
298303
}
@@ -355,17 +360,17 @@ func TestProvider_FetchUserPerms(t *testing.T) {
355360
}
356361

357362
wantRepoIDs := []extsvc.RepoID{
358-
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=",
359-
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1MzY=",
363+
"MDEwOlJlcG9zaXRvcnkyNDI2NTE5678=",
360364
"MDEwOlJlcG9zaXRvcnkyNDI2NTEwMDA=",
361365
"MDEwOlJlcG9zaXRvcnkyNDI2NTadmin=",
366+
"MDEwOlJlcG9zaXRvcnkyNDI2nsteam2=",
362367
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1234=",
363-
"MDEwOlJlcG9zaXRvcnkyNDI2NTE5678=",
368+
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1MzY=",
364369
"MDEwOlJlcG9zaXRvcnkyNDQ1nsteam1=",
365-
"MDEwOlJlcG9zaXRvcnkyNDI2nsteam2=",
370+
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=",
366371
}
367-
slices.Sort(wantRepoIDs)
368-
slices.Sort(repoIDs.Exacts)
372+
stableSortRepoID(wantRepoIDs)
373+
stableSortRepoID(repoIDs.Exacts)
369374
if diff := cmp.Diff(wantRepoIDs, repoIDs.Exacts); diff != "" {
370375
t.Fatalf("RepoIDs mismatch (-want +got):\n%s", diff)
371376
}
@@ -411,15 +416,15 @@ func TestProvider_FetchUserPerms(t *testing.T) {
411416
}
412417

413418
wantRepoIDs := []extsvc.RepoID{
414-
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=", // from ListAffiliatedRepos
415-
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1MzY=", // from ListAffiliatedRepos
419+
"MDEwOlJlcG9zaXRvcnkyNDI2NTE5678=", // from ListOrgRepositories
416420
"MDEwOlJlcG9zaXRvcnkyNDI2NTEwMDA=", // from ListAffiliatedRepos
417421
"MDEwOlJlcG9zaXRvcnkyNDI2NTadmin=", // from ListOrgRepositories
418422
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1234=", // from ListOrgRepositories
419-
"MDEwOlJlcG9zaXRvcnkyNDI2NTE5678=", // from ListOrgRepositories
423+
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1MzY=", // from ListAffiliatedRepos
424+
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=", // from ListAffiliatedRepos
420425
}
421-
slices.Sort(wantRepoIDs)
422-
slices.Sort(repoIDs.Exacts)
426+
stableSortRepoID(wantRepoIDs)
427+
stableSortRepoID(repoIDs.Exacts)
423428
if diff := cmp.Diff(wantRepoIDs, repoIDs.Exacts); diff != "" {
424429
t.Fatalf("RepoIDs mismatch (-want +got):\n%s", diff)
425430
}
@@ -464,16 +469,16 @@ func TestProvider_FetchUserPerms(t *testing.T) {
464469
p.groupsCache = memCache
465470

466471
wantRepoIDs := []extsvc.RepoID{
467-
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=",
468-
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1MzY=",
472+
"MDEwOlJlcG9zaXRvcnkyNDI2NTE5678=",
469473
"MDEwOlJlcG9zaXRvcnkyNDI2NTEwMDA=",
470474
"MDEwOlJlcG9zaXRvcnkyNDI2NTadmin=",
471-
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1234=",
472-
"MDEwOlJlcG9zaXRvcnkyNDI2NTE5678=",
473475
"MDEwOlJlcG9zaXRvcnkyNDI2nsteam1=",
476+
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1234=",
477+
"MDEwOlJlcG9zaXRvcnkyNDQ1MTc1MzY=",
478+
"MDEwOlJlcG9zaXRvcnkyNTI0MjU2NzE=",
474479
}
475-
slices.Sort(wantRepoIDs)
476480

481+
stableSortRepoID(wantRepoIDs)
477482
// first call
478483
t.Run("first call", func(t *testing.T) {
479484
repoIDs, err := p.FetchUserPerms(context.Background(),
@@ -487,7 +492,7 @@ func TestProvider_FetchUserPerms(t *testing.T) {
487492
t.Fatalf("expected repos to be listed: callsToListOrgRepos=%d, callsToListTeamRepos=%d",
488493
callsToListOrgRepos, callsToListTeamRepos)
489494
}
490-
slices.Sort(repoIDs.Exacts)
495+
stableSortRepoID(repoIDs.Exacts)
491496
if diff := cmp.Diff(wantRepoIDs, repoIDs.Exacts); diff != "" {
492497
t.Fatalf("RepoIDs mismatch (-want +got):\n%s", diff)
493498
}
@@ -508,7 +513,7 @@ func TestProvider_FetchUserPerms(t *testing.T) {
508513
t.Fatalf("expected repos not to be listed: callsToListOrgRepos=%d, callsToListTeamRepos=%d",
509514
callsToListOrgRepos, callsToListTeamRepos)
510515
}
511-
slices.Sort(repoIDs.Exacts)
516+
stableSortRepoID(repoIDs.Exacts)
512517
if diff := cmp.Diff(wantRepoIDs, repoIDs.Exacts); diff != "" {
513518
t.Fatalf("RepoIDs mismatch (-want +got):\n%s", diff)
514519
}
@@ -529,7 +534,7 @@ func TestProvider_FetchUserPerms(t *testing.T) {
529534
t.Fatalf("expected repos to be listed: callsToListOrgRepos=%d, callsToListTeamRepos=%d",
530535
callsToListOrgRepos, callsToListTeamRepos)
531536
}
532-
slices.Sort(repoIDs.Exacts)
537+
stableSortRepoID(repoIDs.Exacts)
533538
if diff := cmp.Diff(wantRepoIDs, repoIDs.Exacts); diff != "" {
534539
t.Fatalf("RepoIDs mismatch (-want +got):\n%s", diff)
535540
}

0 commit comments

Comments
 (0)