CTS: Handle may_exonerate tag

may_exonerate indicates that a test failed for a known issue that
we could exonerate. Merging of test results now removes results
with may_exonerate unless all of them were tagged as such. So, if
for example, a test fails for a known timeout issue, but has a
subsequent pass, the timeout will be ignored.

This serves to reduce the impact of known, hard-to-fix issues and
allow the CTS roller to make progress with less noise.

Change-Id: I5103a666496398a17b3aa6ccf3f267421e40ba97
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/101804
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
This commit is contained in:
Austin Eng 2022-09-14 14:29:07 +00:00 committed by Dawn LUCI CQ
parent cd86adaf41
commit a48e46f3a4
5 changed files with 117 additions and 43 deletions

View File

@ -219,6 +219,7 @@ func GetResults(
tags := result.NewTags() tags := result.NewTags()
duration := rpb.GetDuration().AsDuration() duration := rpb.GetDuration().AsDuration()
mayExonerate := false
for _, sp := range rpb.Tags { for _, sp := range rpb.Tags {
if sp.Key == "typ_tag" { if sp.Key == "typ_tag" {
@ -230,6 +231,12 @@ func GetResults(
return err return err
} }
} }
if sp.Key == "may_exonerate" {
var err error
if mayExonerate, err = strconv.ParseBool(sp.Value); err != nil {
return err
}
}
} }
if status == result.Pass && duration > cfg.Test.SlowThreshold { if status == result.Pass && duration > cfg.Test.SlowThreshold {
@ -237,10 +244,11 @@ func GetResults(
} }
results = append(results, result.Result{ results = append(results, result.Result{
Query: query.Parse(testName), Query: query.Parse(testName),
Status: status, Status: status,
Tags: tags, Tags: tags,
Duration: duration, Duration: duration,
MayExonerate: mayExonerate,
}) })
return nil return nil

View File

@ -53,11 +53,7 @@ func (c *cmd) Run(ctx context.Context, cfg common.Config) error {
return fmt.Errorf("while reading '%v': %w", path, err) return fmt.Errorf("while reading '%v': %w", path, err)
} }
// Combine and merge // Combine and merge
if len(results) > 0 { results = result.Merge(results, r)
results = result.Merge(results, r)
} else {
results = r
}
} }
// Open output file // Open output file

View File

