From d3df71ef9856e14f9b7bda6180325f39b1ec942c Mon Sep 17 00:00:00 2001 From: Christoph Hegemann Date: Wed, 10 Jul 2024 15:22:53 +0200 Subject: [PATCH] Adds a test for search-based usages (#63610) Closes https://linear.app/sourcegraph/issue/GRAPH-726/test-syntactic-and-search-based-usages Testing just the search-based usages just requires mocking the SearchClient, which works out nicely. ## Test plan The whole PR is just a test --- internal/codeintel/codenav/BUILD.bazel | 6 ++ internal/codeintel/codenav/service.go | 13 +++ .../codenav/service_syntactic_usages_test.go | 99 +++++++++++++++++++ internal/codeintel/codenav/syntactic.go | 16 +-- internal/codeintel/codenav/utils_test.go | 95 ++++++++++++++++++ 5 files changed, 223 insertions(+), 6 deletions(-) create mode 100644 internal/codeintel/codenav/service_syntactic_usages_test.go create mode 100644 internal/codeintel/codenav/utils_test.go diff --git a/internal/codeintel/codenav/BUILD.bazel b/internal/codeintel/codenav/BUILD.bazel index 6c775333f84..d70c5291f72 100644 --- a/internal/codeintel/codenav/BUILD.bazel +++ b/internal/codeintel/codenav/BUILD.bazel @@ -70,7 +70,9 @@ go_test( "service_references_test.go", "service_snapshot_test.go", "service_stencil_test.go", + "service_syntactic_usages_test.go", "service_test.go", + "utils_test.go", ], embed = [":codenav"], tags = [TAG_PLATFORM_GRAPH], @@ -87,10 +89,14 @@ go_test( "//internal/gitserver", "//internal/gitserver/gitdomain", "//internal/observation", + "//internal/search", "//internal/search/client", + "//internal/search/result", + "//internal/search/streaming", "//internal/types", "//lib/codeintel/precise", "@com_github_google_go_cmp//cmp", + "@com_github_life4_genesis//slices", "@com_github_sourcegraph_go_diff//diff", "@com_github_sourcegraph_log//:log", "@com_github_sourcegraph_scip//bindings/go/scip", diff --git a/internal/codeintel/codenav/service.go b/internal/codeintel/codenav/service.go index 04acce8b392..a861adade6e 100644 --- a/internal/codeintel/codenav/service.go +++ b/internal/codeintel/codenav/service.go @@ -1341,6 +1341,19 @@ func (s *Service) SearchBasedUsages( } } + return s.searchBasedUsagesInner(ctx, trace, args, symbolName, language, syntacticUploadID) +} + +// searchBasedUsagesInner is extracted from SearchBasedUsages to allow +// testing of the core logic, by only mocking the search client. +func (s *Service) searchBasedUsagesInner( + ctx context.Context, + trace observation.TraceLogger, + args UsagesForSymbolArgs, + symbolName string, + language string, + syntacticUploadID core.Option[int], +) (matches []SearchBasedMatch, err error) { searchCoords := searchArgs{ repo: args.Repo.Name, commit: args.Commit, diff --git a/internal/codeintel/codenav/service_syntactic_usages_test.go b/internal/codeintel/codenav/service_syntactic_usages_test.go new file mode 100644 index 00000000000..2137f0a255f --- /dev/null +++ b/internal/codeintel/codenav/service_syntactic_usages_test.go @@ -0,0 +1,99 @@ +package codenav + +import ( + "context" + "testing" + + genslices "github.com/life4/genesis/slices" + "github.com/sourcegraph/log" + "github.com/sourcegraph/scip/bindings/go/scip" + "github.com/stretchr/testify/require" + + "github.com/sourcegraph/sourcegraph/internal/codeintel/core" + "github.com/sourcegraph/sourcegraph/internal/gitserver" + "github.com/sourcegraph/sourcegraph/internal/observation" +) + +func expectSearchRanges(t *testing.T, matches []SearchBasedMatch, ranges ...scip.Range) { + t.Helper() + for _, match := range matches { + _, err := genslices.Find(ranges, func(r scip.Range) bool { + return match.Range.CompareStrict(r) == 0 + }) + if err != nil { + t.Errorf("Did not expect match at %q", match.Range.String()) + } + } + + for _, r := range ranges { + _, err := genslices.Find(matches, func(match SearchBasedMatch) bool { + return match.Range.CompareStrict(r) == 0 + }) + if err != nil { + t.Errorf("Expected match at %q", r.String()) + } + } +} + +func expectDefinitionRanges(t *testing.T, matches []SearchBasedMatch, ranges ...scip.Range) { + t.Helper() + for _, match := range matches { + _, err := genslices.Find(ranges, func(r scip.Range) bool { + return match.Range.CompareStrict(r) == 0 + }) + if match.IsDefinition && err != nil { + t.Errorf("Did not expect match at %q to be a definition", match.Range.String()) + return + } else if !match.IsDefinition && err == nil { + t.Errorf("Expected match at %q to be a definition", match.Range.String()) + return + } + } +} + +func TestSearchBasedUsages_ResultWithoutSymbols(t *testing.T) { + refRange := scipRange(1) + refRange2 := scipRange(2) + + mockSearchClient := FakeSearchClient(). + WithFile("path.java", refRange, refRange2). + Build() + + svc := newService( + observation.TestContextTB(t), defaultMockRepoStore(), + NewMockLsifStore(), NewMockUploadService(), gitserver.NewMockClient(), + mockSearchClient, log.NoOp(), + ) + + usages, searchErr := svc.searchBasedUsagesInner( + context.Background(), observation.TestTraceLogger(log.NoOp()), UsagesForSymbolArgs{}, + "symbol", "Java", core.None[int](), + ) + require.NoError(t, searchErr) + expectSearchRanges(t, usages, refRange, refRange2) +} + +func TestSearchBasedUsages_ResultWithSymbol(t *testing.T) { + refRange := scipRange(1) + defRange := scipRange(2) + refRange2 := scipRange(3) + + mockSearchClient := FakeSearchClient(). + WithFile("path.java", refRange, refRange2, defRange). + WithSymbols("path.java", defRange). + Build() + + svc := newService( + observation.TestContextTB(t), defaultMockRepoStore(), + NewMockLsifStore(), NewMockUploadService(), gitserver.NewMockClient(), + mockSearchClient, log.NoOp(), + ) + + usages, searchErr := svc.searchBasedUsagesInner( + context.Background(), observation.TestTraceLogger(log.NoOp()), UsagesForSymbolArgs{}, + "symbol", "Java", core.None[int](), + ) + require.NoError(t, searchErr) + expectSearchRanges(t, usages, refRange, refRange2, defRange) + expectDefinitionRanges(t, usages, defRange) +} diff --git a/internal/codeintel/codenav/syntactic.go b/internal/codeintel/codenav/syntactic.go index 0b12ac3f233..069ef717c2d 100644 --- a/internal/codeintel/codenav/syntactic.go +++ b/internal/codeintel/codenav/syntactic.go @@ -73,12 +73,7 @@ func findCandidateOccurrencesViaSearch( inconsistentFilepaths = 1 continue } - scipRange, err := scip.NewRange([]int32{ - int32(matchRange.Start.Line), - int32(matchRange.Start.Column), - int32(matchRange.End.Line), - int32(matchRange.End.Column), - }) + scipRange, err := scipFromResultRange(matchRange) if err != nil { trace.Warn("Failed to create scip range from match range", log.String("error", err.Error()), @@ -246,3 +241,12 @@ func sliceRangeFromReader(reader io.Reader, range_ scip.Range) (substr string, e } return "", errors.New("symbol range is out-of-bounds") } + +func scipFromResultRange(resultRange result.Range) (scip.Range, error) { + return scip.NewRange([]int32{ + int32(resultRange.Start.Line), + int32(resultRange.Start.Column), + int32(resultRange.End.Line), + int32(resultRange.End.Column), + }) +} diff --git a/internal/codeintel/codenav/utils_test.go b/internal/codeintel/codenav/utils_test.go new file mode 100644 index 00000000000..d156efa2123 --- /dev/null +++ b/internal/codeintel/codenav/utils_test.go @@ -0,0 +1,95 @@ +// This file does not contain tests for utils.go, instead it contains utils +// for setting up the test environment for our codenav tests. +package codenav + +import ( + "context" + "strings" + + genslices "github.com/life4/genesis/slices" + "github.com/sourcegraph/scip/bindings/go/scip" + + "github.com/sourcegraph/sourcegraph/internal/search" + searchClient "github.com/sourcegraph/sourcegraph/internal/search/client" + "github.com/sourcegraph/sourcegraph/internal/search/result" + "github.com/sourcegraph/sourcegraph/internal/search/streaming" +) + +// Generates a fake scip.Range that is easy to tell from other ranges +func scipRange(x int32) scip.Range { + return scip.NewRangeUnchecked([]int32{x, x, x}) +} + +func scipToResultPosition(p scip.Position) result.Location { + return result.Location{ + Line: int(p.Line), + Column: int(p.Character), + } +} + +func scipToResultRange(r scip.Range) result.Range { + return result.Range{ + Start: scipToResultPosition(r.Start), + End: scipToResultPosition(r.End), + } +} + +// scipToSymbolMatch "reverse engineers" the lsp.Range function on result.Symbol +func scipToSymbolMatch(r scip.Range) *result.SymbolMatch { + return &result.SymbolMatch{ + Symbol: result.Symbol{ + Line: int(r.Start.Line + 1), + Character: int(r.Start.Character), + Name: strings.Repeat("a", int(r.End.Character-r.Start.Character)), + }} +} + +type FakeSearchBuilder struct { + fileMatches []result.Match + symbolMatches []result.Match +} + +func FakeSearchClient() FakeSearchBuilder { + return FakeSearchBuilder{ + fileMatches: []result.Match{}, + symbolMatches: make([]result.Match, 0), + } +} + +func (b FakeSearchBuilder) WithFile(file string, ranges ...scip.Range) FakeSearchBuilder { + b.fileMatches = append(b.fileMatches, &result.FileMatch{ + File: result.File{Path: file}, + ChunkMatches: result.ChunkMatches{{ + Ranges: genslices.Map(ranges, scipToResultRange), + }}, + }) + return b +} + +func (b FakeSearchBuilder) WithSymbols(file string, ranges ...scip.Range) FakeSearchBuilder { + b.symbolMatches = append(b.symbolMatches, &result.FileMatch{ + File: result.File{Path: file}, + Symbols: genslices.Map(ranges, scipToSymbolMatch), + }) + return b +} + +func (b FakeSearchBuilder) Build() searchClient.SearchClient { + mockSearchClient := searchClient.NewMockSearchClient() + mockSearchClient.PlanFunc.SetDefaultHook(func(_ context.Context, _ string, _ *string, query string, _ search.Mode, _ search.Protocol, _ *int32) (*search.Inputs, error) { + return &search.Inputs{OriginalQuery: query}, nil + }) + mockSearchClient.ExecuteFunc.SetDefaultHook(func(_ context.Context, s streaming.Sender, i *search.Inputs) (*search.Alert, error) { + if strings.Contains(i.OriginalQuery, "type:file") { + s.Send(streaming.SearchEvent{ + Results: b.fileMatches, + }) + } else if strings.Contains(i.OriginalQuery, "type:symbol") { + s.Send(streaming.SearchEvent{ + Results: b.symbolMatches, + }) + } + return nil, nil + }) + return mockSearchClient +}