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

Commit 99f7d97

Browse files
varungandhi-srcvarungandhi-src
authored
chore: Switch over to fake RepoStore in codenav tests (#64284)
Reduce the exposed surface area with a smaller interface minimalRepoStore, and make sure we're using fakes in the tests which better document the intent, instead of using a mock repo store. The fake makes sure that the methods are mutually consistent, which is easier to do when working with a smaller interface.
1 parent 1cac35b commit 99f7d97

12 files changed

+99
-68
lines changed

‎internal/codeintel/codenav/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ go_test(
9090
"//internal/codeintel/core",
9191
"//internal/codeintel/uploads/shared",
9292
"//internal/collections",
93-
"//internal/database/dbmocks",
93+
"//internal/database",
9494
"//internal/gitserver",
9595
"//internal/gitserver/gitdomain",
9696
"//internal/observation",

‎internal/codeintel/codenav/commit_cache.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88

99
"github.com/sourcegraph/sourcegraph/internal/api"
1010
"github.com/sourcegraph/sourcegraph/internal/collections"
11-
"github.com/sourcegraph/sourcegraph/internal/database"
1211
"github.com/sourcegraph/sourcegraph/internal/gitserver"
1312
"github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain"
1413
"github.com/sourcegraph/sourcegraph/lib/errors"
@@ -25,13 +24,13 @@ type RepositoryCommit struct {
2524
}
2625

2726
type commitCache struct {
28-
repoStore database.RepoStore
27+
repoStore minimalRepoStore
2928
gitserverClient gitserver.Client
3029
mutex sync.RWMutex
3130
cache map[api.RepoID]map[api.CommitID]bool
3231
}
3332

34-
func NewCommitCache(repoStore database.RepoStore, client gitserver.Client) CommitCache {
33+
func NewCommitCache(repoStore minimalRepoStore, client gitserver.Client) CommitCache {
3534
return &commitCache{
3635
repoStore: repoStore,
3736
gitserverClient: client,

‎internal/codeintel/codenav/request_state.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func (r *RequestState) SetLocalGitTreeTranslator(client gitserver.Client, repo *
9999
r.GitTreeTranslator = NewGitTreeTranslator(client, *repo)
100100
}
101101

102-
func (r *RequestState) SetLocalCommitCache(repoStore database.RepoStore, client gitserver.Client) {
102+
func (r *RequestState) SetLocalCommitCache(repoStore minimalRepoStore, client gitserver.Client) {
103103
r.commitCache = NewCommitCache(repoStore, client)
104104
}
105105

‎internal/codeintel/codenav/service.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"github.com/sourcegraph/sourcegraph/internal/codeintel/codenav/shared"
2222
"github.com/sourcegraph/sourcegraph/internal/codeintel/core"
2323
uploadsshared "github.com/sourcegraph/sourcegraph/internal/codeintel/uploads/shared"
24-
"github.com/sourcegraph/sourcegraph/internal/database"
2524
"github.com/sourcegraph/sourcegraph/internal/gitserver"
2625
"github.com/sourcegraph/sourcegraph/internal/observation"
2726
searcher "github.com/sourcegraph/sourcegraph/internal/search/client"
@@ -32,7 +31,7 @@ import (
3231
)
3332

3433
type Service struct {
35-
repoStore database.RepoStore
34+
repoStore minimalRepoStore
3635
lsifstore lsifstore.LsifStore
3736
gitserver gitserver.Client
3837
uploadSvc UploadService
@@ -41,9 +40,18 @@ type Service struct {
4140
logger log.Logger
4241
}
4342

43+
// minimalRepoStore covers the subset of database.RepoStore APIs that we need
44+
// for code navigation.
45+
//
46+
// Prefer calling GetReposSetByIDs instead of calling Get in a loop.
47+
type minimalRepoStore interface {
48+
Get(context.Context, api.RepoID) (*types.Repo, error)
49+
GetReposSetByIDs(context.Context, ...api.RepoID) (map[api.RepoID]*types.Repo, error)
50+
}
51+
4452
func newService(
4553
observationCtx *observation.Context,
46-
repoStore database.RepoStore,
54+
repoStore minimalRepoStore,
4755
lsifstore lsifstore.LsifStore,
4856
uploadSvc UploadService,
4957
gitserver gitserver.Client,

‎internal/codeintel/codenav/service_closest_uploads_test.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,25 +66,22 @@ func TestGetClosestCompletedUploadsForBlob(t *testing.T) {
6666
}
6767
for _, testCase := range testCases {
6868
// Set up mocks
69-
mockRepoStore := defaultMockRepoStore()
69+
const testRepoName = "yummy.com/cake"
70+
fakeRepoStore := FakeMinimalRepoStore{data: map[api.RepoID]*types.Repo{repoID: {ID: repoID, Name: testRepoName}}}
7071
mockLsifStore := lsifstoremocks.NewMockLsifStore()
7172
mockUploadSvc := NewMockUploadService()
7273
mockGitserverClient := gitserver.NewMockClient()
7374
mockSearchClient := client.NewMockSearchClient()
7475

7576
// Init service
76-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
77+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
7778

7879
closestUploads := slices.Clone(testCase.closestUploads)
7980
for i := range closestUploads {
8081
closestUploads[i].RepositoryID = repoID
8182
}
8283

8384
mockUploadSvc.InferClosestUploadsFunc.SetDefaultReturn(closestUploads, nil)
84-
const testRepoName = "yummy.com/cake"
85-
mockRepoStore.GetReposSetByIDsFunc.PushReturn(map[api.RepoID]*types.Repo{
86-
repoID: {ID: repoID, Name: testRepoName},
87-
}, nil)
8885

8986
mockGitserverClient.GetCommitFunc.SetDefaultHook(func(_ context.Context, repoName api.RepoName, commitID api.CommitID) (*gitdomain.Commit, error) {
9087
// C1 is deliberately missing from gitserver

‎internal/codeintel/codenav/service_diagnostics_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,18 @@ var uploadRelPath = core.NewUploadRelPathUnchecked
2525

2626
func TestDiagnostics(t *testing.T) {
2727
// Set up mocks
28-
mockRepoStore := defaultMockRepoStore()
28+
fakeRepoStore := AllPresentFakeRepoStore{}
2929
mockLsifStore := lsifstoremocks.NewMockLsifStore()
3030
mockUploadSvc := NewMockUploadService()
3131
mockGitserverClient := gitserver.NewMockClient()
3232
mockSearchClient := client.NewMockSearchClient()
3333

3434
// Init service
35-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
35+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
3636

3737
// Set up request state
3838
mockRequestState := RequestState{}
39-
mockRequestState.SetLocalCommitCache(mockRepoStore, mockGitserverClient)
39+
mockRequestState.SetLocalCommitCache(fakeRepoStore, mockGitserverClient)
4040
mockRequestState.SetLocalGitTreeTranslator(mockGitserverClient, &sgtypes.Repo{})
4141
uploads := []uploadsshared.CompletedUpload{
4242
{ID: 50, Commit: "deadbeef", Root: "sub1/"},
@@ -100,18 +100,18 @@ func TestDiagnostics(t *testing.T) {
100100

101101
func TestDiagnosticsWithSubRepoPermissions(t *testing.T) {
102102
// Set up mocks
103-
mockRepoStore := defaultMockRepoStore()
103+
fakeRepoStore := AllPresentFakeRepoStore{}
104104
mockLsifStore := lsifstoremocks.NewMockLsifStore()
105105
mockUploadSvc := NewMockUploadService()
106106
mockGitserverClient := gitserver.NewMockClient()
107107
mockSearchClient := client.NewMockSearchClient()
108108

109109
// Init service
110-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
110+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
111111

112112
// Set up request state
113113
mockRequestState := RequestState{}
114-
mockRequestState.SetLocalCommitCache(mockRepoStore, mockGitserverClient)
114+
mockRequestState.SetLocalCommitCache(fakeRepoStore, mockGitserverClient)
115115
mockRequestState.SetLocalGitTreeTranslator(mockGitserverClient, &sgtypes.Repo{})
116116
uploads := []uploadsshared.CompletedUpload{
117117
{ID: 50, Commit: "deadbeef", Root: "sub1/"},

‎internal/codeintel/codenav/service_hover_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,18 @@ import (
2121

2222
func TestHover(t *testing.T) {
2323
// Set up mocks
24-
mockRepoStore := defaultMockRepoStore()
24+
fakeRepoStore := AllPresentFakeRepoStore{}
2525
mockLsifStore := lsifstoremocks.NewMockLsifStore()
2626
mockUploadSvc := NewMockUploadService()
2727
mockGitserverClient := gitserver.NewMockClient()
2828
mockSearchClient := client.NewMockSearchClient()
2929

3030
// Init service
31-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
31+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
3232

3333
// Set up request state
3434
mockRequestState := RequestState{}
35-
mockRequestState.SetLocalCommitCache(mockRepoStore, mockGitserverClient)
35+
mockRequestState.SetLocalCommitCache(fakeRepoStore, mockGitserverClient)
3636
mockRequestState.GitTreeTranslator = noopTranslator()
3737
uploads := []uploadsshared.CompletedUpload{
3838
{ID: 50, Commit: "deadbeef", Root: "sub1/"},
@@ -77,18 +77,18 @@ func TestHover(t *testing.T) {
7777

7878
func TestHoverRemote(t *testing.T) {
7979
// Set up mocks
80-
mockRepoStore := defaultMockRepoStore()
80+
fakeRepoStore := AllPresentFakeRepoStore{}
8181
mockLsifStore := lsifstoremocks.NewMockLsifStore()
8282
mockUploadSvc := NewMockUploadService()
8383
mockGitserverClient := gitserver.NewMockClient()
8484
mockSearchClient := client.NewMockSearchClient()
8585

8686
// Init service
87-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
87+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
8888

8989
// Set up request state
9090
mockRequestState := RequestState{}
91-
mockRequestState.SetLocalCommitCache(mockRepoStore, mockGitserverClient)
91+
mockRequestState.SetLocalCommitCache(fakeRepoStore, mockGitserverClient)
9292
mockRequestState.GitTreeTranslator = noopTranslator()
9393
uploads := []uploadsshared.CompletedUpload{
9494
{ID: 50, Commit: "deadbeef"},

‎internal/codeintel/codenav/service_new_test.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,19 @@ func posMatcher(line int, char int) shared.Matcher {
3030
func TestGetDefinitions(t *testing.T) {
3131
t.Run("local", func(t *testing.T) {
3232
// Set up mocks
33-
mockRepoStore := defaultMockRepoStore()
33+
fakeRepoStore := AllPresentFakeRepoStore{}
3434
mockLsifStore := lsifstoremocks.NewMockLsifStore()
3535
mockUploadSvc := NewMockUploadService()
3636
mockGitserverClient := gitserver.NewMockClient()
3737
mockSearchClient := client.NewMockSearchClient()
3838

3939
// Init service
40-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
40+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
4141

4242
// Set up request state
4343
lookupPath := core.NewRepoRelPathUnchecked("sub2/a.go")
4444
mockRequestState := RequestState{Path: lookupPath}
45-
mockRequestState.SetLocalCommitCache(mockRepoStore, mockGitserverClient)
45+
mockRequestState.SetLocalCommitCache(fakeRepoStore, mockGitserverClient)
4646

4747
mockRequestState.GitTreeTranslator = noopTranslator()
4848
mockRequest := OccurrenceRequestArgs{
@@ -92,19 +92,19 @@ func TestGetDefinitions(t *testing.T) {
9292

9393
t.Run("remote", func(t *testing.T) {
9494
// Set up mocks
95-
mockRepoStore := defaultMockRepoStore()
95+
fakeRepoStore := AllPresentFakeRepoStore{}
9696
mockLsifStore := lsifstoremocks.NewMockLsifStore()
9797
mockUploadSvc := NewMockUploadService()
9898
mockGitserverClient := gitserver.NewMockClient()
9999
mockSearchClient := client.NewMockSearchClient()
100100

101101
// Init service
102-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
102+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
103103

104104
// Set up request state
105105
lookupPath := core.NewRepoRelPathUnchecked("sub2/a.go")
106106
mockRequestState := RequestState{Path: lookupPath}
107-
mockRequestState.SetLocalCommitCache(mockRepoStore, mockGitserverClient)
107+
mockRequestState.SetLocalCommitCache(fakeRepoStore, mockGitserverClient)
108108
mockRequestState.SetLocalGitTreeTranslator(mockGitserverClient, &sgtypes.Repo{ID: 42})
109109
mockRequestState.GitTreeTranslator = noopTranslator()
110110
uploads1 := []uploadsshared.CompletedUpload{
@@ -209,19 +209,19 @@ func TestGetDefinitions(t *testing.T) {
209209
func TestGetReferences(t *testing.T) {
210210
t.Run("local", func(t *testing.T) {
211211
// Set up mocks
212-
mockRepoStore := defaultMockRepoStore()
212+
fakeRepoStore := AllPresentFakeRepoStore{}
213213
mockLsifStore := lsifstoremocks.NewMockLsifStore()
214214
mockUploadSvc := NewMockUploadService()
215215
mockGitserverClient := gitserver.NewMockClient()
216216
mockSearchClient := client.NewMockSearchClient()
217217

218218
// Init service
219-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
219+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
220220

221221
// Set up request state
222222
lookupPath := core.NewRepoRelPathUnchecked("sub2/a.go")
223223
mockRequestState := RequestState{Path: lookupPath}
224-
mockRequestState.SetLocalCommitCache(mockRepoStore, mockGitserverClient)
224+
mockRequestState.SetLocalCommitCache(fakeRepoStore, mockGitserverClient)
225225
mockRequestState.GitTreeTranslator = noopTranslator()
226226
uploads := []uploadsshared.CompletedUpload{
227227
{ID: 50, Commit: "deadbeef", Root: "sub2/"},
@@ -285,19 +285,19 @@ func TestGetReferences(t *testing.T) {
285285

286286
t.Run("remote", func(t *testing.T) {
287287
// Set up mocks
288-
mockRepoStore := defaultMockRepoStore()
288+
fakeRepoStore := AllPresentFakeRepoStore{}
289289
mockLsifStore := lsifstoremocks.NewMockLsifStore()
290290
mockUploadSvc := NewMockUploadService()
291291
mockGitserverClient := gitserver.NewMockClient()
292292
mockSearchClient := client.NewMockSearchClient()
293293

294294
// Init service
295-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
295+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
296296

297297
// Set up request state
298298
lookupPath := core.NewRepoRelPathUnchecked("sub2/a.go")
299299
mockRequestState := RequestState{Path: lookupPath}
300-
mockRequestState.SetLocalCommitCache(mockRepoStore, mockGitserverClient)
300+
mockRequestState.SetLocalCommitCache(fakeRepoStore, mockGitserverClient)
301301
mockRequestState.GitTreeTranslator = noopTranslator()
302302
uploads := []uploadsshared.CompletedUpload{
303303
{ID: 50, Commit: "deadbeef", Root: "sub1/"},
@@ -458,19 +458,19 @@ func TestGetReferences(t *testing.T) {
458458
func TestGetImplementations(t *testing.T) {
459459
t.Run("local", func(t *testing.T) {
460460
// Set up mocks
461-
mockRepoStore := defaultMockRepoStore()
461+
fakeRepoStore := AllPresentFakeRepoStore{}
462462
mockLsifStore := lsifstoremocks.NewMockLsifStore()
463463
mockUploadSvc := NewMockUploadService()
464464
mockGitserverClient := gitserver.NewMockClient()
465465
mockSearchClient := client.NewMockSearchClient()
466466

467467
// Init service
468-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
468+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
469469

470470
// Set up request state
471471
lookupPath := core.NewRepoRelPathUnchecked("sub2/a.go")
472472
mockRequestState := RequestState{Path: lookupPath}
473-
mockRequestState.SetLocalCommitCache(mockRepoStore, mockGitserverClient)
473+
mockRequestState.SetLocalCommitCache(fakeRepoStore, mockGitserverClient)
474474
mockRequestState.GitTreeTranslator = noopTranslator()
475475

476476
// Empty result set (prevents nil pointer as scanner is always non-nil)

‎internal/codeintel/codenav/service_ranges_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ index deadbeef1..deadbeef2 100644
3636

3737
func TestRanges(t *testing.T) {
3838
// Set up mocks
39-
mockRepoStore := defaultMockRepoStore()
39+
fakeRepoStore := AllPresentFakeRepoStore{}
4040
mockLsifStore := lsifstoremocks.NewMockLsifStore()
4141
mockUploadSvc := NewMockUploadService()
4242
mockGitserverClient := gitserver.NewMockClient()
@@ -51,11 +51,11 @@ func TestRanges(t *testing.T) {
5151
mockLsifStore.FindDocumentIDsFunc.SetDefaultHook(findDocumentIDsFuncAllowAny())
5252

5353
// Init service
54-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
54+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
5555

5656
// Set up request state
5757
mockRequestState := RequestState{}
58-
mockRequestState.SetLocalCommitCache(mockRepoStore, mockGitserverClient)
58+
mockRequestState.SetLocalCommitCache(fakeRepoStore, mockGitserverClient)
5959
mockRequestState.SetLocalGitTreeTranslator(mockGitserverClient, &sgtypes.Repo{})
6060
uploads := []uploadsshared.CompletedUpload{
6161
{ID: 50, Commit: "deadbeef1", Root: "sub1/", RepositoryID: 42},

‎internal/codeintel/codenav/service_snapshot_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"github.com/sourcegraph/sourcegraph/internal/gitserver"
1717
"github.com/sourcegraph/sourcegraph/internal/observation"
1818
"github.com/sourcegraph/sourcegraph/internal/search/client"
19-
"github.com/sourcegraph/sourcegraph/internal/types"
2019
)
2120

2221
const sampleFile1 = `package food
@@ -25,18 +24,17 @@ type banana struct{}`
2524

2625
func TestSnapshotForDocument(t *testing.T) {
2726
// Set up mocks
28-
mockRepoStore := defaultMockRepoStore()
27+
fakeRepoStore := AllPresentFakeRepoStore{}
2928
mockLsifStore := lsifstoremocks.NewMockLsifStore()
3029
mockUploadSvc := NewMockUploadService()
3130
mockGitserverClient := gitserver.NewMockClient()
3231
mockGitserverClient.DiffFunc.SetDefaultReturn(gitserver.NewDiffFileIterator(io.NopCloser(strings.NewReader(""))), nil)
3332
mockSearchClient := client.NewMockSearchClient()
3433

3534
// Init service
36-
svc := newService(observation.TestContextTB(t), mockRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
35+
svc := newService(observation.TestContextTB(t), fakeRepoStore, mockLsifStore, mockUploadSvc, mockGitserverClient, mockSearchClient, log.NoOp())
3736

3837
mockUploadSvc.GetCompletedUploadsByIDsFunc.SetDefaultReturn([]shared.CompletedUpload{{}}, nil)
39-
mockRepoStore.GetFunc.SetDefaultReturn(&types.Repo{}, nil)
4038
mockGitserverClient.NewFileReaderFunc.SetDefaultReturn(io.NopCloser(bytes.NewReader([]byte(sampleFile1))), nil)
4139
mockLsifStore.SCIPDocumentFunc.SetDefaultReturn(core.Some(&scip.Document{
4240
RelativePath: "burger.go",

0 commit comments

Comments
 (0)