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.

10 files changed, 110 insertions, 49 deletions

diff --git a/pkg/cover/file.go b/pkg/cover/file.go
index c56f88204..ea6c326f0 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 04b7e6914..4cc212fad 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 ad1f9bf64..0f66256e4 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 22e84d8eb..0692bc131 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 36f42fdc1..ab2b1efc5 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 ebc747f47..817099a60 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 000000000..69fadfa42
--- /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 e6c67d1d1..73a08786a 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 43bfee7e0..554ee3f97 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 b9031302a..12224963e 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)+".*)$")