tools: More CTS tooling improvements
• Add the included trybots in the CL description. All of these trybots are tested by the roll, but the final CQ-submit wouldn't necessarily test all of the variants before landing. This would mean that the 'cts export' could miss some results, as it takes the last PS with any results.
• Add --force flag to cts roll to force a roll. Useful for testing.
• Emit timing diagnostics for tests labelled 'Slow' instead of unhelpfully stating they pass.
• Enable the --cl and --ps flags for cts export
• Export with the most recent data to the top of the spreadsheet
Bug: dawn:1401
Change-Id: Id926367ab805bfb9f3032fce9cce7f00daf7a5d4
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/88661
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
diff --git a/tools/src/cmd/cts/common/deps.go b/tools/src/cmd/cts/common/deps.go
index 27b13d3..f82ddc3 100644
--- a/tools/src/cmd/cts/common/deps.go
+++ b/tools/src/cmd/cts/common/deps.go
@@ -57,10 +57,6 @@
b.WriteString(deps[end:])
newDEPS = b.String()
- if deps == newDEPS {
- fmt.Println("CTS is already up to date")
- return "", "", nil
- }
if s := container.NewSet(oldCTSHashes...); len(s) > 1 {
fmt.Println("DEPS contained multiple hashes for CTS, using first for logs")
diff --git a/tools/src/cmd/cts/common/results.go b/tools/src/cmd/cts/common/results.go
index e135e22..095106a 100644
--- a/tools/src/cmd/cts/common/results.go
+++ b/tools/src/cmd/cts/common/results.go
@@ -62,9 +62,11 @@
}
// GetResults loads or fetches the results, based on the values of r.
+// GetResults will update the ResultSource with the inferred patchset, if a file
+// and specific patchset was not specified.
func (r *ResultSource) GetResults(ctx context.Context, cfg Config, auth auth.Options) (result.List, error) {
// Check that File and Patchset weren't both specified
- ps := r.Patchset
+ ps := &r.Patchset
if r.File != "" && ps.Change != 0 {
fmt.Fprintln(flag.CommandLine.Output(), "only one of --results and --cl can be specified")
return nil, subcmd.ErrInvalidCLA
@@ -97,7 +99,9 @@
if err != nil {
return nil, err
}
- results, ps, err := MostRecentResultsForChange(ctx, cfg, r.CacheDir, gerrit, bb, rdb, latest.Number)
+ fmt.Printf("scanning for latest patchset of %v...\n", latest.Number)
+ var results result.List
+ results, *ps, err = MostRecentResultsForChange(ctx, cfg, r.CacheDir, gerrit, bb, rdb, latest.Number)
if err != nil {
return nil, err
}
@@ -112,7 +116,7 @@
if err != nil {
return nil, err
}
- ps, err = gerrit.LatestPatchest(strconv.Itoa(ps.Change))
+ *ps, err = gerrit.LatestPatchest(strconv.Itoa(ps.Change))
if err != nil {
err := fmt.Errorf("failed to find latest patchset of change %v: %w",
ps.Change, err)
@@ -123,12 +127,12 @@
// Obtain the patchset's results, kicking a build if there are no results
// already available.
log.Printf("fetching results from cl %v ps %v...", ps.Change, ps.Patchset)
- builds, err := GetOrStartBuildsAndWait(ctx, cfg, ps, bb, false)
+ builds, err := GetOrStartBuildsAndWait(ctx, cfg, *ps, bb, false)
if err != nil {
return nil, err
}
- results, err := CacheResults(ctx, cfg, ps, r.CacheDir, rdb, builds)
+ results, err := CacheResults(ctx, cfg, *ps, r.CacheDir, rdb, builds)
if err != nil {
return nil, err
}
diff --git a/tools/src/cmd/cts/export/export.go b/tools/src/cmd/cts/export/export.go
index 2e5826e..70792db 100644
--- a/tools/src/cmd/cts/export/export.go
+++ b/tools/src/cmd/cts/export/export.go
@@ -26,13 +26,10 @@
"strings"
"time"
- "dawn.googlesource.com/dawn/tools/src/buildbucket"
"dawn.googlesource.com/dawn/tools/src/cmd/cts/common"
"dawn.googlesource.com/dawn/tools/src/cts/result"
- "dawn.googlesource.com/dawn/tools/src/gerrit"
"dawn.googlesource.com/dawn/tools/src/git"
"dawn.googlesource.com/dawn/tools/src/gitiles"
- "dawn.googlesource.com/dawn/tools/src/resultsdb"
"dawn.googlesource.com/dawn/tools/src/utils"
"go.chromium.org/luci/auth/client/authcli"
"golang.org/x/oauth2"
@@ -46,8 +43,8 @@
type cmd struct {
flags struct {
- auth authcli.Flags
- cacheDir string
+ auth authcli.Flags
+ results common.ResultSource
}
}
@@ -61,7 +58,7 @@
func (c *cmd) RegisterFlags(ctx context.Context, cfg common.Config) ([]string, error) {
c.flags.auth.Register(flag.CommandLine, common.DefaultAuthOptions())
- flag.StringVar(&c.flags.cacheDir, "cache", common.DefaultCacheDir, "path to the results cache")
+ c.flags.results.RegisterFlags(cfg)
return nil, nil
}
@@ -114,34 +111,17 @@
// Fetch the table column names
columns, err := fetchRow[string](s, spreadsheet, dataSheet, 0)
- // Create a gerrit client, and find the latest CTS roll
- gerrit, err := gerrit.New(cfg.Gerrit.Host, gerrit.Credentials{})
- if err != nil {
- return err
- }
- latestRoll, err := common.LatestCTSRoll(gerrit)
- if err != nil {
- return err
- }
-
- // Grab the results from the latest CTS roll
- bb, err := buildbucket.New(ctx, auth)
- if err != nil {
- return err
- }
- rdb, err := resultsdb.New(ctx, auth)
- if err != nil {
- return err
- }
- results, ps, err := common.MostRecentResultsForChange(ctx, cfg, c.flags.cacheDir, gerrit, bb, rdb, latestRoll.Number)
+ // Grab the results
+ results, err := c.flags.results.GetResults(ctx, cfg, auth)
if err != nil {
return err
}
if len(results) == 0 {
return fmt.Errorf("no results found")
}
+ ps := c.flags.results.Patchset
- // Find the CTS revision from the latest CTS roll
+ // Find the CTS revision
dawn, err := gitiles.New(ctx, cfg.Git.Dawn.Host, cfg.Git.Dawn.Project)
if err != nil {
return fmt.Errorf("failed to open dawn host: %w", err)
@@ -187,10 +167,20 @@
}
}
+ // Insert a blank row under the column header row
+ if err := insertBlankRows(s, spreadsheet, dataSheet, 1, 1); err != nil {
+ return err
+ }
+
// Add a new row to the spreadsheet
- _, err = s.Spreadsheets.Values.Append(spreadsheet.SpreadsheetId, "Data", &sheets.ValueRange{
- Values: [][]any{data},
- }).ValueInputOption("RAW").Do()
+ _, err = s.Spreadsheets.Values.BatchUpdate(spreadsheet.SpreadsheetId,
+ &sheets.BatchUpdateValuesRequest{
+ ValueInputOption: "RAW",
+ Data: []*sheets.ValueRange{{
+ Range: rowRange(1, dataSheet),
+ Values: [][]any{data},
+ }},
+ }).Do()
if err != nil {
return fmt.Errorf("failed to update spreadsheet: %v", err)
}
@@ -232,6 +222,26 @@
return out, nil
}
+// insertBlankRows inserts blank rows into the given sheet.
+func insertBlankRows(srv *sheets.Service, spreadsheet *sheets.Spreadsheet, sheet *sheets.Sheet, aboveRow, count int) error {
+ req := sheets.BatchUpdateSpreadsheetRequest{
+ Requests: []*sheets.Request{{
+ InsertRange: &sheets.InsertRangeRequest{
+ Range: &sheets.GridRange{
+ SheetId: sheet.Properties.SheetId,
+ StartRowIndex: int64(aboveRow),
+ EndRowIndex: int64(aboveRow + count),
+ },
+ ShiftDimension: "ROWS",
+ }},
+ },
+ }
+ if _, err := srv.Spreadsheets.BatchUpdate(spreadsheet.SpreadsheetId, &req).Do(); err != nil {
+ return fmt.Errorf("BatchUpdate failed: %v", err)
+ }
+ return nil
+}
+
// countUnimplementedTests checks out the WebGPU CTS at ctsHash, builds the node
// command line tool, and runs it with '--list-unimplemented webgpu:*' to count
// the total number of unimplemented tests, which is returned.
diff --git a/tools/src/cmd/cts/roll/roll.go b/tools/src/cmd/cts/roll/roll.go
index cecacd2..98cc5e6 100644
--- a/tools/src/cmd/cts/roll/roll.go
+++ b/tools/src/cmd/cts/roll/roll.go
@@ -30,6 +30,7 @@
"dawn.googlesource.com/dawn/tools/src/buildbucket"
"dawn.googlesource.com/dawn/tools/src/cmd/cts/common"
+ "dawn.googlesource.com/dawn/tools/src/container"
"dawn.googlesource.com/dawn/tools/src/cts/expectations"
"dawn.googlesource.com/dawn/tools/src/cts/result"
"dawn.googlesource.com/dawn/tools/src/gerrit"
@@ -56,6 +57,7 @@
tscPath string
auth authcli.Flags
cacheDir string
+ force bool // Create a new roll, even if CTS is up to date
rebuild bool // Rebuild the expectations file from scratch
preserve bool // If false, abandon past roll changes
}
@@ -79,6 +81,7 @@
flag.StringVar(&c.flags.gitPath, "git", gitPath, "path to git")
flag.StringVar(&c.flags.tscPath, "tsc", tscPath, "path to tsc")
flag.StringVar(&c.flags.cacheDir, "cache", common.DefaultCacheDir, "path to the results cache")
+ flag.BoolVar(&c.flags.force, "force", false, "create a new roll, even if CTS is up to date")
flag.BoolVar(&c.flags.rebuild, "rebuild", false, "rebuild the expectation file from scratch")
flag.BoolVar(&c.flags.preserve, "preserve", false, "do not abandon existing rolls")
@@ -179,8 +182,9 @@
if err != nil {
return err
}
- if updatedDEPS == "" {
+ if newCTSHash == oldCTSHash && !r.flags.force {
// Already up to date
+ fmt.Println("CTS is already up to date")
return nil
}
@@ -421,6 +425,29 @@
msg.WriteString("\n")
msg.WriteString("Created with './tools/run cts roll'")
msg.WriteString("\n")
+ msg.WriteString("\n")
+ if len(r.cfg.Builders) > 0 {
+ msg.WriteString("Cq-Include-Trybots: ")
+ buildersByBucket := container.NewMap[string, []string]()
+ for _, build := range r.cfg.Builders {
+ key := fmt.Sprintf("luci.%v.%v", build.Project, build.Bucket)
+ buildersByBucket[key] = append(buildersByBucket[key], build.Builder)
+ }
+ first := true
+ for _, bucket := range buildersByBucket.Keys() {
+ // Cq-Include-Trybots: luci.chromium.try:win-dawn-rel;luci.dawn.try:mac-dbg,mac-rel
+ if !first {
+ msg.WriteString(";")
+ }
+ first = false
+ msg.WriteString(bucket)
+ msg.WriteString(":")
+ builders := buildersByBucket[bucket]
+ sort.Strings(builders)
+ msg.WriteString(strings.Join(builders, ","))
+ }
+ msg.WriteString("\n")
+ }
if changeID != "" {
msg.WriteString("Change-Id: ")
msg.WriteString(changeID)
diff --git a/tools/src/cmd/cts/roll/roll_test.go b/tools/src/cmd/cts/roll/roll_test.go
new file mode 100644
index 0000000..b7ba054
--- /dev/null
+++ b/tools/src/cmd/cts/roll/roll_test.go
@@ -0,0 +1,75 @@
+// Copyright 2022 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package roll
+
+import (
+ "testing"
+
+ "dawn.googlesource.com/dawn/tools/src/buildbucket"
+ "dawn.googlesource.com/dawn/tools/src/cmd/cts/common"
+ "dawn.googlesource.com/dawn/tools/src/git"
+ "github.com/google/go-cmp/cmp"
+)
+
+func MustParseHash(s string) git.Hash {
+ hash, err := git.ParseHash("d5e605a556408eaeeda64fb9d33c3f596fd90b70")
+ if err != nil {
+ panic(err)
+ }
+ return hash
+}
+
+func TestRollCommitMessage(t *testing.T) {
+ r := roller{
+ cfg: common.Config{
+ Builders: map[string]buildbucket.Builder{
+ "Win": {Project: "chromium", Bucket: "try", Builder: "win-dawn-rel"},
+ "Mac": {Project: "dawn", Bucket: "try", Builder: "mac-dbg"},
+ "Linux": {Project: "chromium", Bucket: "try", Builder: "linux-dawn-rel"},
+ },
+ },
+ }
+ msg := r.rollCommitMessage(
+ "d5e605a556408eaeeda64fb9d33c3f596fd90b70",
+ "29275672eefe76986bd4baa7c29ed17b66616b1b",
+ []git.CommitInfo{
+ {
+ Hash: MustParseHash("d5e605a556408eaeeda64fb9d33c3f596fd90b70"),
+ Subject: "Added thing A",
+ },
+ {
+ Hash: MustParseHash("29275672eefe76986bd4baa7c29ed17b66616b1b"),
+ Subject: "Tweaked thing B",
+ },
+ },
+ "I4aa059c6c183e622975b74dbdfdfe0b12341ae15",
+ )
+ expect := `Roll third_party/webgpu-cts/ d5e605a55..29275672e (2 commits)
+
+Update expectations and ts_sources
+
+https://chromium.googlesource.com/external/github.com/gpuweb/cts/+log/d5e605a55640..29275672eefe
+ - d5e605 Added thing A
+ - d5e605 Tweaked thing B
+
+Created with './tools/run cts roll'
+
+Cq-Include-Trybots: luci.chromium.try:linux-dawn-rel,win-dawn-rel;luci.dawn.try:mac-dbg
+Change-Id: I4aa059c6c183e622975b74dbdfdfe0b12341ae15
+`
+ if diff := cmp.Diff(msg, expect); diff != "" {
+ t.Errorf("rollCommitMessage: %v", diff)
+ }
+}
diff --git a/tools/src/cts/expectations/update.go b/tools/src/cts/expectations/update.go
index 13a11ab..a494551 100644
--- a/tools/src/cts/expectations/update.go
+++ b/tools/src/cts/expectations/update.go
@@ -19,6 +19,7 @@
"fmt"
"sort"
"strings"
+ "time"
"dawn.googlesource.com/dawn/tools/src/container"
"dawn.googlesource.com/dawn/tools/src/cts/query"
@@ -337,10 +338,27 @@
if keep { // Expectation chunk was marked with 'KEEP'
// Add a diagnostic if all tests of the expectation were 'Pass'
if s := results.Statuses(); len(s) == 1 && s.One() == result.Pass {
- if c := len(results); c > 1 {
- u.diag(Note, in.Line, "all %d tests now pass", len(results))
+ if ex := container.NewSet(in.Status...); len(ex) == 1 && ex.One() == string(result.Slow) {
+ // Expectation was 'Slow'. Give feedback on actual time taken.
+ var longest, average time.Duration
+ for _, r := range results {
+ if r.Duration > longest {
+ longest = r.Duration
+ }
+ average += r.Duration
+ }
+ if c := len(results); c > 1 {
+ average /= time.Duration(c)
+ u.diag(Note, in.Line, "longest test took %v (average %v)", longest, average)
+ } else {
+ u.diag(Note, in.Line, "test took %v", longest)
+ }
} else {
- u.diag(Note, in.Line, "test now passes")
+ if c := len(results); c > 1 {
+ u.diag(Note, in.Line, "all %d tests now pass", len(results))
+ } else {
+ u.diag(Note, in.Line, "test now passes")
+ }
}
}
return []Expectation{in}
