Skip to content

Commit

Permalink
dashboard/app: test coverage /file link
Browse files Browse the repository at this point in the history
1. Init 1 coveragedb client per AppEngine instance.
2. Always init coverage handlers. It simplifies testing.
3. Read webGit from ctx to make it mockable.
4. Read coveragedb client from ctx to make it mockable.
5. Use int for file line number and int64 for merged coverage.
  • Loading branch information
tarasmadan committed Jan 24, 2025
1 parent 64aa306 commit c89367d
Show file tree
Hide file tree
Showing 17 changed files with 356 additions and 47 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions dashboard/app/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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!"
Expand Down
68 changes: 64 additions & 4 deletions dashboard/app/coverage.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"html/template"
"net/http"
"os"
"slices"
"strconv"

Expand All @@ -17,8 +18,36 @@ import (
"github.com/google/syzkaller/pkg/coveragedb/spannerclient"
"github.com/google/syzkaller/pkg/covermerger"
"github.com/google/syzkaller/pkg/validator"
"golang.org/x/exp/maps"
)

var coverageDBClient spannerclient.SpannerClient

func initCoverageDB() {
projectID := os.Getenv("GOOGLE_CLOUD_PROJECT")
if projectID == "" {
// It is a test environment.
// Use SetCoverageDBClient to specify the coveragedb mock or emulator in every test.
return
}
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,
Expand All @@ -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")

Expand Down Expand Up @@ -127,7 +160,8 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques
targetCommit := r.FormValue("commit")
kernelFilePath := r.FormValue("filepath")
manager := r.FormValue("manager")
allowedManagers, err := CachedManagerList(c, hdr.Namespace)
// It is easier to test than the CachedManagerList.
allowedManagers := maps.Keys(nsConfig.Managers)
if err != nil {

Check failure on line 165 in dashboard/app/coverage.go

View workflow job for this annotation

GitHub Actions / build

nilness: impossible condition: nil != nil (syz-linter)
return fmt.Errorf("CachedManagerList: %w", err)

Check failure on line 166 in dashboard/app/coverage.go

View workflow job for this annotation

GitHub Actions / build

lint: Don't start log/error messages with a Capital letter (syz-linter)
}
Expand All @@ -150,24 +184,35 @@ 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)

Check failure on line 191 in dashboard/app/coverage.go

View workflow job for this annotation

GitHub Actions / build

the line is 126 characters long, which exceeds the maximum of 120 characters. (lll)
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)

Check failure on line 199 in dashboard/app/coverage.go

View workflow job for this annotation

GitHub Actions / build

the line is 129 characters long, which exceeds the maximum of 120 characters. (lll)
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 {
Expand All @@ -178,11 +223,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
Expand Down
167 changes: 167 additions & 0 deletions dashboard/app/coverage_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
// 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 covermerger.FileVersProvider
url string
wantInRes []string
}{
{
name: "empty db",
covDB: func(t *testing.T) spannerclient.SpannerClient { return emptyCoverageDBFixture(t, 1) },
fileProv: staticFileProvider(t),
url: "/test2/graph/coverage/file?dateto=2025-01-31&period=month&commit=c0e75905caf368e19aab585d20151500e750de89&filepath=virt/kvm/kvm_main.c",

Check failure on line 32 in dashboard/app/coverage_test.go

View workflow job for this annotation

GitHub Actions / build

the line is 151 characters long, which exceeds the maximum of 120 characters. (lll)
wantInRes: []string{"1 line1"},
},
{
name: "regular db",
covDB: func(t *testing.T) spannerclient.SpannerClient { return coverageDBFixture(t) },
fileProv: staticFileProvider(t),
url: "/test2/graph/coverage/file?dateto=2025-01-31&period=month&commit=c0e75905caf368e19aab585d20151500e750de89&filepath=virt/kvm/kvm_main.c",

Check failure on line 39 in dashboard/app/coverage_test.go

View workflow job for this annotation

GitHub Actions / build

the line is 150 characters long, which exceeds the maximum of 120 characters. (lll)
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: 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",

Check failure on line 49 in dashboard/app/coverage_test.go

View workflow job for this annotation

GitHub Actions / build

the line is 191 characters long, which exceeds the maximum of 120 characters. (lll)
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)
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
}
3 changes: 3 additions & 0 deletions dashboard/app/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, &currentURLKey, r.URL.RequestURI())
authorizedUser, _ := userAccessLevel(currentUser(c), "", getConfig(c))
if !authorizedUser {
Expand Down
12 changes: 5 additions & 7 deletions dashboard/app/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
16 changes: 16 additions & 0 deletions dashboard/app/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/cover/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Loading

0 comments on commit c89367d

Please sign in to comment.