Skip to content

Commit 800b2f8

Browse files
tarasmadantarasmadan
committed
dashboard/app: allow to fetch coverage from multiple managers
Use '&manager=...&manager=...' GET params on the /ns/coverage page.
1 parent d6b2ee5 commit 800b2f8

File tree

8 files changed

+158
-97
lines changed

8 files changed

+158
-97
lines changed

‎dashboard/app/coverage.go‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func handleHeatmap(c context.Context, w http.ResponseWriter, r *http.Request, f
137137
return ErrClientNotFound
138138
}
139139
ss := r.FormValue(covPageParams[SubsystemName])
140-
manager := r.FormValue(covPageParams[ManagerName])
140+
fromManagers := r.Form[covPageParams[ManagerName]]
141141

142142
periodType := r.FormValue(covPageParams[PeriodType])
143143
if periodType == "" {
@@ -196,7 +196,7 @@ func handleHeatmap(c context.Context, w http.ResponseWriter, r *http.Request, f
196196
&coveragedb.SelectScope{
197197
Ns: hdr.Namespace,
198198
Subsystem: ss,
199-
Manager: manager,
199+
Managers: fromManagers,
200200
Periods: periods,
201201
},
202202
onlyUnique, subsystems, managers,
@@ -229,6 +229,20 @@ func makeProxyURIProvider(url string) covermerger.FuncProxyURI {
229229
}
230230
}
231231

232+
func validateManagerNames(managers []string) validator.Result {
233+
if len(managers) == 0 {
234+
return validator.ResultOk
235+
}
236+
for _, manager := range managers {
237+
if vr := validator.AnyOk(
238+
validator.Allowlisted(manager, []string{"", "*"}, covPageParams[ManagerName]),
239+
validator.ManagerName(manager, covPageParams[ManagerName])); !vr.Ok {
240+
return vr
241+
}
242+
}
243+
return validator.ResultOk
244+
}
245+
232246
func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Request) error {
233247
hdr, err := commonHeader(c, r, w, "")
234248
if err != nil {
@@ -242,14 +256,12 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques
242256
periodType := r.FormValue(covPageParams[PeriodType])
243257
targetCommit := r.FormValue(covPageParams[CommitHash])
244258
kernelFilePath := r.FormValue(covPageParams[FilePath])
245-
manager := r.FormValue(covPageParams[ManagerName])
259+
managers := r.Form[covPageParams[ManagerName]]
246260
if err := validator.AnyError("input validation failed",
247261
validator.TimePeriodType(periodType, covPageParams[PeriodType]),
248262
validator.CommitHash(targetCommit, covPageParams[CommitHash]),
249263
validator.KernelFilePath(kernelFilePath, covPageParams[FilePath]),
250-
validator.AnyOk(
251-
validator.Allowlisted(manager, []string{"", "*"}, covPageParams[ManagerName]),
252-
validator.ManagerName(manager, covPageParams[ManagerName])),
264+
validateManagerNames(managers),
253265
); err != nil {
254266
return fmt.Errorf("%w: %w", err, ErrClientBadRequest)
255267
}
@@ -268,16 +280,16 @@ func handleFileCoverage(c context.Context, w http.ResponseWriter, r *http.Reques
268280
return fmt.Errorf("spannerdb client is nil")
269281
}
270282
hitLines, hitCounts, err := coveragedb.ReadLinesHitCount(
271-
c, client, hdr.Namespace, targetCommit, kernelFilePath, manager, tp)
283+
c, client, hdr.Namespace, targetCommit, kernelFilePath, managers, tp)
272284
covMap := coveragedb.MakeCovMap(hitLines, hitCounts)
273285
if err != nil {
274-
return fmt.Errorf("coveragedb.ReadLinesHitCount(%s): %w", manager, err)
286+
return fmt.Errorf("coveragedb.ReadLinesHitCount(%v): %w", managers, err)
275287
}
276288
if onlyUnique {
277289
// This request is expected to be made second by tests.
278290
// Moving it to goroutine don't forget to change multiManagerCovDBFixture.
279291
allHitLines, allHitCounts, err := coveragedb.ReadLinesHitCount(
280-
c, client, hdr.Namespace, targetCommit, kernelFilePath, "*", tp)
292+
c, client, hdr.Namespace, targetCommit, kernelFilePath, nil, tp)
281293
if err != nil {
282294
return fmt.Errorf("coveragedb.ReadLinesHitCount(*): %w", err)
283295
}

‎dashboard/app/coverage_test.go‎

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,14 @@ func TestFileCoverage(t *testing.T) {
3939
wantInRes: []string{"1 line1"},
4040
},
4141
{
42-
name: "regular db",
43-
covDB: func(t *testing.T) spannerclient.SpannerClient { return coverageDBFixture(t) },
42+
name: "regular db",
43+
covDB: func(t *testing.T) spannerclient.SpannerClient {
44+
return coverageDBFixture(t,
45+
[]*coveragedb.LinesCoverage{{
46+
LinesInstrumented: []int64{1, 2, 3},
47+
HitCounts: []int64{4, 5, 6},
48+
}})
49+
},
4450
fileProv: func(t *testing.T) covermerger.FileVersProvider { return staticFileProvider(t) },
4551
url: "/test2/coverage/file?dateto=2025-01-31&period=month" +
4652
"&commit=c0e75905caf368e19aab585d20151500e750de89&filepath=virt/kvm/kvm_main.c",
@@ -62,6 +68,31 @@ func TestFileCoverage(t *testing.T) {
6268
" 3 line3", // Covered only by "*" managers.
6369
},
6470
},
71+
{
72+
name: "multiple managers, merge result",
73+
covDB: func(t *testing.T) spannerclient.SpannerClient {
74+
return coverageDBFixture(t,
75+
[]*coveragedb.LinesCoverage{
76+
{
77+
LinesInstrumented: []int64{1, 2, 3},
78+
HitCounts: []int64{1, 2, 3},
79+
},
80+
{
81+
LinesInstrumented: []int64{1, 2, 3},
82+
HitCounts: []int64{10, 20, 30},
83+
},
84+
})
85+
},
86+
fileProv: func(t *testing.T) covermerger.FileVersProvider { return staticFileProvider(t) },
87+
url: "/test2/coverage/file?dateto=2025-01-31&period=month" +
88+
"&commit=c0e75905caf368e19aab585d20151500e750de89&filepath=virt/kvm/kvm_main.c" +
89+
"&manager=manager1&manager=manager2",
90+
wantInRes: []string{
91+
" 11 1 line1",
92+
" 22 2 line2",
93+
" 33 3 line3",
94+
},
95+
},
6596
}
6697
for _, test := range tests {
6798
t.Run(test.name, func(t *testing.T) {
@@ -112,11 +143,8 @@ func emptyCoverageDBFixture(t *testing.T, times int) spannerclient.SpannerClient
112143
return m
113144
}
114145

115-
func coverageDBFixture(t *testing.T) spannerclient.SpannerClient {
116-
mRowIt := newRowIteratorMock(t, []*coveragedb.LinesCoverage{{
117-
LinesInstrumented: []int64{1, 2, 3},
118-
HitCounts: []int64{4, 5, 6},
119-
}})
146+
func coverageDBFixture(t *testing.T, covLines []*coveragedb.LinesCoverage) spannerclient.SpannerClient {
147+
mRowIt := coveragedb.NewRowIteratorMock(t, covLines)
120148

121149
mTran := mocks.NewReadOnlyTransaction(t)
122150
mTran.On("Query", mock.Anything, mock.Anything).
@@ -131,14 +159,14 @@ func coverageDBFixture(t *testing.T) spannerclient.SpannerClient {
131159
func multiManagerCovDBFixture(t *testing.T) spannerclient.SpannerClient {
132160
mReadFullCoverageTran := mocks.NewReadOnlyTransaction(t)
133161
mReadFullCoverageTran.On("Query", mock.Anything, mock.Anything).
134-
Return(newRowIteratorMock(t, []*coveragedb.LinesCoverage{{
162+
Return(coveragedb.NewRowIteratorMock(t, []*coveragedb.LinesCoverage{{
135163
LinesInstrumented: []int64{1, 2, 3},
136164
HitCounts: []int64{4, 5, 6},
137165
}})).Once()
138166

139167
mReadPartialCoverageTran := mocks.NewReadOnlyTransaction(t)
140168
mReadPartialCoverageTran.On("Query", mock.Anything, mock.Anything).
141-
Return(newRowIteratorMock(t, []*coveragedb.LinesCoverage{{
169+
Return(coveragedb.NewRowIteratorMock(t, []*coveragedb.LinesCoverage{{
142170
LinesInstrumented: []int64{1, 2},
143171
HitCounts: []int64{3, 5},
144172
}})).Once()
@@ -152,25 +180,3 @@ func multiManagerCovDBFixture(t *testing.T) spannerclient.SpannerClient {
152180

153181
return m
154182
}
155-
156-
func newRowIteratorMock[K any](t *testing.T, cov []*K,
157-
) *mocks.RowIterator {
158-
m := mocks.NewRowIterator(t)
159-
m.On("Stop").Once().Return()
160-
for _, item := range cov {
161-
mRow := mocks.NewRow(t)
162-
mRow.On("ToStruct", mock.Anything).
163-
Run(func(args mock.Arguments) {
164-
arg := args.Get(0).(*K)
165-
*arg = *item
166-
}).
167-
Return(nil).Once()
168-
169-
m.On("Next").
170-
Return(mRow, nil).Once()
171-
}
172-
173-
m.On("Next").
174-
Return(nil, iterator.Done).Once()
175-
return m
176-
}

‎dashboard/app/public_json_api_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,11 +328,11 @@ func fileFuncLinesDBFixture(t *testing.T, funcLines []*coveragedb.FuncLines,
328328
fileCovWithLineInfo []*coveragedb.FileCoverageWithLineInfo) spannerclient.SpannerClient {
329329
mPartialTran := mocks.NewReadOnlyTransaction(t)
330330
mPartialTran.On("Query", mock.Anything, mock.Anything).
331-
Return(newRowIteratorMock(t, funcLines)).Once()
331+
Return(coveragedb.NewRowIteratorMock(t, funcLines)).Once()
332332

333333
mFullTran := mocks.NewReadOnlyTransaction(t)
334334
mFullTran.On("Query", mock.Anything, mock.Anything).
335-
Return(newRowIteratorMock(t, fileCovWithLineInfo)).Once()
335+
Return(coveragedb.NewRowIteratorMock(t, fileCovWithLineInfo)).Once()
336336

337337
m := mocks.NewSpannerClient(t)
338338
m.On("Single").

‎dashboard/app/reporting_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,7 +1375,7 @@ func TestCoverageRegression(t *testing.T) {
13751375

13761376
mTran1 := mocks.NewReadOnlyTransaction(t)
13771377
mTran1.On("Query", mock.Anything, mock.Anything).
1378-
Return(newRowIteratorMock(t, []*coveragedb.FileCoverageWithDetails{
1378+
Return(coveragedb.NewRowIteratorMock(t, []*coveragedb.FileCoverageWithDetails{
13791379
{
13801380
Filepath: "file_name.c",
13811381
Instrumented: 100,
@@ -1385,7 +1385,7 @@ func TestCoverageRegression(t *testing.T) {
13851385

13861386
mTran2 := mocks.NewReadOnlyTransaction(t)
13871387
mTran2.On("Query", mock.Anything, mock.Anything).
1388-
Return(newRowIteratorMock(t, []*coveragedb.FileCoverageWithDetails{
1388+
Return(coveragedb.NewRowIteratorMock(t, []*coveragedb.FileCoverageWithDetails{
13891389
{
13901390
Filepath: "file_name.c",
13911391
Instrumented: 0,

‎dashboard/app/static/coverage.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ function initUpdateForm(){
4040
function onShowFileContent(url) {
4141
var curUrlParams = new URLSearchParams(window.location.search);
4242
url += '&subsystem=' + (curUrlParams.get('subsystem') || "")
43-
url += '&manager=' + (curUrlParams.get('manager') || "")
43+
// Get all 'manager' parameters.
44+
var managerParams = curUrlParams.getAll('manager');
45+
managerParams.forEach(function(managerValue) {
46+
url += '&manager=' + (managerValue || "");
47+
});
4448
url += '&unique-only=' + (curUrlParams.get('unique-only') || "")
4549

4650
$.get(url, function(response) {

‎pkg/coveragedb/coveragedb.go‎

Lines changed: 55 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"fmt"
1111
"io"
1212
"maps"
13+
"slices"
1314
"sync/atomic"
1415
"time"
1516

@@ -140,9 +141,25 @@ type LinesCoverage struct {
140141
HitCounts []int64
141142
}
142143

143-
func linesCoverageStmt(ns, filepath, commit, manager string, timePeriod TimePeriod) spanner.Statement {
144-
if manager == "" {
145-
manager = "*"
144+
func (lc *LinesCoverage) Add(other *LinesCoverage) error {
145+
if lc.LinesInstrumented == nil && lc.HitCounts == nil {
146+
lc.LinesInstrumented = other.LinesInstrumented
147+
lc.HitCounts = other.HitCounts
148+
return nil
149+
}
150+
if !slices.Equal(lc.LinesInstrumented, other.LinesInstrumented) {
151+
return fmt.Errorf("slices are not equal")
152+
}
153+
154+
for i := range lc.HitCounts {
155+
lc.HitCounts[i] += other.HitCounts[i]
156+
}
157+
return nil
158+
}
159+
160+
func linesCoverageStmt(ns, filepath, commit string, managers []string, timePeriod TimePeriod) spanner.Statement {
161+
if len(managers) == 0 {
162+
managers = append(managers, "*")
146163
}
147164
return spanner.Statement{
148165
SQL: `
@@ -153,40 +170,50 @@ from merge_history
153170
join files
154171
on merge_history.session = files.session
155172
where
156-
namespace=$1 and dateto=$2 and duration=$3 and filepath=$4 and commit=$5 and manager=$6`,
173+
namespace=$1 and dateto=$2 and duration=$3 and filepath=$4 and commit=$5
174+
and manager=ANY(SELECT id FROM UNNEST($6) AS id)`,
157175
Params: map[string]interface{}{
158176
"p1": ns,
159177
"p2": timePeriod.DateTo,
160178
"p3": timePeriod.Days,
161179
"p4": filepath,
162180
"p5": commit,
163-
"p6": manager,
181+
"p6": managers,
164182
},
165183
}
166184
}
167185

186+
// ReadLinesHitCount returns lines, hit counts and error.
168187
func ReadLinesHitCount(ctx context.Context, client spannerclient.SpannerClient,
169-
ns, commit, file, manager string, tp TimePeriod,
188+
ns, commit, file string, managers []string, tp TimePeriod,
170189
) ([]int64, []int64, error) {
171-
stmt := linesCoverageStmt(ns, file, commit, manager, tp)
190+
stmt := linesCoverageStmt(ns, file, commit, managers, tp)
172191
iter := client.Single().Query(ctx, stmt)
173192
defer iter.Stop()
174193

175-
row, err := iter.Next()
176-
if err == iterator.Done {
177-
return nil, nil, nil
178-
}
179-
if err != nil {
180-
return nil, nil, fmt.Errorf("iter.Next: %w", err)
181-
}
182-
var r LinesCoverage
183-
if err = row.ToStruct(&r); err != nil {
184-
return nil, nil, fmt.Errorf("failed to row.ToStruct() spanner DB: %w", err)
194+
var recordsReceived int
195+
var res LinesCoverage
196+
for {
197+
row, err := iter.Next()
198+
if err == iterator.Done {
199+
break
200+
}
201+
if err != nil {
202+
return nil, nil, fmt.Errorf("iter.Next: %w", err)
203+
}
204+
var r LinesCoverage
205+
if err = row.ToStruct(&r); err != nil {
206+
return nil, nil, fmt.Errorf("failed to row.ToStruct() spanner DB: %w", err)
207+
}
208+
recordsReceived++
209+
if err := res.Add(&r); err != nil {
210+
return nil, nil, fmt.Errorf("res.Add: %w", err)
211+
}
185212
}
186-
if _, err := iter.Next(); err != iterator.Done {
213+
if len(managers) == 1 && recordsReceived > 1 {
187214
return nil, nil, fmt.Errorf("more than 1 line is available")
188215
}
189-
return r.LinesInstrumented, r.HitCounts, nil
216+
return res.LinesInstrumented, res.HitCounts, nil
190217
}
191218

192219
func historyMutation(session string, template *HistoryRecord) *spanner.Mutation {
@@ -403,7 +430,7 @@ func MakeCovMap(keys, vals []int64) map[int]int64 {
403430
type SelectScope struct {
404431
Ns string
405432
Subsystem string
406-
Manager string
433+
Managers []string
407434
Periods []TimePeriod
408435
}
409436

@@ -412,7 +439,7 @@ type SelectScope struct {
412439
func FilesCoverageStream(ctx context.Context, client spannerclient.SpannerClient, ns string, timePeriod TimePeriod,
413440
) (<-chan *FileCoverageWithLineInfo, <-chan error) {
414441
iter := client.Single().Query(ctx,
415-
filesCoverageWithDetailsStmt(ns, "", "", timePeriod, true))
442+
filesCoverageWithDetailsStmt(ns, "", nil, timePeriod, true))
416443
resCh := make(chan *FileCoverageWithLineInfo)
417444
errCh := make(chan error)
418445
go func() {
@@ -435,14 +462,14 @@ func FilesCoverageWithDetails(
435462
for _, timePeriod := range scope.Periods {
436463
needLinesDetails := onlyUnique
437464
iterManager := client.Single().Query(ctx,
438-
filesCoverageWithDetailsStmt(scope.Ns, scope.Subsystem, scope.Manager, timePeriod, needLinesDetails))
465+
filesCoverageWithDetailsStmt(scope.Ns, scope.Subsystem, scope.Managers, timePeriod, needLinesDetails))
439466
defer iterManager.Stop()
440467

441468
var err error
442469
var periodRes []*FileCoverageWithDetails
443470
if onlyUnique {
444471
iterAll := client.Single().Query(ctx,
445-
filesCoverageWithDetailsStmt(scope.Ns, scope.Subsystem, "", timePeriod, needLinesDetails))
472+
filesCoverageWithDetailsStmt(scope.Ns, scope.Subsystem, nil, timePeriod, needLinesDetails))
446473
defer iterAll.Stop()
447474
periodRes, err = readCoverageUniq(iterAll, iterManager)
448475
if err != nil {
@@ -462,10 +489,10 @@ func FilesCoverageWithDetails(
462489
return res, nil
463490
}
464491

465-
func filesCoverageWithDetailsStmt(ns, subsystem, manager string, timePeriod TimePeriod, withLines bool,
492+
func filesCoverageWithDetailsStmt(ns, subsystem string, managers []string, timePeriod TimePeriod, withLines bool,
466493
) spanner.Statement {
467-
if manager == "" {
468-
manager = "*"
494+
if len(managers) == 0 {
495+
managers = append(managers, "*")
469496
}
470497
selectColumns := "commit, instrumented, covered, files.filepath, subsystems"
471498
if withLines {
@@ -479,12 +506,12 @@ from merge_history
479506
join file_subsystems
480507
on merge_history.namespace = file_subsystems.namespace and files.filepath = file_subsystems.filepath
481508
where
482-
merge_history.namespace=$1 and dateto=$2 and duration=$3 and manager=$4`,
509+
merge_history.namespace=$1 and dateto=$2 and duration=$3 and manager=ANY(SELECT id FROM UNNEST($4) AS id)`,
483510
Params: map[string]interface{}{
484511
"p1": ns,
485512
"p2": timePeriod.DateTo,
486513
"p3": timePeriod.Days,
487-
"p4": manager,
514+
"p4": managers,
488515
},
489516
}
490517
if subsystem != "" {

0 commit comments

Comments
 (0)