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

Commit 5479a45

Browse files
keegancsmithkeegancsmith
authored
dotcom: MockSourcegraphDotComMode requires a T for cleanup (#61172)
I had a suspicion another test was failing due to racing with reading dotcom.SourcegraphDotComMode and another test didn't unset it. It turned out this wasn't the case, but I ended improving the API to avoid this issue. Most call sites should be easier to read. Test Plan: go test
1 parent e9730ba commit 5479a45

File tree

39 files changed

+100
-207
lines changed

39 files changed

+100
-207
lines changed

‎cmd/frontend/backend/user_emails_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ func TestCheckEmailAbuse(t *testing.T) {
3939
conf.Mock(cfg)
4040
}()
4141

42-
dotcom.MockSourcegraphDotComMode(true)
43-
defer dotcom.MockSourcegraphDotComMode(false)
42+
dotcom.MockSourcegraphDotComMode(t, true)
4443

4544
now := time.Now()
4645

‎cmd/frontend/graphqlbackend/access_tokens_test.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -360,9 +360,7 @@ func TestMutation_CreateAccessToken(t *testing.T) {
360360
db := dbmocks.NewMockDB()
361361
db.UsersFunc.SetDefaultReturn(users)
362362

363-
orig := dotcom.SourcegraphDotComMode()
364-
dotcom.MockSourcegraphDotComMode(true)
365-
defer dotcom.MockSourcegraphDotComMode(orig)
363+
dotcom.MockSourcegraphDotComMode(t, true)
366364

367365
ctx := actor.WithActor(context.Background(), &actor.Actor{UID: 1})
368366
_, err := newSchemaResolver(db, gitserver.NewTestClient(t)).CreateAccessToken(ctx,
@@ -383,9 +381,7 @@ func TestMutation_CreateAccessToken(t *testing.T) {
383381
conf.Get().AuthAccessTokens = &schema.AuthAccessTokens{Allow: string(conf.AccessTokensAdmin)}
384382
defer func() { conf.Get().AuthAccessTokens = nil }()
385383

386-
orig := dotcom.SourcegraphDotComMode()
387-
dotcom.MockSourcegraphDotComMode(true)
388-
defer dotcom.MockSourcegraphDotComMode(orig)
384+
dotcom.MockSourcegraphDotComMode(t, true)
389385

390386
ctx := actor.WithActor(context.Background(), &actor.Actor{UID: 1})
391387
_, err := newSchemaResolver(db, gitserver.NewTestClient(t)).CreateAccessToken(ctx,

‎cmd/frontend/graphqlbackend/event_logs_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@ func TestUser_EventLogs(t *testing.T) {
1919
users := dbmocks.NewMockUserStore()
2020
db.UsersFunc.SetDefaultReturn(users)
2121

22-
orig := dotcom.SourcegraphDotComMode()
23-
dotcom.MockSourcegraphDotComMode(true)
24-
defer dotcom.MockSourcegraphDotComMode(orig) // reset
22+
dotcom.MockSourcegraphDotComMode(t, true)
2523

2624
tests := []struct {
2725
name string

‎cmd/frontend/graphqlbackend/org_test.go

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,7 @@ func TestOrganization(t *testing.T) {
6767
})
6868

6969
t.Run("users not invited or not a member cannot access on Sourcegraph.com", func(t *testing.T) {
70-
orig := dotcom.SourcegraphDotComMode()
71-
dotcom.MockSourcegraphDotComMode(true)
72-
defer dotcom.MockSourcegraphDotComMode(orig)
70+
dotcom.MockSourcegraphDotComMode(t, true)
7371

7472
RunTests(t, []*Test{
7573
{
@@ -97,9 +95,7 @@ func TestOrganization(t *testing.T) {
9795
})
9896

9997
t.Run("org members can access on Sourcegraph.com", func(t *testing.T) {
100-
orig := dotcom.SourcegraphDotComMode()
101-
dotcom.MockSourcegraphDotComMode(true)
102-
defer dotcom.MockSourcegraphDotComMode(orig)
98+
dotcom.MockSourcegraphDotComMode(t, true)
10399

104100
ctx := actor.WithActor(context.Background(), &actor.Actor{UID: 1})
105101

@@ -136,9 +132,7 @@ func TestOrganization(t *testing.T) {
136132
})
137133

138134
t.Run("invited users can access on Sourcegraph.com", func(t *testing.T) {
139-
orig := dotcom.SourcegraphDotComMode()
140-
dotcom.MockSourcegraphDotComMode(true)
141-
defer dotcom.MockSourcegraphDotComMode(orig)
135+
dotcom.MockSourcegraphDotComMode(t, true)
142136

143137
ctx := actor.WithActor(context.Background(), &actor.Actor{UID: 1})
144138

@@ -180,9 +174,7 @@ func TestOrganization(t *testing.T) {
180174
})
181175

182176
t.Run("invited users can access org by ID on Sourcegraph.com", func(t *testing.T) {
183-
orig := dotcom.SourcegraphDotComMode()
184-
dotcom.MockSourcegraphDotComMode(true)
185-
defer dotcom.MockSourcegraphDotComMode(orig)
177+
dotcom.MockSourcegraphDotComMode(t, true)
186178

187179
ctx := actor.WithActor(context.Background(), &actor.Actor{UID: 1})
188180

@@ -274,8 +266,7 @@ func TestCreateOrganization(t *testing.T) {
274266
})
275267

276268
t.Run("Creates organization and sets statistics", func(t *testing.T) {
277-
dotcom.MockSourcegraphDotComMode(true)
278-
defer dotcom.MockSourcegraphDotComMode(false)
269+
dotcom.MockSourcegraphDotComMode(t, true)
279270

280271
id, err := uuid.NewV4()
281272
if err != nil {
@@ -312,8 +303,7 @@ func TestCreateOrganization(t *testing.T) {
312303
})
313304

314305
t.Run("Fails for unauthenticated user", func(t *testing.T) {
315-
dotcom.MockSourcegraphDotComMode(true)
316-
defer dotcom.MockSourcegraphDotComMode(false)
306+
dotcom.MockSourcegraphDotComMode(t, true)
317307

318308
RunTest(t, &Test{
319309
Schema: mustParseGraphQLSchema(t, db),
@@ -338,8 +328,7 @@ func TestCreateOrganization(t *testing.T) {
338328
})
339329

340330
t.Run("Fails for suspicious organization name", func(t *testing.T) {
341-
dotcom.MockSourcegraphDotComMode(true)
342-
defer dotcom.MockSourcegraphDotComMode(false)
331+
dotcom.MockSourcegraphDotComMode(t, true)
343332

344333
RunTest(t, &Test{
345334
Schema: mustParseGraphQLSchema(t, db),
@@ -421,8 +410,7 @@ func TestAddOrganizationMember(t *testing.T) {
421410
})
422411

423412
t.Run("Does not work for site admin on Cloud", func(t *testing.T) {
424-
dotcom.MockSourcegraphDotComMode(true)
425-
defer dotcom.MockSourcegraphDotComMode(false)
413+
dotcom.MockSourcegraphDotComMode(t, true)
426414

427415
RunTest(t, &Test{
428416
Schema: mustParseGraphQLSchema(t, db),
@@ -447,7 +435,7 @@ func TestAddOrganizationMember(t *testing.T) {
447435
})
448436

449437
t.Run("Works on Cloud if site admin is org member", func(t *testing.T) {
450-
dotcom.MockSourcegraphDotComMode(true)
438+
dotcom.MockSourcegraphDotComMode(t, true)
451439
orgMembers.GetByOrgIDAndUserIDFunc.SetDefaultHook(func(ctx context.Context, orgID int32, userID int32) (*types.OrgMembership, error) {
452440
if userID == 1 {
453441
return &types.OrgMembership{OrgID: orgID, UserID: 1}, nil
@@ -459,7 +447,6 @@ func TestAddOrganizationMember(t *testing.T) {
459447
})
460448

461449
defer func() {
462-
dotcom.MockSourcegraphDotComMode(false)
463450
orgMembers.GetByOrgIDAndUserIDFunc.SetDefaultReturn(nil, &database.ErrOrgMemberNotFound{})
464451
}()
465452

‎cmd/frontend/graphqlbackend/roles_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,7 @@ func TestUserRoleListing(t *testing.T) {
164164
require.NoError(t, err)
165165

166166
t.Run("on sourcegraph.com", func(t *testing.T) {
167-
orig := dotcom.SourcegraphDotComMode()
168-
dotcom.MockSourcegraphDotComMode(true)
169-
defer dotcom.MockSourcegraphDotComMode(orig)
167+
dotcom.MockSourcegraphDotComMode(t, true)
170168

171169
t.Run("non-admin", func(t *testing.T) {
172170
userAPIID := string(MarshalUserID(userID))

‎cmd/frontend/graphqlbackend/settings_mutation_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,7 @@ func TestSettingsMutation(t *testing.T) {
119119
users := dbmocks.NewMockUserStore()
120120
db.UsersFunc.SetDefaultReturn(users)
121121

122-
orig := dotcom.SourcegraphDotComMode()
123-
dotcom.MockSourcegraphDotComMode(true)
124-
defer dotcom.MockSourcegraphDotComMode(orig) // reset
122+
dotcom.MockSourcegraphDotComMode(t, true)
125123

126124
tests := []struct {
127125
name string

‎cmd/frontend/graphqlbackend/user_test.go

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,7 @@ func TestUser(t *testing.T) {
5656
db.UsersFunc.SetDefaultReturn(users)
5757

5858
t.Run("allowed on Sourcegraph.com", func(t *testing.T) {
59-
orig := dotcom.SourcegraphDotComMode()
60-
dotcom.MockSourcegraphDotComMode(true)
61-
defer dotcom.MockSourcegraphDotComMode(orig)
62-
59+
dotcom.MockSourcegraphDotComMode(t, true)
6360
checkUserByUsername(t)
6461
})
6562

@@ -101,9 +98,7 @@ func TestUser(t *testing.T) {
10198
})
10299
}
103100

104-
orig := dotcom.SourcegraphDotComMode()
105-
dotcom.MockSourcegraphDotComMode(true)
106-
defer dotcom.MockSourcegraphDotComMode(orig)
101+
dotcom.MockSourcegraphDotComMode(t, true)
107102

108103
t.Run("for anonymous viewer", func(t *testing.T) {
109104
users.GetByCurrentAuthUserFunc.SetDefaultReturn(nil, database.ErrNoCurrentUser)
@@ -182,9 +177,7 @@ func TestUser_LatestSettings(t *testing.T) {
182177
db.UsersFunc.SetDefaultReturn(users)
183178
db.SettingsFunc.SetDefaultReturn(dbmocks.NewMockSettingsStore())
184179

185-
orig := dotcom.SourcegraphDotComMode()
186-
dotcom.MockSourcegraphDotComMode(true)
187-
defer dotcom.MockSourcegraphDotComMode(orig)
180+
dotcom.MockSourcegraphDotComMode(t, true)
188181

189182
tests := []struct {
190183
name string
@@ -247,11 +240,7 @@ func TestUser_ViewerCanAdminister(t *testing.T) {
247240
users := dbmocks.NewMockUserStore()
248241
db.UsersFunc.SetDefaultReturn(users)
249242

250-
orig := dotcom.SourcegraphDotComMode()
251-
dotcom.MockSourcegraphDotComMode(true)
252-
t.Cleanup(func() {
253-
dotcom.MockSourcegraphDotComMode(orig)
254-
})
243+
dotcom.MockSourcegraphDotComMode(t, true)
255244

256245
tests := []struct {
257246
name string
@@ -366,9 +355,8 @@ func TestUpdateUser(t *testing.T) {
366355
})
367356

368357
t.Run("disallow suspicious names", func(t *testing.T) {
369-
orig := dotcom.SourcegraphDotComMode()
370-
dotcom.MockSourcegraphDotComMode(true)
371-
defer dotcom.MockSourcegraphDotComMode(orig)
358+
359+
dotcom.MockSourcegraphDotComMode(t, true)
372360

373361
users := dbmocks.NewMockUserStore()
374362
users.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{ID: 1}, nil)
@@ -466,9 +454,7 @@ func TestUpdateUser(t *testing.T) {
466454
users := dbmocks.NewMockUserStore()
467455
db.UsersFunc.SetDefaultReturn(users)
468456

469-
orig := dotcom.SourcegraphDotComMode()
470-
dotcom.MockSourcegraphDotComMode(true)
471-
defer dotcom.MockSourcegraphDotComMode(orig)
457+
dotcom.MockSourcegraphDotComMode(t, true)
472458

473459
tests := []struct {
474460
name string
@@ -747,9 +733,8 @@ func TestUser_Organizations(t *testing.T) {
747733
}
748734

749735
t.Run("on Sourcegraph.com", func(t *testing.T) {
750-
orig := dotcom.SourcegraphDotComMode()
751-
dotcom.MockSourcegraphDotComMode(true)
752-
t.Cleanup(func() { dotcom.MockSourcegraphDotComMode(orig) })
736+
737+
dotcom.MockSourcegraphDotComMode(t, true)
753738

754739
t.Run("same user", func(t *testing.T) {
755740
expectOrgSuccess(t, 1)

‎cmd/frontend/graphqlbackend/users_randomize_password_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,7 @@ func TestRandomizeUserPassword(t *testing.T) {
5757

5858
t.Run("DotCom mode", func(t *testing.T) {
5959
// Test dotcom mode
60-
orig := dotcom.SourcegraphDotComMode()
61-
dotcom.MockSourcegraphDotComMode(true)
62-
defer dotcom.MockSourcegraphDotComMode(orig)
60+
dotcom.MockSourcegraphDotComMode(t, true)
6361

6462
t.Run("Errors on DotCom when sending emails is not enabled", func(t *testing.T) {
6563
conf.Mock(smtpDisabledConf)

‎cmd/frontend/graphqlbackend/users_test.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -262,14 +262,7 @@ type usersQueryTest struct {
262262

263263
func runUsersQuery(t *testing.T, schema *graphql.Schema, want usersQueryTest) {
264264
t.Helper()
265-
266-
if want.dotcom {
267-
orig := dotcom.SourcegraphDotComMode()
268-
dotcom.MockSourcegraphDotComMode(true)
269-
t.Cleanup(func() {
270-
dotcom.MockSourcegraphDotComMode(orig)
271-
})
272-
}
265+
dotcom.MockSourcegraphDotComMode(t, want.dotcom)
273266

274267
type node struct {
275268
Username string `json:"username"`

‎cmd/frontend/internal/app/ui/handlers_test.go

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,27 +74,24 @@ func TestRedirects(t *testing.T) {
7474
}
7575

7676
t.Run("on Sourcegraph.com", func(t *testing.T) {
77-
orig := dotcom.SourcegraphDotComMode()
78-
dotcom.MockSourcegraphDotComMode(true)
79-
defer dotcom.MockSourcegraphDotComMode(orig) // reset
77+
dotcom.MockSourcegraphDotComMode(t, true)
78+
8079
t.Run("root", func(t *testing.T) {
8180
check(t, "/", http.StatusTemporaryRedirect, "/search", "Mozilla/5.0")
8281
})
8382
})
8483

8584
t.Run("on Sourcegraph.com from Cookiebot", func(t *testing.T) {
86-
orig := dotcom.SourcegraphDotComMode()
87-
dotcom.MockSourcegraphDotComMode(true)
88-
defer dotcom.MockSourcegraphDotComMode(orig) // reset
85+
dotcom.MockSourcegraphDotComMode(t, true)
86+
8987
t.Run("root", func(t *testing.T) {
9088
check(t, "/", http.StatusTemporaryRedirect, "/search", "Mozilla/5.0 Cookiebot")
9189
})
9290
})
9391

9492
t.Run("non-Sourcegraph.com", func(t *testing.T) {
95-
orig := dotcom.SourcegraphDotComMode()
96-
dotcom.MockSourcegraphDotComMode(false)
97-
defer dotcom.MockSourcegraphDotComMode(orig) // reset
93+
dotcom.MockSourcegraphDotComMode(t, false)
94+
9895
t.Run("root", func(t *testing.T) {
9996
check(t, "/", http.StatusTemporaryRedirect, "/search", "Mozilla/5.0")
10097
})

0 commit comments

Comments
 (0)