@ -21,6 +21,7 @@ import (
"dawn.googlesource.com/dawn/tools/src/cmd/cts/common" "dawn.googlesource.com/dawn/tools/src/cmd/cts/common"
"dawn.googlesource.com/dawn/tools/src/cts/expectations" "dawn.googlesource.com/dawn/tools/src/cts/expectations"
"dawn.googlesource.com/dawn/tools/src/cts/result"
"go.chromium.org/luci/auth/client/authcli" "go.chromium.org/luci/auth/client/authcli"
) )
@ -65,6 +66,9 @@ func (c *cmd) Run(ctx context.Context, cfg common.Config) error {
return err return err
} }
// Merge to remove duplicates
results = result.Merge(results)
// Load the expectations file // Load the expectations file
ex, err := expectations.Load(c.flags.expectations) ex, err := expectations.Load(c.flags.expectations)
if err != nil { if err != nil {

View File

@ -22,6 +22,7 @@ import (
"os" "os"
"path/filepath" "path/filepath"
"sort" "sort"
"strconv"
"strings" "strings"
"time" "time"
@ -35,6 +36,9 @@ type Result struct {
Tags Tags Tags Tags
Status Status Status Status
Duration time.Duration Duration time.Duration
// If true, this result may be exonerated if there are other
// results with the same query and tags that have MayExonerate: false
MayExonerate bool
} }
// Format writes the Result to the fmt.State // Format writes the Result to the fmt.State
@ -43,9 +47,9 @@ type Result struct {
// This matches the order in which results are sorted. // This matches the order in which results are sorted.
func (r Result) Format(f fmt.State, verb rune) { func (r Result) Format(f fmt.State, verb rune) {
if len(r.Tags) > 0 { if len(r.Tags) > 0 {
fmt.Fprintf(f, "%v %v %v %v", r.Query, TagsToString(r.Tags), r.Status, r.Duration) fmt.Fprintf(f, "%v %v %v %v %v", r.Query, TagsToString(r.Tags), r.Status, r.Duration, r.MayExonerate)
} else { } else {
fmt.Fprintf(f, "%v %v %v", r.Query, r.Status, r.Duration) fmt.Fprintf(f, "%v %v %v %v", r.Query, r.Status, r.Duration, r.MayExonerate)
} }
} }
@ -112,19 +116,24 @@ func Parse(in string) (Result, error) {
b := token() b := token()
c := token() c := token()
d := token() d := token()
if a == "" || b == "" || c == "" || token() != "" { e := token()
if a == "" || b == "" || c == "" || d == "" || token() != "" {
return Result{}, fmt.Errorf("unable to parse result '%v'", in) return Result{}, fmt.Errorf("unable to parse result '%v'", in)
} }
query := query.Parse(a) query := query.Parse(a)
if d == "" { if e == "" {
status := Status(b) status := Status(b)
duration, err := time.ParseDuration(c) duration, err := time.ParseDuration(c)
if err != nil { if err != nil {
return Result{}, fmt.Errorf("unable to parse result '%v': %w", in, err) return Result{}, fmt.Errorf("unable to parse result '%v': %w", in, err)
} }
return Result{query, nil, status, duration}, nil mayExonerate, err := strconv.ParseBool(d)
if err != nil {
return Result{}, fmt.Errorf("unable to parse result '%v': %w", in, err)
}
return Result{query, nil, status, duration, mayExonerate}, nil
} else { } else {
tags := StringToTags(b) tags := StringToTags(b)
status := Status(c) status := Status(c)
@ -132,7 +141,11 @@ func Parse(in string) (Result, error) {
if err != nil { if err != nil {
return Result{}, fmt.Errorf("unable to parse result '%v': %w", in, err) return Result{}, fmt.Errorf("unable to parse result '%v': %w", in, err)
} }
return Result{query, tags, status, duration}, nil mayExonerate, err := strconv.ParseBool(e)
if err != nil {
return Result{}, fmt.Errorf("unable to parse result '%v': %w", in, err)
}
return Result{query, tags, status, duration, mayExonerate}, nil
} }
} }
@ -165,10 +178,11 @@ func (l List) TransformTags(f func(Tags) Tags) List {
cache[key] = tags cache[key] = tags
} }
out = append(out, Result{ out = append(out, Result{
Query: r.Query, Query: r.Query,
Tags: tags, Tags: tags,
Status: r.Status, Status: r.Status,
Duration: r.Duration, Duration: r.Duration,
MayExonerate: r.MayExonerate,
}) })
} }
return out return out
@ -189,6 +203,23 @@ func (l List) ReplaceDuplicates(f func(Statuses) Status) List {
k := key{r.Query, TagsToString(r.Tags)} k := key{r.Query, TagsToString(r.Tags)}
keyToIndices[k] = append(keyToIndices[k], i) keyToIndices[k] = append(keyToIndices[k], i)
} }
// Filter out exonerated results
for key, indices := range keyToIndices {
keptIndices := []int{}
for _, i := range indices {
// Copy all indices which are not exonerated into keptIndices.
if !l[i].MayExonerate {
keptIndices = append(keptIndices, i)
}
}
// Change indices to only the kept ones. If keptIndices is empty,
// then all results were marked with may_exonerate, and we keep all
// of them.
if len(keptIndices) > 0 {
keyToIndices[key] = keptIndices
}
}
// Resolve duplicates // Resolve duplicates
type StatusAndDuration struct { type StatusAndDuration struct {
Status Status Status Status
@ -221,10 +252,11 @@ func (l List) ReplaceDuplicates(f func(Statuses) Status) List {
k := key{r.Query, TagsToString(r.Tags)} k := key{r.Query, TagsToString(r.Tags)}
if sd, ok := merged[k]; ok { if sd, ok := merged[k]; ok {
out = append(out, Result{ out = append(out, Result{
Query: r.Query, Query: r.Query,
Tags: r.Tags, Tags: r.Tags,
Status: sd.Status, Status: sd.Status,
Duration: sd.Duration, Duration: sd.Duration,
MayExonerate: l[keyToIndices[k][0]].MayExonerate,
}) })
delete(merged, k) // Remove from map to prevent duplicates delete(merged, k) // Remove from map to prevent duplicates
} }
@ -360,12 +392,17 @@ func Write(w io.Writer, l List) error {
return nil return nil
} }
// Merge merges and sorts two results lists. // Merge merges and sorts multiple results lists.
// Duplicates are removed using the Deduplicate() function. // Duplicates are removed using the Deduplicate() function.
func Merge(a, b List) List { func Merge(lists ...List) List {
merged := make(List, 0, len(a)+len(b)) n := 0
merged = append(merged, a...) for _, l := range lists {
merged = append(merged, b...) n += len(l)
}
merged := make(List, 0, n)
for _, l := range lists {
merged = append(merged, l...)
}
out := merged.ReplaceDuplicates(Deduplicate) out := merged.ReplaceDuplicates(Deduplicate)
out.Sort() out.Sort()
return out return out

View File

@ -40,28 +40,31 @@ func TestStringAndParse(t *testing.T) {
for _, test := range []Test{ for _, test := range []Test{
{ {
result.Result{ result.Result{
Query: Q(`a`), Query: Q(`a`),
Status: result.Failure, Status: result.Failure,
Duration: time.Second * 42, Duration: time.Second * 42,
MayExonerate: false,
}, },
`a Failure 42s`, `a Failure 42s false`,
}, { }, {
result.Result{ result.Result{
Query: Q(`a:b,c,*`), Query: Q(`a:b,c,*`),
Tags: T("x"), Tags: T("x"),
Status: result.Pass, Status: result.Pass,
Duration: time.Second * 42, Duration: time.Second * 42,
MayExonerate: true,
}, },
`a:b,c,* x Pass 42s`, `a:b,c,* x Pass 42s true`,
}, },
{ {
result.Result{ result.Result{
Query: Q(`a:b,c:d,*`), Query: Q(`a:b,c:d,*`),
Tags: T("zzz", "x", "yy"), Tags: T("zzz", "x", "yy"),
Status: result.Failure, Status: result.Failure,
Duration: time.Second * 42, Duration: time.Second * 42,
MayExonerate: false,
}, },
`a:b,c:d,* x,yy,zzz Failure 42s`, `a:b,c:d,* x,yy,zzz Failure 42s false`,
}, },
} { } {
if diff := cmp.Diff(test.result.String(), test.expect); diff != "" { if diff := cmp.Diff(test.result.String(), test.expect); diff != "" {
@ -85,7 +88,8 @@ func TestParseError(t *testing.T) {
}{ }{
{``, `unable to parse result ''`}, {``, `unable to parse result ''`},
{`a`, `unable to parse result 'a'`}, {`a`, `unable to parse result 'a'`},
{`a b c d`, `unable to parse result 'a b c d': time: invalid duration "d"`}, {`a b c d e`, `unable to parse result 'a b c d e': time: invalid duration "d"`},
{`a b c 10s e`, `unable to parse result 'a b c 10s e': strconv.ParseBool: parsing "e": invalid syntax`},
} { } {
_, err := result.Parse(test.in) _, err := result.Parse(test.in)
got := "" got := ""
@ -378,6 +382,31 @@ func TestReplaceDuplicates(t *testing.T) {
result.Result{Query: Q(`b`), Status: result.Pass}, result.Result{Query: Q(`b`), Status: result.Pass},
}, },
}, },
{ //////////////////////////////////////////////////////////////////////
location: utils.ThisLine(),
results: result.List{
result.Result{Query: Q(`a`), Status: result.Failure, Duration: 1, MayExonerate: true},
result.Result{Query: Q(`a`), Status: result.Failure, Duration: 3, MayExonerate: true},
result.Result{Query: Q(`b`), Status: result.Failure, Duration: 1, MayExonerate: false},
result.Result{Query: Q(`b`), Status: result.Failure, Duration: 3, MayExonerate: false},
result.Result{Query: Q(`c`), Status: result.Pass, Duration: 1, MayExonerate: false},
result.Result{Query: Q(`c`), Status: result.Pass, Duration: 3, MayExonerate: false},
result.Result{Query: Q(`d`), Status: result.Failure, Duration: 1, MayExonerate: true},
result.Result{Query: Q(`d`), Status: result.Pass, Duration: 3, MayExonerate: false},
result.Result{Query: Q(`e`), Status: result.Failure, Duration: 1, MayExonerate: false},
result.Result{Query: Q(`e`), Status: result.Pass, Duration: 3, MayExonerate: true},
},
fn: func(result.Statuses) result.Status {
return result.Abort
},
expect: result.List{
result.Result{Query: Q(`a`), Status: result.Failure, Duration: 2, MayExonerate: true},
result.Result{Query: Q(`b`), Status: result.Failure, Duration: 2, MayExonerate: false},
result.Result{Query: Q(`c`), Status: result.Pass, Duration: 2, MayExonerate: false},
result.Result{Query: Q(`d`), Status: result.Pass, Duration: 3, MayExonerate: false},
result.Result{Query: Q(`e`), Status: result.Failure, Duration: 1, MayExonerate: false},
},
},
} { } {
got := test.results.ReplaceDuplicates(test.fn) got := test.results.ReplaceDuplicates(test.fn)
if diff := cmp.Diff(got, test.expect); diff != "" { if diff := cmp.Diff(got, test.expect); diff != "" {