diff options
| author | Taras Madan <tarasmadan@google.com> | 2025-01-23 21:54:41 +0100 |
|---|---|---|
| committer | Taras Madan <tarasmadan@google.com> | 2025-01-27 10:05:21 +0000 |
| commit | 2bf68614de1620ef12f086d9e86d5c8b334bf32d (patch) | |
| tree | 24b69669a1ee44c1a34988d917cb248faed40cd3 /dashboard/app | |
| parent | 0868754a9d325ba9011e1cb74510f68d4b627c79 (diff) | |
dashboard/app: test coverage /file link
1. Init coveragedb client once and propagate it through context to enable mocking.
2. Always init coverage handlers. It simplifies testing.
3. Read webGit and coveragedb client from ctx to make it mockable.
4. Use int for file line number and int64 for merged coverage.
5. Add tests.
Diffstat (limited to 'dashboard/app')
| -rw-r--r-- | dashboard/app/api.go | 9 | ||||
| -rw-r--r-- | dashboard/app/batch_coverage.go | 4 | ||||
| -rw-r--r-- | dashboard/app/config.go | 1 | ||||
| -rw-r--r-- | dashboard/app/coverage.go | 77 | ||||
| -rw-r--r-- | dashboard/app/coverage_test.go | 171 | ||||
| -rw-r--r-- | dashboard/app/entities_spanner.go | 11 | ||||
| -rw-r--r-- | dashboard/app/handler.go | 3 | ||||
| -rw-r--r-- | dashboard/app/main.go | 12 | ||||
| -rw-r--r-- | dashboard/app/util_test.go | 16 |
9 files changed, 268 insertions, 36 deletions
diff --git a/dashboard/app/api.go b/dashboard/app/api.go index b06c5ac07..e1269b3d0 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -26,7 +26,6 @@ import ( "github.com/google/syzkaller/pkg/asset" "github.com/google/syzkaller/pkg/auth" "github.com/google/syzkaller/pkg/coveragedb" - "github.com/google/syzkaller/pkg/coveragedb/spannerclient" "github.com/google/syzkaller/pkg/debugtracer" "github.com/google/syzkaller/pkg/email" "github.com/google/syzkaller/pkg/gcs" @@ -105,6 +104,7 @@ var maxCrashes = func() int { func handleJSON(fn JSONHandler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { c := appengine.NewContext(r) + c = SetCoverageDBClient(c, coverageDBClient) reply, err := fn(c, r) if err != nil { status := logErrorPrepareStatus(c, err) @@ -1948,12 +1948,7 @@ func apiSaveCoverage(c context.Context, payload io.Reader) (interface{}, error) sss = service.List() log.Infof(c, "found %d subsystems for %s namespace", len(sss), descr.Namespace) } - client, err := spannerclient.NewClient(c, appengine.AppID(context.Background())) - if err != nil { - return 0, fmt.Errorf("coveragedb.NewClient() failed: %s", err.Error()) - } - defer client.Close() - rowsCreated, err := coveragedb.SaveMergeResult(c, client, descr, jsonDec, sss) + rowsCreated, err := coveragedb.SaveMergeResult(c, GetCoverageDBClient(c), descr, jsonDec, sss) if err != nil { log.Errorf(c, "error storing coverage for ns %s, date %s: %v", descr.Namespace, descr.DateTo.String(), err) diff --git a/dashboard/app/batch_coverage.go b/dashboard/app/batch_coverage.go index 6d53496a7..aba85cf59 100644 --- a/dashboard/app/batch_coverage.go +++ b/dashboard/app/batch_coverage.go @@ -43,7 +43,7 @@ func handleBatchCoverage(w http.ResponseWriter, r *http.Request) { if err != nil { log.Errorf(ctx, "failed nsDataAvailable(%s): %s", ns, err) } - periodsMerged, rowsMerged, err := coveragedb.NsDataMerged(ctx, "syzkaller", ns) + periodsMerged, rowsMerged, err := coveragedb.NsDataMerged(ctx, coverageDBClient, ns) if err != nil { log.Errorf(ctx, "failed coveragedb.NsDataMerged(%s): %s", ns, err) } @@ -154,7 +154,7 @@ func nsDataAvailable(ctx context.Context, ns string) ([]coveragedb.TimePeriod, [ func handleBatchCoverageClean(w http.ResponseWriter, r *http.Request) { ctx := context.Background() - totalDeleted, err := coveragedb.DeleteGarbage(ctx) + totalDeleted, err := coveragedb.DeleteGarbage(ctx, coverageDBClient) if err != nil { errMsg := fmt.Sprintf("failed to coveragedb.DeleteGarbage: %s", err.Error()) log.Errorf(ctx, "%s", errMsg) diff --git a/dashboard/app/config.go b/dashboard/app/config.go index 6d173cb19..c71d09d8e 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -425,6 +425,7 @@ func installConfig(cfg *GlobalConfig) { initAPIHandlers() initKcidb() initBatchProcessors() + initCoverageDB() } var contextConfigKey = "Updated config (to be used during tests). Use only in tests!" diff --git a/dashboard/app/coverage.go b/dashboard/app/coverage.go index 7bc0d2a06..c91c7d665 100644 --- a/dashboard/app/coverage.go +++ b/dashboard/app/coverage.go @@ -8,6 +8,7 @@ import ( "fmt" "html/template" "net/http" + "os" "slices" "strconv" @@ -17,8 +18,36 @@ import ( "github.com/google/syzkaller/pkg/coveragedb/spannerclient" "github.com/google/syzkaller/pkg/covermerger" "github.com/google/syzkaller/pkg/validator" + "google.golang.org/appengine/v2" ) +var coverageDBClient spannerclient.SpannerClient + +func initCoverageDB() { + if !appengine.IsAppEngine() { + // It is a test environment. + // Use SetCoverageDBClient to specify the coveragedb mock or emulator in every test. + return + } + projectID := os.Getenv("GOOGLE_CLOUD_PROJECT") + var err error + coverageDBClient, err = spannerclient.NewClient(context.Background(), projectID) + if err != nil { + panic("spanner.NewClient: " + err.Error()) + } +} + +var keyCoverageDBClient = "coveragedb client key" + +func SetCoverageDBClient(ctx context.Context, client spannerclient.SpannerClient) context.Context { + return context.WithValue(ctx, &keyCoverageDBClient, client) +} + +func GetCoverageDBClient(ctx context.Context) spannerclient.SpannerClient { + client, _ := ctx.Value(&keyCoverageDBClient).(spannerclient.SpannerClient) + return client +} + type funcStyleBodyJS func( ctx context.Context, client spannerclient.SpannerClient, scope *cover.SelectScope, onlyUnique bool, sss, managers []string, @@ -37,6 +66,10 @@ func handleHeatmap(c context.Context, w http.ResponseWriter, r *http.Request, f if err != nil { return err } + nsConfig := getNsConfig(c, hdr.Namespace) + if nsConfig.Coverage == nil { + return ErrClientNotFound + } ss := r.FormValue("subsystem") manager := r.FormValue("manager") @@ -76,15 +109,9 @@ func handleHeatmap(c context.Context, w http.ResponseWriter, r *http.Request, f onlyUnique := r.FormValue("unique-only") == "1" - spannerClient, err := spannerclient.NewClient(c, "syzkaller") - if err != nil { - return fmt.Errorf("spanner.NewClient: %s", err.Error()) - } - defer spannerClient.Close() - var style template.CSS var body, js template.HTML - if style, body, js, err = f(c, spannerClient, + if style, body, js, err = f(c, GetCoverageDBClient(c), &cover.SelectScope{ Ns: hdr.Namespace, Subsystem: ss, @@ -147,24 +174,37 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques } onlyUnique := r.FormValue("unique-only") == "1" mainNsRepo, _ := nsConfig.mainRepoBranch() - hitLines, hitCounts, err := coveragedb.ReadLinesHitCount(c, hdr.Namespace, targetCommit, manager, kernelFilePath, tp) + client := GetCoverageDBClient(c) + if client == nil { + return fmt.Errorf("spannerdb client is nil") + } + hitLines, hitCounts, err := coveragedb.ReadLinesHitCount( + c, client, hdr.Namespace, targetCommit, kernelFilePath, manager, tp) covMap := cover.MakeCovMap(hitLines, hitCounts) if err != nil { return fmt.Errorf("coveragedb.ReadLinesHitCount(%s): %w", manager, err) } if onlyUnique { - allHitLines, allHitCounts, err := coveragedb.ReadLinesHitCount(c, hdr.Namespace, targetCommit, manager, kernelFilePath, tp) + // This request is expected to be made second by tests. + // Moving it to goroutine don't forget to change multiManagerCovDBFixture. + allHitLines, allHitCounts, err := coveragedb.ReadLinesHitCount( + c, client, hdr.Namespace, targetCommit, kernelFilePath, "*", tp) if err != nil { return fmt.Errorf("coveragedb.ReadLinesHitCount(*): %w", err) } covMap = cover.UniqCoverage(cover.MakeCovMap(allHitLines, allHitCounts), covMap) } + webGit := getWebGit(c) // Get mock if available. + if webGit == nil { + webGit = covermerger.MakeWebGit(makeProxyURIProvider(nsConfig.Coverage.WebGitURI)) + } + content, err := cover.RendFileCoverage( mainNsRepo, targetCommit, kernelFilePath, - makeProxyURIProvider(nsConfig.Coverage.WebGitURI), + webGit, &covermerger.MergeResult{HitCounts: covMap}, cover.DefaultHTMLRenderConfig()) if err != nil { @@ -175,11 +215,26 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques return nil } +var keyWebGit = "file content provider" + +func setWebGit(ctx context.Context, provider covermerger.FileVersProvider) context.Context { + return context.WithValue(ctx, &keyWebGit, provider) +} + +func getWebGit(ctx context.Context) covermerger.FileVersProvider { + res, _ := ctx.Value(&keyWebGit).(covermerger.FileVersProvider) + return res +} + func handleCoverageGraph(c context.Context, w http.ResponseWriter, r *http.Request) error { hdr, err := commonHeader(c, r, w, "") if err != nil { return err } + nsConfig := getNsConfig(c, hdr.Namespace) + if nsConfig.Coverage == nil { + return ErrClientNotFound + } periodType := r.FormValue("period") if periodType == "" { periodType = coveragedb.QuarterPeriod @@ -187,7 +242,7 @@ func handleCoverageGraph(c context.Context, w http.ResponseWriter, r *http.Reque if periodType != coveragedb.QuarterPeriod && periodType != coveragedb.MonthPeriod { return fmt.Errorf("only quarter and month are allowed, but received %s instead", periodType) } - hist, err := MergedCoverage(c, hdr.Namespace, periodType) + hist, err := MergedCoverage(c, GetCoverageDBClient(c), hdr.Namespace, periodType) if err != nil { return err } diff --git a/dashboard/app/coverage_test.go b/dashboard/app/coverage_test.go new file mode 100644 index 000000000..ef8321c9f --- /dev/null +++ b/dashboard/app/coverage_test.go @@ -0,0 +1,171 @@ +// Copyright 2025 syzkaller project authors. All rights reserved. +// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +package main + +import ( + "strings" + "testing" + + "github.com/google/syzkaller/pkg/coveragedb" + "github.com/google/syzkaller/pkg/coveragedb/mocks" + "github.com/google/syzkaller/pkg/coveragedb/spannerclient" + "github.com/google/syzkaller/pkg/covermerger" + mergermocks "github.com/google/syzkaller/pkg/covermerger/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "google.golang.org/api/iterator" +) + +func TestFileCoverage(t *testing.T) { + tests := []struct { + name string + covDB func(t *testing.T) spannerclient.SpannerClient + fileProv func(t *testing.T) covermerger.FileVersProvider + url string + wantInRes []string + }{ + { + name: "empty db", + covDB: func(t *testing.T) spannerclient.SpannerClient { return emptyCoverageDBFixture(t, 1) }, + fileProv: func(t *testing.T) covermerger.FileVersProvider { return staticFileProvider(t) }, + url: "/test2/graph/coverage/file?dateto=2025-01-31&period=month" + + "&commit=c0e75905caf368e19aab585d20151500e750de89&filepath=virt/kvm/kvm_main.c", + wantInRes: []string{"1 line1"}, + }, + { + name: "regular db", + covDB: func(t *testing.T) spannerclient.SpannerClient { return coverageDBFixture(t) }, + fileProv: func(t *testing.T) covermerger.FileVersProvider { return staticFileProvider(t) }, + url: "/test2/graph/coverage/file?dateto=2025-01-31&period=month" + + "&commit=c0e75905caf368e19aab585d20151500e750de89&filepath=virt/kvm/kvm_main.c", + wantInRes: []string{ + "4 1 line1", + "5 2 line2", + "6 3 line3"}, + }, + { + name: "multimanager db", + covDB: func(t *testing.T) spannerclient.SpannerClient { return multiManagerCovDBFixture(t) }, + fileProv: func(t *testing.T) covermerger.FileVersProvider { return staticFileProvider(t) }, + url: "/test2/graph/coverage/file?dateto=2025-01-31&period=month" + + "&commit=c0e75905caf368e19aab585d20151500e750de89&filepath=virt/kvm/kvm_main.c" + + "&manager=special-cc-manager&unique-only=1", + wantInRes: []string{ + " 0 1 line1", // Covered, is not unique. + " 5 2 line2", // Covered and is unique. + " 3 line3", // Covered only by "*" managers. + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + c := NewCtx(t) + defer c.Close() + c.setCoverageMocks("test2", test.covDB(t), test.fileProv(t)) + fileCovPage, err := c.GET(test.url) + assert.NoError(t, err) + got := string(fileCovPage) + for _, want := range test.wantInRes { + if !strings.Contains(got, want) { + t.Errorf(`"%s" wasn't found in "%s"'`, want, got) + } + } + }) + } +} + +func staticFileProvider(t *testing.T) covermerger.FileVersProvider { + m := mergermocks.NewFileVersProvider(t) + m.On("GetFileVersions", mock.Anything, mock.Anything). + Return(func(targetFilePath string, repoCommits ...covermerger.RepoCommit, + ) covermerger.FileVersions { + res := covermerger.FileVersions{} + for _, rc := range repoCommits { + res[rc] = `line1 +line2 +line3` + } + return res + }, nil) + return m +} + +func emptyCoverageDBFixture(t *testing.T, times int) spannerclient.SpannerClient { + mRowIterator := mocks.NewRowIterator(t) + mRowIterator.On("Stop").Return().Times(times) + mRowIterator.On("Next"). + Return(nil, iterator.Done).Times(times) + + mTran := mocks.NewReadOnlyTransaction(t) + mTran.On("Query", mock.Anything, mock.Anything). + Return(mRowIterator).Times(times) + + m := mocks.NewSpannerClient(t) + m.On("Single"). + Return(mTran).Times(times) + return m +} + +func coverageDBFixture(t *testing.T) spannerclient.SpannerClient { + mRowIt := newRowIteratorMock(t, []*coveragedb.LinesCoverage{{ + LinesInstrumented: []int64{1, 2, 3}, + HitCounts: []int64{4, 5, 6}, + }}) + + mTran := mocks.NewReadOnlyTransaction(t) + mTran.On("Query", mock.Anything, mock.Anything). + Return(mRowIt).Once() + + m := mocks.NewSpannerClient(t) + m.On("Single"). + Return(mTran).Once() + return m +} + +func multiManagerCovDBFixture(t *testing.T) spannerclient.SpannerClient { + mReadFullCoverageTran := mocks.NewReadOnlyTransaction(t) + mReadFullCoverageTran.On("Query", mock.Anything, mock.Anything). + Return(newRowIteratorMock(t, []*coveragedb.LinesCoverage{{ + LinesInstrumented: []int64{1, 2, 3}, + HitCounts: []int64{4, 5, 6}, + }})).Once() + + mReadPartialCoverageTran := mocks.NewReadOnlyTransaction(t) + mReadPartialCoverageTran.On("Query", mock.Anything, mock.Anything). + Return(newRowIteratorMock(t, []*coveragedb.LinesCoverage{{ + LinesInstrumented: []int64{1, 2}, + HitCounts: []int64{3, 5}, + }})).Once() + + m := mocks.NewSpannerClient(t) + // The order matters. Full coverage is fetched second. + m.On("Single"). + Return(mReadPartialCoverageTran).Once() + m.On("Single"). + Return(mReadFullCoverageTran).Once() + + return m +} + +func newRowIteratorMock(t *testing.T, cov []*coveragedb.LinesCoverage, +) *mocks.RowIterator { + m := mocks.NewRowIterator(t) + m.On("Stop").Once().Return() + for _, item := range cov { + mRow := mocks.NewRow(t) + mRow.On("ToStruct", mock.Anything). + Run(func(args mock.Arguments) { + arg := args.Get(0).(*coveragedb.LinesCoverage) + *arg = *item + }). + Return(nil).Once() + + m.On("Next"). + Return(mRow, nil).Once() + } + + m.On("Next"). + Return(nil, iterator.Done).Once() + return m +} diff --git a/dashboard/app/entities_spanner.go b/dashboard/app/entities_spanner.go index e3f3bad9e..c550ef92e 100644 --- a/dashboard/app/entities_spanner.go +++ b/dashboard/app/entities_spanner.go @@ -6,7 +6,6 @@ package main import ( "context" "fmt" - "os" "cloud.google.com/go/civil" "cloud.google.com/go/spanner" @@ -24,14 +23,8 @@ type CoverageHistory struct { } // MergedCoverage uses dates, not time. -func MergedCoverage(ctx context.Context, ns, periodType string) (*CoverageHistory, error) { - projectID := os.Getenv("GOOGLE_CLOUD_PROJECT") - client, err := spannerclient.NewClient(ctx, projectID) - if err != nil { - return nil, fmt.Errorf("spanner.NewClient() failed: %s", err.Error()) - } - defer client.Close() - +func MergedCoverage(ctx context.Context, client spannerclient.SpannerClient, ns, periodType string, +) (*CoverageHistory, error) { minDays, maxDays, err := coveragedb.MinMaxDays(periodType) if err != nil { return nil, fmt.Errorf("coveragedb.MinMaxDays: %w", err) diff --git a/dashboard/app/handler.go b/dashboard/app/handler.go index e737c2b0b..27817ab8c 100644 --- a/dashboard/app/handler.go +++ b/dashboard/app/handler.go @@ -32,6 +32,9 @@ func handlerWrapper(fn contextHandler) http.Handler { func handleContext(fn contextHandler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { c := appengine.NewContext(r) + if coverageDBClient != nil { // Nil in prod. + c = SetCoverageDBClient(c, coverageDBClient) + } c = context.WithValue(c, ¤tURLKey, r.URL.RequestURI()) authorizedUser, _ := userAccessLevel(currentUser(c), "", getConfig(c)) if !authorizedUser { diff --git a/dashboard/app/main.go b/dashboard/app/main.go index d765c8df9..071dc8544 100644 --- a/dashboard/app/main.go +++ b/dashboard/app/main.go @@ -65,13 +65,11 @@ func initHTTPHandlers() { http.Handle("/"+ns+"/graph/fuzzing", handlerWrapper(handleGraphFuzzing)) http.Handle("/"+ns+"/graph/crashes", handlerWrapper(handleGraphCrashes)) http.Handle("/"+ns+"/graph/found-bugs", handlerWrapper(handleFoundBugsGraph)) - if nsConfig.Coverage != nil { - http.Handle("/"+ns+"/graph/coverage/file", handlerWrapper(handleFileCoverage)) - http.Handle("/"+ns+"/graph/coverage", handlerWrapper(handleCoverageGraph)) - http.Handle("/"+ns+"/graph/coverage_heatmap", handlerWrapper(handleCoverageHeatmap)) - if nsConfig.Subsystems.Service != nil { - http.Handle("/"+ns+"/graph/coverage_subsystems_heatmap", handlerWrapper(handleSubsystemsCoverageHeatmap)) - } + http.Handle("/"+ns+"/graph/coverage/file", handlerWrapper(handleFileCoverage)) + http.Handle("/"+ns+"/graph/coverage", handlerWrapper(handleCoverageGraph)) + http.Handle("/"+ns+"/graph/coverage_heatmap", handlerWrapper(handleCoverageHeatmap)) + if nsConfig.Subsystems.Service != nil { + http.Handle("/"+ns+"/graph/coverage_subsystems_heatmap", handlerWrapper(handleSubsystemsCoverageHeatmap)) } http.Handle("/"+ns+"/repos", handlerWrapper(handleRepos)) http.Handle("/"+ns+"/bug-summaries", handlerWrapper(handleBugSummaries)) diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go index d2ed77e32..be7841b27 100644 --- a/dashboard/app/util_test.go +++ b/dashboard/app/util_test.go @@ -28,6 +28,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/syzkaller/dashboard/api" "github.com/google/syzkaller/dashboard/dashapi" + "github.com/google/syzkaller/pkg/coveragedb/spannerclient" + "github.com/google/syzkaller/pkg/covermerger" "github.com/google/syzkaller/pkg/email" "github.com/google/syzkaller/pkg/subsystem" "google.golang.org/appengine/v2/aetest" @@ -226,6 +228,20 @@ func (c *Ctx) setSubsystems(ns string, list []*subsystem.Subsystem, rev int) { } } +func (c *Ctx) setCoverageMocks(ns string, dbClientMock spannerclient.SpannerClient, + fileProvMock covermerger.FileVersProvider) { + c.transformContext = func(ctx context.Context) context.Context { + newConfig := replaceNamespaceConfig(ctx, ns, func(cfg *Config) *Config { + ret := *cfg + ret.Coverage = &CoverageConfig{WebGitURI: "test-git"} + return &ret + }) + ctxWithSpanner := SetCoverageDBClient(ctx, dbClientMock) + ctxWithSpannerAndFileProvider := setWebGit(ctxWithSpanner, fileProvMock) + return contextWithConfig(ctxWithSpannerAndFileProvider, newConfig) + } +} + func (c *Ctx) setKernelRepos(ns string, list []KernelRepo) { c.transformContext = func(c context.Context) context.Context { newConfig := replaceNamespaceConfig(c, ns, func(cfg *Config) *Config { |
