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

Commit 0e958d1

Browse files
authored
misc improvements to graphqlbackend (#63943)
- skip graphqlbackend tests that use the DB when using `go test -short` - fix race condition in (settingsResolver).Author - fix OrgMembers.GetByOrgIDAndUserID mocks: The actual function returns a non-nil error (`database.ErrOrgMemberNotFound` type) when the user is not an org member. Our mocks should do the same for correctness. ## Test plan CI
1 parent 9491958 commit 0e958d1

File tree

13 files changed

+39
-19
lines changed

13 files changed

+39
-19
lines changed

cmd/frontend/graphqlbackend/default_settings.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func (r *defaultSettingsResolver) LatestSettings(_ context.Context) (*settingsRe
3535
Subject: api.SettingsSubject{Default: true},
3636
Contents: `{"experimentalFeatures": {}}`,
3737
}
38-
return &settingsResolver{r.db, &settingsSubjectResolver{defaultSettings: r}, settings, nil}, nil
38+
return &settingsResolver{db: r.db, subject: &settingsSubjectResolver{defaultSettings: r}, settings: settings}, nil
3939
}
4040

4141
func (r *defaultSettingsResolver) SettingsURL() *string { return nil }

cmd/frontend/graphqlbackend/org.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func (o *OrgResolver) LatestSettings(ctx context.Context) (*settingsResolver, er
253253
return nil, nil
254254
}
255255

256-
return &settingsResolver{o.db, &settingsSubjectResolver{org: o}, settings, nil}, nil
256+
return &settingsResolver{db: o.db, subject: &settingsSubjectResolver{org: o}, settings: settings}, nil
257257
}
258258

259259
func (o *OrgResolver) SettingsCascade() *settingsCascade {

cmd/frontend/graphqlbackend/org_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func TestOrganization(t *testing.T) {
3131
users.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{ID: 1}, nil)
3232

3333
orgMembers := dbmocks.NewMockOrgMemberStore()
34-
orgMembers.GetByOrgIDAndUserIDFunc.SetDefaultReturn(nil, nil)
34+
orgMembers.GetByOrgIDAndUserIDFunc.SetDefaultReturn(nil, &database.ErrOrgMemberNotFound{})
3535

3636
orgs := dbmocks.NewMockOrgStore()
3737
mockedOrg := types.Org{ID: 1, Name: "acme"}

cmd/frontend/graphqlbackend/preview_repository_comparison_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ import (
2121
)
2222

2323
func TestPreviewRepositoryComparisonResolver(t *testing.T) {
24+
if testing.Short() {
25+
t.Skip()
26+
}
27+
2428
logger := logtest.Scoped(t)
2529
ctx := context.Background()
2630
db := database.NewDB(logger, nil)

cmd/frontend/graphqlbackend/repository_comparison_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ func TestRepositoryComparisonNoMergeBase(t *testing.T) {
6161
}
6262

6363
func TestRepositoryComparisonRootCommit(t *testing.T) {
64+
if testing.Short() {
65+
t.Skip()
66+
}
67+
6468
logger := logtest.Scoped(t)
6569
ctx := context.Background()
6670
db := database.NewDB(logger, nil)
@@ -94,6 +98,10 @@ func TestRepositoryComparisonRootCommit(t *testing.T) {
9498
}
9599

96100
func TestRepositoryComparison(t *testing.T) {
101+
if testing.Short() {
102+
t.Skip()
103+
}
104+
97105
logger := logtest.Scoped(t)
98106
ctx := context.Background()
99107
db := database.NewDB(logger, nil)

cmd/frontend/graphqlbackend/repository_text_search_index_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ import (
2020
)
2121

2222
func TestRetrievingAndDeduplicatingIndexedRefs(t *testing.T) {
23+
if testing.Short() {
24+
t.Skip()
25+
}
26+
2327
logger := logtest.Scoped(t)
2428
db := database.NewDB(logger, nil)
2529
defaultBranchRef := "refs/heads/main"

cmd/frontend/graphqlbackend/settings.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"os"
66
"strconv"
7+
"sync"
78

89
"github.com/sourcegraph/sourcegraph/internal/api"
910
"github.com/sourcegraph/sourcegraph/internal/database"
@@ -17,7 +18,10 @@ type settingsResolver struct {
1718
db database.DB
1819
subject *settingsSubjectResolver
1920
settings *api.Settings
20-
user *types.User
21+
22+
authorUserOnce sync.Once
23+
authorUser *types.User
24+
authorUserErr error
2125
}
2226

2327
func (o *settingsResolver) ID() int32 {
@@ -45,14 +49,14 @@ func (o *settingsResolver) Author(ctx context.Context) (*UserResolver, error) {
4549
if o.settings.AuthorUserID == nil {
4650
return nil, nil
4751
}
48-
if o.user == nil {
49-
var err error
50-
o.user, err = o.db.Users().GetByID(ctx, *o.settings.AuthorUserID)
51-
if err != nil {
52-
return nil, err
53-
}
52+
53+
o.authorUserOnce.Do(func() {
54+
o.authorUser, o.authorUserErr = o.db.Users().GetByID(ctx, *o.settings.AuthorUserID)
55+
})
56+
if o.authorUserErr != nil {
57+
return nil, o.authorUserErr
5458
}
55-
return NewUserResolver(ctx, o.db, o.user), nil
59+
return NewUserResolver(ctx, o.db, o.authorUser), nil
5660
}
5761

5862
var globalSettingsAllowEdits, _ = strconv.ParseBool(env.Get("GLOBAL_SETTINGS_ALLOW_EDITS", "false", "When GLOBAL_SETTINGS_FILE is in use, allow edits in the application to be made which will be overwritten on next process restart"))

cmd/frontend/graphqlbackend/site.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func (r *siteResolver) LatestSettings(ctx context.Context) (*settingsResolver, e
156156
if settings == nil {
157157
return nil, nil
158158
}
159-
return &settingsResolver{r.db, &settingsSubjectResolver{site: r}, settings, nil}, nil
159+
return &settingsResolver{db: r.db, subject: &settingsSubjectResolver{site: r}, settings: settings}, nil
160160
}
161161

162162
func (r *siteResolver) SettingsCascade() *settingsCascade {

cmd/frontend/graphqlbackend/site_admin_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ func TestDeleteOrganization_OnPremise(t *testing.T) {
307307
users.GetByCurrentAuthUserFunc.SetDefaultReturn(&types.User{ID: 1}, nil)
308308

309309
orgMembers := dbmocks.NewMockOrgMemberStore()
310-
orgMembers.GetByOrgIDAndUserIDFunc.SetDefaultReturn(nil, nil)
310+
orgMembers.GetByOrgIDAndUserIDFunc.SetDefaultReturn(nil, &database.ErrOrgMemberNotFound{})
311311

312312
orgs := dbmocks.NewMockOrgStore()
313313

cmd/frontend/graphqlbackend/user.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ func (r *UserResolver) LatestSettings(ctx context.Context) (*settingsResolver, e
393393
if settings == nil {
394394
return nil, nil
395395
}
396-
return &settingsResolver{r.db, &settingsSubjectResolver{user: r}, settings, nil}, nil
396+
return &settingsResolver{db: r.db, subject: &settingsSubjectResolver{user: r}, settings: settings}, nil
397397
}
398398

399399
func (r *UserResolver) SettingsCascade() *settingsCascade {

0 commit comments

Comments
 (0)