From 88a53bf90e2c1b519b54b78923d5d82977ae1fc6 Mon Sep 17 00:00:00 2001 From: Taras Madan Date: Thu, 9 Jan 2025 15:45:40 +0100 Subject: [PATCH 1/3] dashboard/app: show manager unique coverage 1. Make heatmap testable, move out the spanner client instantiation. 2. Generate spannerdb.ReadOnlyTransaction mocks. 3. Generate spannerdb.RowIterator mocks. 4. Generate spannerdb.Row mocks. 5. Prepare spannerdb fixture. 6. Fixed html control name + value. 7. Added multiple tests. 8. Show line coverage from selected manager. 9. Propagate coverage url params to file coverage url. --- dashboard/app/coverage.go | 36 ++- dashboard/app/static/coverage.js | 7 +- pkg/cover/file.go | 2 +- pkg/cover/heatmap.go | 206 ++++++++++++++--- pkg/cover/heatmap_test.go | 240 ++++++++++++++++++++ pkg/cover/templates/heatmap.html | 4 +- pkg/coveragedb/coveragedb.go | 33 ++- pkg/coveragedb/coveragedb_mock_test.go | 3 + pkg/coveragedb/mocks/ReadOnlyTransaction.go | 51 +++++ pkg/coveragedb/mocks/Row.go | 42 ++++ pkg/coveragedb/mocks/RowIterator.go | 62 +++++ pkg/covermerger/covermerger.go | 6 +- pkg/covermerger/file_line_merger.go | 4 +- pkg/validator/validator.go | 21 ++ pkg/validator/validator_test.go | 18 ++ 15 files changed, 673 insertions(+), 62 deletions(-) create mode 100644 pkg/coveragedb/mocks/ReadOnlyTransaction.go create mode 100644 pkg/coveragedb/mocks/Row.go create mode 100644 pkg/coveragedb/mocks/RowIterator.go diff --git a/dashboard/app/coverage.go b/dashboard/app/coverage.go index e0ceab563ee1..7bc0d2a06660 100644 --- a/dashboard/app/coverage.go +++ b/dashboard/app/coverage.go @@ -14,11 +14,14 @@ import ( "cloud.google.com/go/civil" "github.com/google/syzkaller/pkg/cover" "github.com/google/syzkaller/pkg/coveragedb" + "github.com/google/syzkaller/pkg/coveragedb/spannerclient" "github.com/google/syzkaller/pkg/covermerger" "github.com/google/syzkaller/pkg/validator" ) -type funcStyleBodyJS func(ctx context.Context, projectID string, scope *cover.SelectScope, sss, managers []string, +type funcStyleBodyJS func( + ctx context.Context, client spannerclient.SpannerClient, + scope *cover.SelectScope, onlyUnique bool, sss, managers []string, ) (template.CSS, template.HTML, template.HTML, error) func handleCoverageHeatmap(c context.Context, w http.ResponseWriter, r *http.Request) error { @@ -71,16 +74,24 @@ func handleHeatmap(c context.Context, w http.ResponseWriter, r *http.Request, f slices.Sort(managers) slices.Sort(subsystems) + 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, "syzkaller", + if style, body, js, err = f(c, spannerClient, &cover.SelectScope{ Ns: hdr.Namespace, Subsystem: ss, Manager: manager, Periods: periods, }, - subsystems, managers); err != nil { + onlyUnique, subsystems, managers); err != nil { return fmt.Errorf("failed to generate heatmap: %w", err) } return serveTemplate(w, "custom_content.html", struct { @@ -115,10 +126,14 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques periodType := r.FormValue("period") targetCommit := r.FormValue("commit") kernelFilePath := r.FormValue("filepath") + manager := r.FormValue("manager") if err := validator.AnyError("input validation failed", validator.TimePeriodType(periodType, "period"), validator.CommitHash(targetCommit, "commit"), validator.KernelFilePath(kernelFilePath, "filepath"), + validator.AnyOk( + validator.Allowlisted(manager, []string{"", "*"}, "manager"), + validator.ManagerName(manager, "manager")), ); err != nil { return fmt.Errorf("%w: %w", err, ErrClientBadRequest) } @@ -130,10 +145,19 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques if err != nil { return fmt.Errorf("coveragedb.MakeTimePeriod: %w", err) } + onlyUnique := r.FormValue("unique-only") == "1" mainNsRepo, _ := nsConfig.mainRepoBranch() - hitCounts, err := coveragedb.ReadLinesHitCount(c, hdr.Namespace, targetCommit, kernelFilePath, tp) + hitLines, hitCounts, err := coveragedb.ReadLinesHitCount(c, hdr.Namespace, targetCommit, manager, kernelFilePath, tp) + covMap := cover.MakeCovMap(hitLines, hitCounts) if err != nil { - return fmt.Errorf("coveragedb.ReadLinesHitCount: %w", err) + return fmt.Errorf("coveragedb.ReadLinesHitCount(%s): %w", manager, err) + } + if onlyUnique { + allHitLines, allHitCounts, err := coveragedb.ReadLinesHitCount(c, hdr.Namespace, targetCommit, manager, kernelFilePath, tp) + if err != nil { + return fmt.Errorf("coveragedb.ReadLinesHitCount(*): %w", err) + } + covMap = cover.UniqCoverage(cover.MakeCovMap(allHitLines, allHitCounts), covMap) } content, err := cover.RendFileCoverage( @@ -141,7 +165,7 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques targetCommit, kernelFilePath, makeProxyURIProvider(nsConfig.Coverage.WebGitURI), - &covermerger.MergeResult{HitCounts: hitCounts}, + &covermerger.MergeResult{HitCounts: covMap}, cover.DefaultHTMLRenderConfig()) if err != nil { return fmt.Errorf("cover.RendFileCoverage: %w", err) diff --git a/dashboard/app/static/coverage.js b/dashboard/app/static/coverage.js index 92fccc0914e8..8a5190b90dec 100644 --- a/dashboard/app/static/coverage.js +++ b/dashboard/app/static/coverage.js @@ -20,7 +20,7 @@ function initUpdateForm(){ } $('#target-subsystem').val(curUrlParams.get('subsystem')); $('#target-manager').val(curUrlParams.get('manager')); - $("#only-unique").prop("checked", curUrlParams.get('subsystem') == "1"); + $("#unique-only").prop("checked", curUrlParams.get('unique-only') == "1"); } // This handler is called when user clicks on the coverage percentage. @@ -28,6 +28,11 @@ function initUpdateForm(){ // "#file-content-prev" and "#file-content-curr" are the file content
s. // "#file-details-prev" and "#file-details-curr" are the corresponding
s used to show per-file details. function onShowFileContent(url) { + var curUrlParams = new URLSearchParams(window.location.search); + url += '&subsystem=' + curUrlParams.get('subsystem') + url += '&manager=' + curUrlParams.get('manager') + url += '&unique-only=' + curUrlParams.get('unique-only') + $.get(url, function(response) { $("#file-content-prev").html($("#file-content-curr").html()); $("#file-content-curr").html(response); diff --git a/pkg/cover/file.go b/pkg/cover/file.go index a0561dfe74e9..c56f882045c6 100644 --- a/pkg/cover/file.go +++ b/pkg/cover/file.go @@ -101,7 +101,7 @@ func GetMergeResult(c context.Context, ns, repo, forCommit, sourceCommit, filePa func rendResult(content string, coverage *covermerger.MergeResult, renderConfig *CoverageRenderConfig) string { if coverage == nil { coverage = &covermerger.MergeResult{ - HitCounts: map[int]int{}, + HitCounts: map[int]int64{}, LineDetails: map[int][]*covermerger.FileRecord{}, } } diff --git a/pkg/cover/heatmap.go b/pkg/cover/heatmap.go index b8e3bbfa6690..04b7e6914b77 100644 --- a/pkg/cover/heatmap.go +++ b/pkg/cover/heatmap.go @@ -17,6 +17,7 @@ import ( "github.com/google/syzkaller/pkg/coveragedb/spannerclient" _ "github.com/google/syzkaller/pkg/subsystem/lists" "golang.org/x/exp/maps" + "golang.org/x/sync/errgroup" "google.golang.org/api/iterator" ) @@ -115,6 +116,16 @@ type fileCoverageWithDetails struct { Subsystems []string } +type fileCoverageWithLineInfo struct { + fileCoverageWithDetails + LinesInstrumented []int64 + HitCounts []int64 +} + +func (fc *fileCoverageWithLineInfo) CovMap() map[int]int64 { + return MakeCovMap(fc.LinesInstrumented, fc.HitCounts) +} + type pageColumnTarget struct { TimePeriod coveragedb.TimePeriod Commit string @@ -157,18 +168,17 @@ func filesCoverageToTemplateData(fCov []*fileCoverageWithDetails) *templateHeatm return &res } -func filesCoverageWithDetailsStmt(ns, subsystem, manager string, timePeriod coveragedb.TimePeriod) spanner.Statement { +func filesCoverageWithDetailsStmt(ns, subsystem, manager string, timePeriod coveragedb.TimePeriod, withLines bool, +) spanner.Statement { if manager == "" { manager = "*" } + selectColumns := "commit, instrumented, covered, files.filepath, subsystems" + if withLines { + selectColumns += ", linesinstrumented, hitcounts" + } stmt := spanner.Statement{ - SQL: ` -select - commit, - instrumented, - covered, - files.filepath, - subsystems + SQL: "select " + selectColumns + ` from merge_history join files on merge_history.session = files.session @@ -187,37 +197,171 @@ where stmt.SQL += " and $5=ANY(subsystems)" stmt.Params["p5"] = subsystem } + stmt.SQL += "\norder by files.filepath" return stmt } -func filesCoverageWithDetails(ctx context.Context, projectID string, scope *SelectScope, -) ([]*fileCoverageWithDetails, error) { - client, err := spannerclient.NewClient(ctx, projectID) +func readCoverage(iterManager spannerclient.RowIterator) ([]*fileCoverageWithDetails, error) { + res := []*fileCoverageWithDetails{} + ch := make(chan *fileCoverageWithDetails) + var err error + go func() { + defer close(ch) + err = readIterToChan(context.Background(), iterManager, ch) + }() + for fc := range ch { + res = append(res, fc) + } if err != nil { - return nil, fmt.Errorf("spanner.NewClient() failed: %s", err.Error()) + return nil, fmt.Errorf("readIterToChan: %w", err) } - defer client.Close() + return res, nil +} +// Unique coverage from specific manager is more expensive to get. +// We get unique coverage comparing manager and total coverage on the AppEngine side. +func readCoverageUniq(full, mgr spannerclient.RowIterator, +) ([]*fileCoverageWithDetails, error) { + eg, ctx := errgroup.WithContext(context.Background()) + fullCh := make(chan *fileCoverageWithLineInfo) + eg.Go(func() error { + defer close(fullCh) + return readIterToChan(ctx, full, fullCh) + }) + partCh := make(chan *fileCoverageWithLineInfo) + eg.Go(func() error { + defer close(partCh) + return readIterToChan(ctx, mgr, partCh) + }) res := []*fileCoverageWithDetails{} - for _, timePeriod := range scope.Periods { - stmt := filesCoverageWithDetailsStmt(scope.Ns, scope.Subsystem, scope.Manager, timePeriod) - iter := client.Single().Query(ctx, stmt) - defer iter.Stop() - for { - row, err := iter.Next() - if err == iterator.Done { - break + eg.Go(func() error { + partCov := <-partCh + for fullCov := range fullCh { + if partCov == nil || partCov.Filepath > fullCov.Filepath { + // No pair for the file in full aggregation is available. + cov := fullCov.fileCoverageWithDetails + cov.Covered = 0 + res = append(res, &cov) + continue + } + if partCov.Filepath == fullCov.Filepath { + if partCov.Commit != fullCov.Commit || + !IsComparable( + fullCov.LinesInstrumented, fullCov.HitCounts, + partCov.LinesInstrumented, partCov.HitCounts) { + return fmt.Errorf("db record for file %s doesn't match", fullCov.Filepath) + } + resItem := fullCov.fileCoverageWithDetails // Use Instrumented count from full aggregation. + resItem.Covered = 0 + for _, hc := range UniqCoverage(fullCov.CovMap(), partCov.CovMap()) { + if hc > 0 { + resItem.Covered++ + } + } + res = append(res, &resItem) + partCov = <-partCh + continue } + // Partial coverage is a subset of full coverage. + // File can't exist only in partial set. + return fmt.Errorf("currupted db, file %s can't exist", partCov.Filepath) + } + return nil + }) + if err := eg.Wait(); err != nil { + return nil, fmt.Errorf("eg.Wait: %w", err) + } + return res, nil +} + +func MakeCovMap(keys, vals []int64) map[int]int64 { + res := map[int]int64{} + for i, key := range keys { + res[int(key)] = vals[i] + } + return res +} + +func IsComparable(fullLines, fullHitCounts, partialLines, partialHitCounts []int64) bool { + if len(fullLines) != len(fullHitCounts) || + len(partialLines) != len(partialHitCounts) || + len(fullLines) < len(partialLines) { + return false + } + fullCov := MakeCovMap(fullLines, fullHitCounts) + for iPartial, ln := range partialLines { + partialHitCount := partialHitCounts[iPartial] + if fullHitCount, fullExist := fullCov[int(ln)]; !fullExist || fullHitCount < partialHitCount { + return false + } + } + return true +} + +// Returns partial hitcounts that are the only source of the full hitcounts. +func UniqCoverage(fullCov, partCov map[int]int64) map[int]int64 { + res := maps.Clone(partCov) + for ln := range partCov { + if partCov[ln] != fullCov[ln] { + res[ln] = 0 + } + } + return res +} + +func readIterToChan[K fileCoverageWithLineInfo | fileCoverageWithDetails]( + ctx context.Context, iter spannerclient.RowIterator, ch chan<- *K) error { + for { + row, err := iter.Next() + if err == iterator.Done { + break + } + if err != nil { + return fmt.Errorf("iter.Next: %w", err) + } + var r K + if err = row.ToStruct(&r); err != nil { + return fmt.Errorf("row.ToStruct: %w", err) + } + select { + case ch <- &r: + case <-ctx.Done(): + return nil + } + } + return nil +} + +func filesCoverageWithDetails( + ctx context.Context, client spannerclient.SpannerClient, scope *SelectScope, onlyUnique bool, +) ([]*fileCoverageWithDetails, error) { + var res []*fileCoverageWithDetails + for _, timePeriod := range scope.Periods { + needLinesDetails := onlyUnique + iterManager := client.Single().Query(ctx, + filesCoverageWithDetailsStmt(scope.Ns, scope.Subsystem, scope.Manager, timePeriod, needLinesDetails)) + defer iterManager.Stop() + + var err error + var periodRes []*fileCoverageWithDetails + if onlyUnique { + iterAll := client.Single().Query(ctx, + filesCoverageWithDetailsStmt(scope.Ns, scope.Subsystem, "", timePeriod, needLinesDetails)) + defer iterAll.Stop() + periodRes, err = readCoverageUniq(iterAll, iterManager) if err != nil { - return nil, fmt.Errorf("failed to iter.Next() spanner DB: %w", err) + return nil, fmt.Errorf("uniqueFilesCoverageWithDetails: %w", err) } - var r fileCoverageWithDetails - if err = row.ToStruct(&r); err != nil { - return nil, fmt.Errorf("failed to row.ToStruct() spanner DB: %w", err) + } else { + periodRes, err = readCoverage(iterManager) + if err != nil { + return nil, fmt.Errorf("readCoverage: %w", err) } + } + for _, r := range periodRes { r.TimePeriod = timePeriod - res = append(res, &r) } + res = append(res, periodRes...) } return res, nil } @@ -252,9 +396,10 @@ type SelectScope struct { Periods []coveragedb.TimePeriod } -func DoHeatMapStyleBodyJS(ctx context.Context, projectID string, scope *SelectScope, sss, managers []string, +func DoHeatMapStyleBodyJS( + ctx context.Context, client spannerclient.SpannerClient, scope *SelectScope, onlyUnique bool, sss, managers []string, ) (template.CSS, template.HTML, template.HTML, error) { - covAndDates, err := filesCoverageWithDetails(ctx, projectID, scope) + covAndDates, err := filesCoverageWithDetails(ctx, client, scope, onlyUnique) if err != nil { return "", "", "", fmt.Errorf("failed to filesCoverageWithDetails: %w", err) } @@ -264,9 +409,10 @@ func DoHeatMapStyleBodyJS(ctx context.Context, projectID string, scope *SelectSc return stylesBodyJSTemplate(templData) } -func DoSubsystemsHeatMapStyleBodyJS(ctx context.Context, projectID string, scope *SelectScope, sss, managers []string, +func DoSubsystemsHeatMapStyleBodyJS( + ctx context.Context, client spannerclient.SpannerClient, scope *SelectScope, onlyUnique bool, sss, managers []string, ) (template.CSS, template.HTML, template.HTML, error) { - covWithDetails, err := filesCoverageWithDetails(ctx, projectID, scope) + covWithDetails, err := filesCoverageWithDetails(ctx, client, scope, onlyUnique) if err != nil { panic(err) } diff --git a/pkg/cover/heatmap_test.go b/pkg/cover/heatmap_test.go index d872e31d1f90..ad1f9bf645ff 100644 --- a/pkg/cover/heatmap_test.go +++ b/pkg/cover/heatmap_test.go @@ -4,14 +4,254 @@ package cover import ( + "context" "testing" "time" "cloud.google.com/go/civil" "github.com/google/syzkaller/pkg/coveragedb" + "github.com/google/syzkaller/pkg/coveragedb/mocks" + "github.com/google/syzkaller/pkg/coveragedb/spannerclient" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "google.golang.org/api/iterator" ) +func TestFilesCoverageWithDetails(t *testing.T) { + period, _ := coveragedb.MakeTimePeriod( + civil.Date{Year: 2025, Month: 1, Day: 1}, + "day") + tests := []struct { + name string + scope *SelectScope + client func() spannerclient.SpannerClient + onlyUnique bool + want []*fileCoverageWithDetails + wantErr bool + }{ + { + name: "empty scope", + scope: &SelectScope{}, + want: nil, + wantErr: false, + }, + { + name: "single day, no filters, empty DB => no coverage", + scope: &SelectScope{ + Ns: "upstream", + Periods: []coveragedb.TimePeriod{period}, + }, + client: func() spannerclient.SpannerClient { return emptyCoverageDBFixture(t, 1) }, + want: nil, + wantErr: false, + }, + { + name: "single day, unique coverage, empty DB => no coverage", + scope: &SelectScope{ + Ns: "upstream", + Periods: []coveragedb.TimePeriod{period}, + }, + client: func() spannerclient.SpannerClient { return emptyCoverageDBFixture(t, 2) }, + onlyUnique: true, + want: nil, + wantErr: false, + }, + { + name: "single day, unique coverage, empty partial result => 0/3 covered", + scope: &SelectScope{ + Ns: "upstream", + Periods: []coveragedb.TimePeriod{period}, + }, + client: func() spannerclient.SpannerClient { + return fullCoverageDBFixture( + t, + []*fileCoverageWithLineInfo{ + { + fileCoverageWithDetails: fileCoverageWithDetails{ + Filepath: "file1", + Instrumented: 3, + Covered: 3, + }, + LinesInstrumented: []int64{1, 2, 3}, + HitCounts: []int64{1, 1, 1}, + }, + }, + nil) + }, + onlyUnique: true, + want: []*fileCoverageWithDetails{ + { + Filepath: "file1", + Instrumented: 3, + Covered: 0, + TimePeriod: period, + }, + }, + wantErr: false, + }, + { + name: "single day, unique coverage, full result match => 3/3 covered", + scope: &SelectScope{ + Ns: "upstream", + Periods: []coveragedb.TimePeriod{period}, + }, + client: func() spannerclient.SpannerClient { + return fullCoverageDBFixture( + t, + []*fileCoverageWithLineInfo{ + { + fileCoverageWithDetails: fileCoverageWithDetails{ + Filepath: "file1", + Instrumented: 3, + Covered: 3, + }, + LinesInstrumented: []int64{1, 2, 3}, + HitCounts: []int64{1, 1, 1}, + }, + }, + []*fileCoverageWithLineInfo{ + { + fileCoverageWithDetails: fileCoverageWithDetails{ + Filepath: "file1", + Instrumented: 3, + Covered: 3, + }, + LinesInstrumented: []int64{1, 2, 3}, + HitCounts: []int64{1, 1, 1}, + }, + }) + }, + onlyUnique: true, + want: []*fileCoverageWithDetails{ + { + Filepath: "file1", + Instrumented: 3, + Covered: 3, + TimePeriod: period, + }, + }, + wantErr: false, + }, + { + name: "single day, unique coverage, partial result match => 3/5 covered", + scope: &SelectScope{ + Ns: "upstream", + Periods: []coveragedb.TimePeriod{period}, + }, + client: func() spannerclient.SpannerClient { + return fullCoverageDBFixture( + t, + []*fileCoverageWithLineInfo{ + { + fileCoverageWithDetails: fileCoverageWithDetails{ + Filepath: "file1", + Instrumented: 5, + Covered: 5, + }, + LinesInstrumented: []int64{1, 2, 3, 4, 5}, + HitCounts: []int64{3, 4, 5, 6, 7}, + }, + }, + []*fileCoverageWithLineInfo{ + { + fileCoverageWithDetails: fileCoverageWithDetails{ + Filepath: "file1", + Instrumented: 4, + Covered: 3, + }, + LinesInstrumented: []int64{1, 2, 3, 5}, + HitCounts: []int64{3, 0, 5, 7}, + }, + }) + }, + onlyUnique: true, + want: []*fileCoverageWithDetails{ + { + Filepath: "file1", + Instrumented: 5, + Covered: 3, + TimePeriod: period, + }, + }, + wantErr: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var testClient spannerclient.SpannerClient + if test.client != nil { + testClient = test.client() + } + got, gotErr := filesCoverageWithDetails( + context.Background(), + testClient, test.scope, test.onlyUnique) + if test.wantErr { + assert.Error(t, gotErr) + } else { + assert.NoError(t, gotErr) + } + assert.Equal(t, test.want, got) + }) + } +} + +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 fullCoverageDBFixture( + t *testing.T, full, partial []*fileCoverageWithLineInfo, +) spannerclient.SpannerClient { + mPartialTran := mocks.NewReadOnlyTransaction(t) + mPartialTran.On("Query", mock.Anything, mock.Anything). + Return(newRowIteratorMock(t, partial)).Once() + + mFullTran := mocks.NewReadOnlyTransaction(t) + mFullTran.On("Query", mock.Anything, mock.Anything). + Return(newRowIteratorMock(t, full)).Once() + + m := mocks.NewSpannerClient(t) + m.On("Single"). + Return(mPartialTran).Once() + m.On("Single"). + Return(mFullTran).Once() + return m +} + +func newRowIteratorMock(t *testing.T, events []*fileCoverageWithLineInfo, +) *mocks.RowIterator { + m := mocks.NewRowIterator(t) + m.On("Stop").Once().Return() + for _, item := range events { + mRow := mocks.NewRow(t) + mRow.On("ToStruct", mock.Anything). + Run(func(args mock.Arguments) { + arg := args.Get(0).(*fileCoverageWithLineInfo) + *arg = *item + }). + Return(nil).Once() + + m.On("Next"). + Return(mRow, nil).Once() + } + + m.On("Next"). + Return(nil, iterator.Done).Once() + return m +} + func TestFilesCoverageToTemplateData(t *testing.T) { tests := []struct { name string diff --git a/pkg/cover/templates/heatmap.html b/pkg/cover/templates/heatmap.html index c3fcd5db7268..cc5da9bcc37c 100644 --- a/pkg/cover/templates/heatmap.html +++ b/pkg/cover/templates/heatmap.html @@ -115,7 +115,7 @@

- +

- +

diff --git a/pkg/coveragedb/coveragedb.go b/pkg/coveragedb/coveragedb.go index 4488d1b6b725..22e84d8ebd23 100644 --- a/pkg/coveragedb/coveragedb.go +++ b/pkg/coveragedb/coveragedb.go @@ -57,10 +57,10 @@ type Coverage struct { HitCounts []int64 } -func (c *Coverage) AddLineHitCount(line, hitCount int) { +func (c *Coverage) AddLineHitCount(line int, hitCount int64) { c.Instrumented++ c.LinesInstrumented = append(c.LinesInstrumented, int64(line)) - c.HitCounts = append(c.HitCounts, int64(hitCount)) + c.HitCounts = append(c.HitCounts, hitCount) if hitCount > 0 { c.Covered++ } @@ -122,7 +122,10 @@ type linesCoverage struct { HitCounts []int64 } -func linesCoverageStmt(ns, filepath, commit string, timePeriod TimePeriod) spanner.Statement { +func linesCoverageStmt(ns, filepath, commit, manager string, timePeriod TimePeriod) spanner.Statement { + if manager == "" { + manager = "*" + } return spanner.Statement{ SQL: ` select @@ -132,47 +135,43 @@ from merge_history join files on merge_history.session = files.session where - namespace=$1 and dateto=$2 and duration=$3 and filepath=$4 and commit=$5 and manager='*'`, + namespace=$1 and dateto=$2 and duration=$3 and filepath=$4 and commit=$5 and manager=$6`, Params: map[string]interface{}{ "p1": ns, "p2": timePeriod.DateTo, "p3": timePeriod.Days, "p4": filepath, "p5": commit, + "p6": manager, }, } } -func ReadLinesHitCount(ctx context.Context, ns, commit, file string, tp TimePeriod, -) (map[int]int, error) { +func ReadLinesHitCount(ctx context.Context, ns, commit, file, manager string, tp TimePeriod, +) ([]int64, []int64, error) { projectID := os.Getenv("GOOGLE_CLOUD_PROJECT") client, err := spannerclient.NewClient(ctx, projectID) if err != nil { - return nil, fmt.Errorf("spanner.NewClient: %w", err) + return nil, nil, fmt.Errorf("spanner.NewClient: %w", err) } defer client.Close() - stmt := linesCoverageStmt(ns, file, commit, tp) + stmt := linesCoverageStmt(ns, file, commit, manager, tp) iter := client.Single().Query(ctx, stmt) defer iter.Stop() row, err := iter.Next() if err == iterator.Done { - return nil, nil + return nil, nil, nil } if err != nil { - return nil, fmt.Errorf("iter.Next: %w", err) + return nil, nil, fmt.Errorf("iter.Next: %w", err) } var r linesCoverage if err = row.ToStruct(&r); err != nil { - return nil, fmt.Errorf("failed to row.ToStruct() spanner DB: %w", err) - } - - res := map[int]int{} - for i, instrLine := range r.LinesInstrumented { - res[int(instrLine)] = int(r.HitCounts[i]) + return nil, nil, fmt.Errorf("failed to row.ToStruct() spanner DB: %w", err) } - return res, nil + return r.LinesInstrumented, r.HitCounts, nil } func historyMutation(session string, template *HistoryRecord) *spanner.Mutation { diff --git a/pkg/coveragedb/coveragedb_mock_test.go b/pkg/coveragedb/coveragedb_mock_test.go index 8f20a43edee2..b0ffb7815967 100644 --- a/pkg/coveragedb/coveragedb_mock_test.go +++ b/pkg/coveragedb/coveragedb_mock_test.go @@ -19,6 +19,9 @@ import ( ) //go:generate ../../tools/mockery.sh --name SpannerClient -r +//go:generate ../../tools/mockery.sh --name ReadOnlyTransaction -r +//go:generate ../../tools/mockery.sh --name RowIterator -r +//go:generate ../../tools/mockery.sh --name Row -r type spannerMockTune func(*testing.T, *mocks.SpannerClient) diff --git a/pkg/coveragedb/mocks/ReadOnlyTransaction.go b/pkg/coveragedb/mocks/ReadOnlyTransaction.go new file mode 100644 index 000000000000..79a75cb721f4 --- /dev/null +++ b/pkg/coveragedb/mocks/ReadOnlyTransaction.go @@ -0,0 +1,51 @@ +// Code generated by mockery v2.45.1. DO NOT EDIT. + +package mocks + +import ( + context "context" + + spanner "cloud.google.com/go/spanner" + mock "github.com/stretchr/testify/mock" + + spannerclient "github.com/google/syzkaller/pkg/coveragedb/spannerclient" +) + +// ReadOnlyTransaction is an autogenerated mock type for the ReadOnlyTransaction type +type ReadOnlyTransaction struct { + mock.Mock +} + +// Query provides a mock function with given fields: ctx, statement +func (_m *ReadOnlyTransaction) Query(ctx context.Context, statement spanner.Statement) spannerclient.RowIterator { + ret := _m.Called(ctx, statement) + + if len(ret) == 0 { + panic("no return value specified for Query") + } + + var r0 spannerclient.RowIterator + if rf, ok := ret.Get(0).(func(context.Context, spanner.Statement) spannerclient.RowIterator); ok { + r0 = rf(ctx, statement) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(spannerclient.RowIterator) + } + } + + return r0 +} + +// NewReadOnlyTransaction creates a new instance of ReadOnlyTransaction. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewReadOnlyTransaction(t interface { + mock.TestingT + Cleanup(func()) +}) *ReadOnlyTransaction { + mock := &ReadOnlyTransaction{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/coveragedb/mocks/Row.go b/pkg/coveragedb/mocks/Row.go new file mode 100644 index 000000000000..a8ac4ce789a9 --- /dev/null +++ b/pkg/coveragedb/mocks/Row.go @@ -0,0 +1,42 @@ +// Code generated by mockery v2.45.1. DO NOT EDIT. + +package mocks + +import mock "github.com/stretchr/testify/mock" + +// Row is an autogenerated mock type for the Row type +type Row struct { + mock.Mock +} + +// ToStruct provides a mock function with given fields: p +func (_m *Row) ToStruct(p interface{}) error { + ret := _m.Called(p) + + if len(ret) == 0 { + panic("no return value specified for ToStruct") + } + + var r0 error + if rf, ok := ret.Get(0).(func(interface{}) error); ok { + r0 = rf(p) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// NewRow creates a new instance of Row. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewRow(t interface { + mock.TestingT + Cleanup(func()) +}) *Row { + mock := &Row{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/coveragedb/mocks/RowIterator.go b/pkg/coveragedb/mocks/RowIterator.go new file mode 100644 index 000000000000..865240d3ba5d --- /dev/null +++ b/pkg/coveragedb/mocks/RowIterator.go @@ -0,0 +1,62 @@ +// Code generated by mockery v2.45.1. DO NOT EDIT. + +package mocks + +import ( + spannerclient "github.com/google/syzkaller/pkg/coveragedb/spannerclient" + mock "github.com/stretchr/testify/mock" +) + +// RowIterator is an autogenerated mock type for the RowIterator type +type RowIterator struct { + mock.Mock +} + +// Next provides a mock function with given fields: +func (_m *RowIterator) Next() (spannerclient.Row, error) { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for Next") + } + + var r0 spannerclient.Row + var r1 error + if rf, ok := ret.Get(0).(func() (spannerclient.Row, error)); ok { + return rf() + } + if rf, ok := ret.Get(0).(func() spannerclient.Row); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(spannerclient.Row) + } + } + + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// Stop provides a mock function with given fields: +func (_m *RowIterator) Stop() { + _m.Called() +} + +// NewRowIterator creates a new instance of RowIterator. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewRowIterator(t interface { + mock.TestingT + Cleanup(func()) +}) *RowIterator { + mock := &RowIterator{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/covermerger/covermerger.go b/pkg/covermerger/covermerger.go index 11ed5c0431b3..34b53459373f 100644 --- a/pkg/covermerger/covermerger.go +++ b/pkg/covermerger/covermerger.go @@ -44,7 +44,7 @@ type RepoCommit struct { } type MergeResult struct { - HitCounts map[int]int + HitCounts map[int]int64 FileExists bool LineDetails map[int][]*FileRecord } @@ -118,10 +118,10 @@ func mergedCoverageRecords(fmr *FileMergeResult) []*coveragedb.MergedCoverageRec for _, line := range lines { mgrStat[allManagers].AddLineHitCount(line, fmr.HitCounts[line]) - managerHitCounts := map[string]int{} + managerHitCounts := map[string]int64{} for _, lineDetail := range fmr.LineDetails[line] { manager := lineDetail.Manager - managerHitCounts[manager] += lineDetail.HitCount + managerHitCounts[manager] += int64(lineDetail.HitCount) } for manager, managerHitCount := range managerHitCounts { if _, ok := mgrStat[manager]; !ok { diff --git a/pkg/covermerger/file_line_merger.go b/pkg/covermerger/file_line_merger.go index 5d9c0ab7a0d9..ebc747f47abc 100644 --- a/pkg/covermerger/file_line_merger.go +++ b/pkg/covermerger/file_line_merger.go @@ -20,7 +20,7 @@ func makeFileLineCoverMerger(fvs fileVersions, base RepoCommit) FileCoverageMerg } a := &FileLineCoverMerger{ MergeResult: &MergeResult{ - HitCounts: make(map[int]int), + HitCounts: make(map[int]int64), FileExists: true, LineDetails: make(map[int][]*FileRecord), }, @@ -49,7 +49,7 @@ func (a *FileLineCoverMerger) Add(record *FileRecord) { return } if targetLine := a.matchers[record.RepoCommit].SameLinePos(record.StartLine); targetLine != -1 { - a.HitCounts[targetLine] += record.HitCount + a.HitCounts[targetLine] += int64(record.HitCount) a.LineDetails[targetLine] = append(a.LineDetails[targetLine], record) } } diff --git a/pkg/validator/validator.go b/pkg/validator/validator.go index 9aebc8150506..b9031302aa7d 100644 --- a/pkg/validator/validator.go +++ b/pkg/validator/validator.go @@ -48,12 +48,33 @@ func PanicIfNot(results ...Result) error { return nil } +var ErrValueNotAllowed = errors.New("value is not allowed") + +func Allowlisted(str string, allowlist []string, valueName ...string) Result { + for _, allowed := range allowlist { + if allowed == str { + return Result{ + Ok: true, + } + } + } + if len(valueName) == 0 { + return Result{ + Err: fmt.Errorf("value %s is not allowed", str), + } + } + return Result{ + Err: fmt.Errorf("%s(%s) is not allowed", valueName[0], str), + } +} + var ( EmptyStr = makeStrLenFunc("not empty", 0) AlphaNumeric = makeStrReFunc("not an alphanum", "^[a-zA-Z0-9]*$") CommitHash = makeCombinedStrFunc("not a hash", AlphaNumeric, makeStrLenFunc("len is not 40", 40)) KernelFilePath = makeStrReFunc("not a kernel file path", "^[./_a-zA-Z0-9-]*$") NamespaceName = makeStrReFunc("not a namespace name", "^[a-zA-Z0-9_.-]{4,32}$") + ManagerName = makeStrReFunc("not a manager name", "^ci[a-z0-9-]*$") DashClientName = makeStrReFunc("not a dashboard client name", "^[a-zA-Z0-9_.-]{4,100}$") DashClientKey = makeStrReFunc("not a dashboard client key", "^([a-zA-Z0-9]{16,128})|("+regexp.QuoteMeta(auth.OauthMagic)+".*)$") diff --git a/pkg/validator/validator_test.go b/pkg/validator/validator_test.go index 9aa08e2d0224..ffe9356005fb 100644 --- a/pkg/validator/validator_test.go +++ b/pkg/validator/validator_test.go @@ -35,6 +35,16 @@ func TestIsNamespaceName(t *testing.T) { validator.NamespaceName("up", "ns").Err.Error()) } +// nolint: dupl +func TestIsManagerName(t *testing.T) { + assert.True(t, validator.ManagerName("ci-upstream").Ok) + assert.False(t, validator.ManagerName("").Ok) + + assert.Equal(t, "not a manager name", validator.ManagerName("*").Err.Error()) + assert.Equal(t, "manager: not a manager name", + validator.ManagerName("*", "manager").Err.Error()) +} + // nolint: dupl func TestIsDashboardClientName(t *testing.T) { assert.True(t, validator.DashClientName("name").Ok) @@ -86,3 +96,11 @@ func TestAnyOk(t *testing.T) { assert.Equal(t, badResult, validator.AnyOk(badResult)) assert.Equal(t, validator.ResultOk, validator.AnyOk(badResult, validator.ResultOk)) } + +func TestAllowlisted(t *testing.T) { + assert.True(t, validator.Allowlisted("good", []string{"good", "also-good"}).Ok) + assert.False(t, validator.Allowlisted("bad", []string{"good", "also-good"}).Ok) + assert.Equal(t, + validator.Result{Ok: false, Err: errors.New("name(bad) is not allowed")}, + validator.Allowlisted("bad", nil, "name")) +} From 83cb39b4e7850fed218e1de6f3e95f60f2a28902 Mon Sep 17 00:00:00 2001 From: Taras Madan Date: Thu, 23 Jan 2025 21:54:41 +0100 Subject: [PATCH 2/3] 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. --- Makefile | 1 + dashboard/app/api.go | 9 +- dashboard/app/batch_coverage.go | 4 +- dashboard/app/config.go | 1 + dashboard/app/coverage.go | 77 ++++++++-- dashboard/app/coverage_test.go | 171 ++++++++++++++++++++++ dashboard/app/entities_spanner.go | 11 +- dashboard/app/handler.go | 3 + dashboard/app/main.go | 12 +- dashboard/app/util_test.go | 16 ++ pkg/cover/file.go | 4 +- pkg/cover/heatmap.go | 10 +- pkg/cover/heatmap_test.go | 16 +- pkg/coveragedb/coveragedb.go | 41 +++--- pkg/covermerger/covermerger_test.go | 6 +- pkg/covermerger/file_line_merger.go | 2 +- pkg/covermerger/mocks/FileVersProvider.go | 64 ++++++++ pkg/covermerger/provider_monorepo.go | 10 +- pkg/covermerger/provider_web.go | 4 +- pkg/validator/validator.go | 2 +- tools/syz-cover/syz-cover.go | 3 +- 21 files changed, 381 insertions(+), 86 deletions(-) create mode 100644 dashboard/app/coverage_test.go create mode 100644 pkg/covermerger/mocks/FileVersProvider.go diff --git a/Makefile b/Makefile index 07716b436795..b57e51d0efcf 100644 --- a/Makefile +++ b/Makefile @@ -236,6 +236,7 @@ generate_go: format_cpp $(GO) generate ./executor ./pkg/ifuzz ./pkg/build ./pkg/rpcserver $(GO) generate ./vm/proxyapp $(GO) generate ./pkg/coveragedb + $(GO) generate ./pkg/covermerger generate_rpc: flatc -o pkg/flatrpc --warnings-as-errors --gen-object-api --filename-suffix "" --go --gen-onefile --go-namespace flatrpc pkg/flatrpc/flatrpc.fbs diff --git a/dashboard/app/api.go b/dashboard/app/api.go index b06c5ac07833..e1269b3d087d 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 6d53496a7087..aba85cf596da 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 6d173cb19550..c71d09d8e88e 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 7bc0d2a06660..c91c7d66564b 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 000000000000..ef8321c9f85e --- /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 e3f3bad9e23d..c550ef92e217 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 e737c2b0b4ce..27817ab8c82e 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 d765c8df96a8..071dc854438f 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 d2ed77e32a67..be7841b27fb0 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 { diff --git a/pkg/cover/file.go b/pkg/cover/file.go index c56f882045c6..ea6c326f0a05 100644 --- a/pkg/cover/file.go +++ b/pkg/cover/file.go @@ -40,10 +40,10 @@ func DefaultHTMLRenderConfig() *CoverageRenderConfig { } } -func RendFileCoverage(repo, forCommit, filePath string, proxy covermerger.FuncProxyURI, +func RendFileCoverage(repo, forCommit, filePath string, fileProvider covermerger.FileVersProvider, mr *covermerger.MergeResult, renderConfig *CoverageRenderConfig) (string, error) { repoCommit := covermerger.RepoCommit{Repo: repo, Commit: forCommit} - files, err := covermerger.MakeWebGit(proxy).GetFileVersions(filePath, repoCommit) + files, err := fileProvider.GetFileVersions(filePath, repoCommit) if err != nil { return "", fmt.Errorf("failed to GetFileVersions: %w", err) } diff --git a/pkg/cover/heatmap.go b/pkg/cover/heatmap.go index 04b7e6914b77..4cc212fadac6 100644 --- a/pkg/cover/heatmap.go +++ b/pkg/cover/heatmap.go @@ -116,13 +116,13 @@ type fileCoverageWithDetails struct { Subsystems []string } -type fileCoverageWithLineInfo struct { +type FileCoverageWithLineInfo struct { fileCoverageWithDetails LinesInstrumented []int64 HitCounts []int64 } -func (fc *fileCoverageWithLineInfo) CovMap() map[int]int64 { +func (fc *FileCoverageWithLineInfo) CovMap() map[int]int64 { return MakeCovMap(fc.LinesInstrumented, fc.HitCounts) } @@ -223,12 +223,12 @@ func readCoverage(iterManager spannerclient.RowIterator) ([]*fileCoverageWithDet func readCoverageUniq(full, mgr spannerclient.RowIterator, ) ([]*fileCoverageWithDetails, error) { eg, ctx := errgroup.WithContext(context.Background()) - fullCh := make(chan *fileCoverageWithLineInfo) + fullCh := make(chan *FileCoverageWithLineInfo) eg.Go(func() error { defer close(fullCh) return readIterToChan(ctx, full, fullCh) }) - partCh := make(chan *fileCoverageWithLineInfo) + partCh := make(chan *FileCoverageWithLineInfo) eg.Go(func() error { defer close(partCh) return readIterToChan(ctx, mgr, partCh) @@ -309,7 +309,7 @@ func UniqCoverage(fullCov, partCov map[int]int64) map[int]int64 { return res } -func readIterToChan[K fileCoverageWithLineInfo | fileCoverageWithDetails]( +func readIterToChan[K FileCoverageWithLineInfo | fileCoverageWithDetails]( ctx context.Context, iter spannerclient.RowIterator, ch chan<- *K) error { for { row, err := iter.Next() diff --git a/pkg/cover/heatmap_test.go b/pkg/cover/heatmap_test.go index ad1f9bf645ff..0f66256e4e8d 100644 --- a/pkg/cover/heatmap_test.go +++ b/pkg/cover/heatmap_test.go @@ -65,7 +65,7 @@ func TestFilesCoverageWithDetails(t *testing.T) { client: func() spannerclient.SpannerClient { return fullCoverageDBFixture( t, - []*fileCoverageWithLineInfo{ + []*FileCoverageWithLineInfo{ { fileCoverageWithDetails: fileCoverageWithDetails{ Filepath: "file1", @@ -98,7 +98,7 @@ func TestFilesCoverageWithDetails(t *testing.T) { client: func() spannerclient.SpannerClient { return fullCoverageDBFixture( t, - []*fileCoverageWithLineInfo{ + []*FileCoverageWithLineInfo{ { fileCoverageWithDetails: fileCoverageWithDetails{ Filepath: "file1", @@ -109,7 +109,7 @@ func TestFilesCoverageWithDetails(t *testing.T) { HitCounts: []int64{1, 1, 1}, }, }, - []*fileCoverageWithLineInfo{ + []*FileCoverageWithLineInfo{ { fileCoverageWithDetails: fileCoverageWithDetails{ Filepath: "file1", @@ -141,7 +141,7 @@ func TestFilesCoverageWithDetails(t *testing.T) { client: func() spannerclient.SpannerClient { return fullCoverageDBFixture( t, - []*fileCoverageWithLineInfo{ + []*FileCoverageWithLineInfo{ { fileCoverageWithDetails: fileCoverageWithDetails{ Filepath: "file1", @@ -152,7 +152,7 @@ func TestFilesCoverageWithDetails(t *testing.T) { HitCounts: []int64{3, 4, 5, 6, 7}, }, }, - []*fileCoverageWithLineInfo{ + []*FileCoverageWithLineInfo{ { fileCoverageWithDetails: fileCoverageWithDetails{ Filepath: "file1", @@ -212,7 +212,7 @@ func emptyCoverageDBFixture(t *testing.T, times int) spannerclient.SpannerClient } func fullCoverageDBFixture( - t *testing.T, full, partial []*fileCoverageWithLineInfo, + t *testing.T, full, partial []*FileCoverageWithLineInfo, ) spannerclient.SpannerClient { mPartialTran := mocks.NewReadOnlyTransaction(t) mPartialTran.On("Query", mock.Anything, mock.Anything). @@ -230,7 +230,7 @@ func fullCoverageDBFixture( return m } -func newRowIteratorMock(t *testing.T, events []*fileCoverageWithLineInfo, +func newRowIteratorMock(t *testing.T, events []*FileCoverageWithLineInfo, ) *mocks.RowIterator { m := mocks.NewRowIterator(t) m.On("Stop").Once().Return() @@ -238,7 +238,7 @@ func newRowIteratorMock(t *testing.T, events []*fileCoverageWithLineInfo, mRow := mocks.NewRow(t) mRow.On("ToStruct", mock.Anything). Run(func(args mock.Arguments) { - arg := args.Get(0).(*fileCoverageWithLineInfo) + arg := args.Get(0).(*FileCoverageWithLineInfo) *arg = *item }). Return(nil).Once() diff --git a/pkg/coveragedb/coveragedb.go b/pkg/coveragedb/coveragedb.go index 22e84d8ebd23..0692bc131eb5 100644 --- a/pkg/coveragedb/coveragedb.go +++ b/pkg/coveragedb/coveragedb.go @@ -9,7 +9,6 @@ import ( "errors" "fmt" "io" - "os" "sync/atomic" "time" @@ -74,6 +73,9 @@ type MergedCoverageRecord struct { func SaveMergeResult(ctx context.Context, client spannerclient.SpannerClient, descr *HistoryRecord, dec *json.Decoder, sss []*subsystem.Subsystem) (int, error) { + if client == nil { + return 0, fmt.Errorf("nil spannerclient") + } var rowsCreated int ssMatcher := subsystem.MakePathMatcher(sss) ssCache := make(map[string][]string) @@ -117,7 +119,7 @@ func SaveMergeResult(ctx context.Context, client spannerclient.SpannerClient, de return rowsCreated, nil } -type linesCoverage struct { +type LinesCoverage struct { LinesInstrumented []int64 HitCounts []int64 } @@ -147,15 +149,9 @@ where } } -func ReadLinesHitCount(ctx context.Context, ns, commit, file, manager string, tp TimePeriod, +func ReadLinesHitCount(ctx context.Context, client spannerclient.SpannerClient, + ns, commit, file, manager string, tp TimePeriod, ) ([]int64, []int64, error) { - projectID := os.Getenv("GOOGLE_CLOUD_PROJECT") - client, err := spannerclient.NewClient(ctx, projectID) - if err != nil { - return nil, nil, fmt.Errorf("spanner.NewClient: %w", err) - } - defer client.Close() - stmt := linesCoverageStmt(ns, file, commit, manager, tp) iter := client.Single().Query(ctx, stmt) defer iter.Stop() @@ -167,10 +163,13 @@ func ReadLinesHitCount(ctx context.Context, ns, commit, file, manager string, tp if err != nil { return nil, nil, fmt.Errorf("iter.Next: %w", err) } - var r linesCoverage + var r LinesCoverage if err = row.ToStruct(&r); err != nil { return nil, nil, fmt.Errorf("failed to row.ToStruct() spanner DB: %w", err) } + if _, err := iter.Next(); err != iterator.Done { + return nil, nil, fmt.Errorf("more than 1 line is available") + } return r.LinesInstrumented, r.HitCounts, nil } @@ -230,13 +229,11 @@ func fileSubsystems(filePath string, ssMatcher *subsystem.PathMatcher, ssCache m return sss } -func NsDataMerged(ctx context.Context, projectID, ns string) ([]TimePeriod, []int64, error) { - client, err := spannerclient.NewClient(ctx, projectID) - if err != nil { - return nil, nil, fmt.Errorf("spanner.NewClient() failed: %s", err.Error()) +func NsDataMerged(ctx context.Context, client spannerclient.SpannerClient, ns string, +) ([]TimePeriod, []int64, error) { + if client == nil { + return nil, nil, fmt.Errorf("nil spannerclient") } - defer client.Close() - stmt := spanner.Statement{ SQL: ` select @@ -285,13 +282,11 @@ func NsDataMerged(ctx context.Context, projectID, ns string) ([]TimePeriod, []in // Note that in case of an error during batch deletion, some files may be deleted but not counted in the total. // // Returns the number of orphaned file entries successfully deleted. -func DeleteGarbage(ctx context.Context) (int64, error) { +func DeleteGarbage(ctx context.Context, client spannerclient.SpannerClient) (int64, error) { batchSize := 10_000 - client, err := spannerclient.NewClient(ctx, os.Getenv("GOOGLE_CLOUD_PROJECT")) - if err != nil { - return 0, fmt.Errorf("coveragedb.NewClient: %w", err) + if client == nil { + return 0, fmt.Errorf("nil spannerclient") } - defer client.Close() iter := client.Single().Query(ctx, spanner.Statement{ SQL: `SELECT session, filepath @@ -328,7 +323,7 @@ func DeleteGarbage(ctx context.Context) (int64, error) { } } goSpannerDelete(ctx, batch, eg, client, &totalDeleted) - if err = eg.Wait(); err != nil { + if err := eg.Wait(); err != nil { return 0, fmt.Errorf("spanner.Delete: %w", err) } return totalDeleted.Load(), nil diff --git a/pkg/covermerger/covermerger_test.go b/pkg/covermerger/covermerger_test.go index 36f42fdc1bd2..ab2b1efc5b1d 100644 --- a/pkg/covermerger/covermerger_test.go +++ b/pkg/covermerger/covermerger_test.go @@ -95,7 +95,7 @@ func TestMergerdCoverageRecords(t *testing.T) { FilePath: "file.c", MergeResult: &MergeResult{ FileExists: true, - HitCounts: map[int]int{ + HitCounts: map[int]int64{ 1: 5, 2: 7, }, @@ -367,8 +367,8 @@ type fileVersProviderMock struct { } func (m *fileVersProviderMock) GetFileVersions(targetFilePath string, repoCommits ...RepoCommit, -) (fileVersions, error) { - res := make(fileVersions) +) (FileVersions, error) { + res := make(FileVersions) for _, repoCommit := range repoCommits { filePath := filepath.Join(m.Workdir, "repos", repoCommit.Commit, targetFilePath) if bytes, err := os.ReadFile(filePath); err == nil { diff --git a/pkg/covermerger/file_line_merger.go b/pkg/covermerger/file_line_merger.go index ebc747f47abc..817099a60786 100644 --- a/pkg/covermerger/file_line_merger.go +++ b/pkg/covermerger/file_line_merger.go @@ -5,7 +5,7 @@ package covermerger import "github.com/google/syzkaller/pkg/log" -func makeFileLineCoverMerger(fvs fileVersions, base RepoCommit) FileCoverageMerger { +func makeFileLineCoverMerger(fvs FileVersions, base RepoCommit) FileCoverageMerger { baseFile := "" baseFileExists := false for repoCommit, fv := range fvs { diff --git a/pkg/covermerger/mocks/FileVersProvider.go b/pkg/covermerger/mocks/FileVersProvider.go new file mode 100644 index 000000000000..69fadfa425c7 --- /dev/null +++ b/pkg/covermerger/mocks/FileVersProvider.go @@ -0,0 +1,64 @@ +// Code generated by mockery v2.45.1. DO NOT EDIT. + +package mocks + +import ( + covermerger "github.com/google/syzkaller/pkg/covermerger" + mock "github.com/stretchr/testify/mock" +) + +// FileVersProvider is an autogenerated mock type for the FileVersProvider type +type FileVersProvider struct { + mock.Mock +} + +// GetFileVersions provides a mock function with given fields: targetFilePath, repoCommits +func (_m *FileVersProvider) GetFileVersions(targetFilePath string, repoCommits ...covermerger.RepoCommit) (covermerger.FileVersions, error) { + _va := make([]interface{}, len(repoCommits)) + for _i := range repoCommits { + _va[_i] = repoCommits[_i] + } + var _ca []interface{} + _ca = append(_ca, targetFilePath) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + if len(ret) == 0 { + panic("no return value specified for GetFileVersions") + } + + var r0 covermerger.FileVersions + var r1 error + if rf, ok := ret.Get(0).(func(string, ...covermerger.RepoCommit) (covermerger.FileVersions, error)); ok { + return rf(targetFilePath, repoCommits...) + } + if rf, ok := ret.Get(0).(func(string, ...covermerger.RepoCommit) covermerger.FileVersions); ok { + r0 = rf(targetFilePath, repoCommits...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(covermerger.FileVersions) + } + } + + if rf, ok := ret.Get(1).(func(string, ...covermerger.RepoCommit) error); ok { + r1 = rf(targetFilePath, repoCommits...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewFileVersProvider creates a new instance of FileVersProvider. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewFileVersProvider(t interface { + mock.TestingT + Cleanup(func()) +}) *FileVersProvider { + mock := &FileVersProvider{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/pkg/covermerger/provider_monorepo.go b/pkg/covermerger/provider_monorepo.go index e6c67d1d1901..73a08786a1c6 100644 --- a/pkg/covermerger/provider_monorepo.go +++ b/pkg/covermerger/provider_monorepo.go @@ -3,6 +3,8 @@ package covermerger +//go:generate ../../tools/mockery.sh --name FileVersProvider -r + import ( "fmt" "path/filepath" @@ -15,7 +17,7 @@ import ( type FileVersProvider interface { GetFileVersions(targetFilePath string, repoCommits ...RepoCommit, - ) (fileVersions, error) + ) (FileVersions, error) } type monoRepo struct { @@ -24,10 +26,10 @@ type monoRepo struct { repo vcs.Repo } -type fileVersions map[RepoCommit]string +type FileVersions map[RepoCommit]string func (mr *monoRepo) GetFileVersions(targetFilePath string, repoCommits ...RepoCommit, -) (fileVersions, error) { +) (FileVersions, error) { mr.mu.RLock() if !mr.allRepoCommitsPresent(repoCommits) { mr.mu.RUnlock() @@ -35,7 +37,7 @@ func (mr *monoRepo) GetFileVersions(targetFilePath string, repoCommits ...RepoCo mr.mu.RLock() } defer mr.mu.RUnlock() - res := make(fileVersions) + res := make(FileVersions) for _, repoCommit := range repoCommits { fileBytes, err := mr.repo.Object(targetFilePath, repoCommit.Commit) // It is ok if some file doesn't exist. It means we have repo FS diff. diff --git a/pkg/covermerger/provider_web.go b/pkg/covermerger/provider_web.go index 43bfee7e0832..554ee3f97fa8 100644 --- a/pkg/covermerger/provider_web.go +++ b/pkg/covermerger/provider_web.go @@ -18,8 +18,8 @@ type webGit struct { } func (mr *webGit) GetFileVersions(targetFilePath string, repoCommits ...RepoCommit, -) (fileVersions, error) { - res := make(fileVersions) +) (FileVersions, error) { + res := make(FileVersions) for _, repoCommit := range repoCommits { fileBytes, err := mr.loadFile(targetFilePath, repoCommit.Repo, repoCommit.Commit) // It is ok if some file doesn't exist. It means we have repo FS diff. diff --git a/pkg/validator/validator.go b/pkg/validator/validator.go index b9031302aa7d..12224963ec59 100644 --- a/pkg/validator/validator.go +++ b/pkg/validator/validator.go @@ -74,7 +74,7 @@ var ( CommitHash = makeCombinedStrFunc("not a hash", AlphaNumeric, makeStrLenFunc("len is not 40", 40)) KernelFilePath = makeStrReFunc("not a kernel file path", "^[./_a-zA-Z0-9-]*$") NamespaceName = makeStrReFunc("not a namespace name", "^[a-zA-Z0-9_.-]{4,32}$") - ManagerName = makeStrReFunc("not a manager name", "^ci[a-z0-9-]*$") + ManagerName = makeStrReFunc("not a manager name", "^[a-z0-9-]*$") DashClientName = makeStrReFunc("not a dashboard client name", "^[a-zA-Z0-9_.-]{4,100}$") DashClientKey = makeStrReFunc("not a dashboard client key", "^([a-zA-Z0-9]{16,128})|("+regexp.QuoteMeta(auth.OauthMagic)+".*)$") diff --git a/tools/syz-cover/syz-cover.go b/tools/syz-cover/syz-cover.go index bbdb4bd96be4..4a72b9bf6479 100644 --- a/tools/syz-cover/syz-cover.go +++ b/tools/syz-cover/syz-cover.go @@ -38,6 +38,7 @@ import ( "github.com/google/syzkaller/pkg/cover" "github.com/google/syzkaller/pkg/cover/backend" "github.com/google/syzkaller/pkg/coveragedb" + "github.com/google/syzkaller/pkg/covermerger" "github.com/google/syzkaller/pkg/log" "github.com/google/syzkaller/pkg/mgrconfig" "github.com/google/syzkaller/pkg/osutil" @@ -92,7 +93,7 @@ func toolFileCover() { *flagRepo, *flagCommit, *flagForFile, - nil, // no proxy - get files directly from WebGits + covermerger.MakeWebGit(nil), // get files directly from WebGits mr, config, ) From 7ee1f673827901bd3f398b567108d6adbf9fa3f6 Mon Sep 17 00:00:00 2001 From: Taras Madan Date: Fri, 24 Jan 2025 18:12:48 +0100 Subject: [PATCH 3/3] dashboard/app: looking for the unique coverage, hide record with zero hitcount --- pkg/cover/heatmap.go | 25 ++++++++++++++++++------- pkg/cover/heatmap_test.go | 35 +++++++++++++++++++++++++++-------- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/pkg/cover/heatmap.go b/pkg/cover/heatmap.go index 4cc212fadac6..ce98113cfde5 100644 --- a/pkg/cover/heatmap.go +++ b/pkg/cover/heatmap.go @@ -71,8 +71,19 @@ func (thm *templateHeatmapRow) addParts(depth int, pathLeft []string, filePath s thm.builder[nextElement].addParts(depth+1, pathLeft[1:], filePath, instrumented, covered, timePeriod) } -func (thm *templateHeatmapRow) prepareDataFor(pageColumns []pageColumnTarget) { - thm.Items = maps.Values(thm.builder) +func (thm *templateHeatmapRow) prepareDataFor(pageColumns []pageColumnTarget, skipEmpty bool) { + for _, item := range thm.builder { + if !skipEmpty { + thm.Items = append(thm.Items, item) + continue + } + for _, hitCount := range item.covered { + if hitCount > 0 { + thm.Items = append(thm.Items, item) + break + } + } + } sort.Slice(thm.Items, func(i, j int) bool { if thm.Items[i].IsDir != thm.Items[j].IsDir { return thm.Items[i].IsDir @@ -102,7 +113,7 @@ func (thm *templateHeatmapRow) prepareDataFor(pageColumns []pageColumnTarget) { thm.LastDayInstrumented = thm.instrumented[lastDate] } for _, item := range thm.builder { - item.prepareDataFor(pageColumns) + item.prepareDataFor(pageColumns, skipEmpty) } } @@ -131,7 +142,7 @@ type pageColumnTarget struct { Commit string } -func filesCoverageToTemplateData(fCov []*fileCoverageWithDetails) *templateHeatmap { +func filesCoverageToTemplateData(fCov []*fileCoverageWithDetails, hideEmpty bool) *templateHeatmap { res := templateHeatmap{ Root: &templateHeatmapRow{ IsDir: true, @@ -164,7 +175,7 @@ func filesCoverageToTemplateData(fCov []*fileCoverageWithDetails) *templateHeatm res.Periods = append(res.Periods, fmt.Sprintf("%s(%d)", tp.DateTo.String(), tp.Days)) } - res.Root.prepareDataFor(targetDateAndCommits) + res.Root.prepareDataFor(targetDateAndCommits, hideEmpty) return &res } @@ -403,7 +414,7 @@ func DoHeatMapStyleBodyJS( if err != nil { return "", "", "", fmt.Errorf("failed to filesCoverageWithDetails: %w", err) } - templData := filesCoverageToTemplateData(covAndDates) + templData := filesCoverageToTemplateData(covAndDates, onlyUnique) templData.Subsystems = sss templData.Managers = managers return stylesBodyJSTemplate(templData) @@ -430,7 +441,7 @@ func DoSubsystemsHeatMapStyleBodyJS( ssCovAndDates = append(ssCovAndDates, &newRecord) } } - templData := filesCoverageToTemplateData(ssCovAndDates) + templData := filesCoverageToTemplateData(ssCovAndDates, onlyUnique) templData.Managers = managers return stylesBodyJSTemplate(templData) } diff --git a/pkg/cover/heatmap_test.go b/pkg/cover/heatmap_test.go index 0f66256e4e8d..3a90dba101e3 100644 --- a/pkg/cover/heatmap_test.go +++ b/pkg/cover/heatmap_test.go @@ -254,16 +254,16 @@ func newRowIteratorMock(t *testing.T, events []*FileCoverageWithLineInfo, func TestFilesCoverageToTemplateData(t *testing.T) { tests := []struct { - name string - input []*fileCoverageWithDetails - want *templateHeatmap + name string + input []*fileCoverageWithDetails + hideEmpty bool + want *templateHeatmap }{ { name: "empty input", input: []*fileCoverageWithDetails{}, want: &templateHeatmap{ Root: &templateHeatmapRow{ - Items: []*templateHeatmapRow{}, IsDir: true, }, }, @@ -283,7 +283,6 @@ func TestFilesCoverageToTemplateData(t *testing.T) { Root: &templateHeatmapRow{ Items: []*templateHeatmapRow{ { - Items: []*templateHeatmapRow{}, Name: "file1", Coverage: []int64{100}, IsDir: false, @@ -332,7 +331,6 @@ func TestFilesCoverageToTemplateData(t *testing.T) { { Items: []*templateHeatmapRow{ { - Items: []*templateHeatmapRow{}, Name: "file1", Coverage: []int64{100, 0}, IsDir: false, @@ -347,7 +345,6 @@ func TestFilesCoverageToTemplateData(t *testing.T) { "/graph/coverage/file?dateto=2024-07-02&period=day&commit=commit2&filepath=dir/file1"}, }, { - Items: []*templateHeatmapRow{}, Name: "file2", Coverage: []int64{0, 0}, IsDir: false, @@ -385,10 +382,32 @@ func TestFilesCoverageToTemplateData(t *testing.T) { Periods: []string{"2024-07-01(1)", "2024-07-02(1)"}, }, }, + { + name: "hide empty", + hideEmpty: true, + input: []*fileCoverageWithDetails{ + { + Filepath: "file1", + Instrumented: 1, + Covered: 0, + TimePeriod: makeTimePeriod(t, civil.Date{Year: 2024, Month: time.July, Day: 1}, coveragedb.DayPeriod), + Commit: "commit1", + }, + }, + want: &templateHeatmap{ + Root: &templateHeatmapRow{ + Coverage: []int64{0}, + IsDir: true, + LastDayInstrumented: 1, + Tooltips: []string{"Instrumented:\t1 blocks\nCovered:\t0 blocks"}, + }, + Periods: []string{"2024-07-01(1)"}, + }, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got := filesCoverageToTemplateData(test.input) + got := filesCoverageToTemplateData(test.input, test.hideEmpty) assert.EqualExportedValues(t, test.want, got) }) }