diff --git a/dashboard/app/coverage.go b/dashboard/app/coverage.go index e0ceab563ee1..2db55e28cba0 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 { diff --git a/pkg/cover/heatmap.go b/pkg/cover/heatmap.go index b8e3bbfa6690..60bfff0e8b99 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,12 @@ type fileCoverageWithDetails struct { Subsystems []string } +type fileCoverageWithLineInfo struct { + fileCoverageWithDetails + LinesInstrumented []int64 + HitCounts []int64 +} + type pageColumnTarget struct { TimePeriod coveragedb.TimePeriod Commit string @@ -157,18 +164,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 +193,143 @@ 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 len(partCov.LinesInstrumented) > len(fullCov.LinesInstrumented) || + len(partCov.HitCounts) > len(fullCov.HitCounts) || + partCov.Commit != fullCov.Commit { + return fmt.Errorf("db record for file %s don't match", fullCov.Filepath) + } + res = append(res, uniqCoverage(fullCov, partCov)) + 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 uniqCoverage(full, partial *fileCoverageWithLineInfo) *fileCoverageWithDetails { + res := full.fileCoverageWithDetails // Use Instrumented count from full aggregation. + res.Covered = 0 // We're recalculating only the covered lines. + fullCov := map[int64]int64{} + for i, ln := range full.LinesInstrumented { + fullCov[ln] = full.HitCounts[i] + } + for i, ln := range partial.LinesInstrumented { + if hitCount, exist := fullCov[ln]; exist && hitCount > 0 && hitCount == partial.HitCounts[i] { + res.Covered++ + } + } + 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 +364,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 +377,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_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 +}