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>

tools/src/cmd/cts/common/deps.go

@@ -57,10 +57,6 @@ func UpdateCTSHashInDeps(deps, newCTSHash string) (newDEPS, oldCTSHash string, e
 	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")

tools/src/cmd/cts/common/results.go

@@ -62,9 +62,11 @@ func (r *ResultSource) RegisterFlags(cfg Config) {
 }
 
 // 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 @@ func (r *ResultSource) GetResults(ctx context.Context, cfg Config, auth auth.Opt
 		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 @@ func (r *ResultSource) GetResults(ctx context.Context, cfg Config, auth auth.Opt
 		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 @@ func (r *ResultSource) GetResults(ctx context.Context, cfg Config, auth auth.Opt
 	// 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
 	}

tools/src/cmd/cts/export/export.go

@@ -26,13 +26,10 @@ import (
 	"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 @@ func init() {
 
 type cmd struct {
 	flags struct {
-		auth     authcli.Flags
+		auth    authcli.Flags
-		cacheDir string
+		results common.ResultSource
 	}
 }
 
@@ -61,7 +58,7 @@ func (cmd) Desc() string {
 
 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 @@ func (c *cmd) Run(ctx context.Context, cfg common.Config) error {
 	// Fetch the table column names
 	columns, err := fetchRow[string](s, spreadsheet, dataSheet, 0)
 
-	// Create a gerrit client, and find the latest CTS roll
+	// Grab the results
-	gerrit, err := gerrit.New(cfg.Gerrit.Host, gerrit.Credentials{})
+	results, err := c.flags.results.GetResults(ctx, cfg, auth)
-	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)
 	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 @@ func (c *cmd) Run(ctx context.Context, cfg common.Config) error {
 		}
 	}
 
+	// 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{
+	_, err = s.Spreadsheets.Values.BatchUpdate(spreadsheet.SpreadsheetId,
-		Values: [][]any{data},
+		&sheets.BatchUpdateValuesRequest{
-	}).ValueInputOption("RAW").Do()
+			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 @@ func fetchRow[T any](srv *sheets.Service, spreadsheet *sheets.Spreadsheet, sheet
 	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.

tools/src/cmd/cts/roll/roll.go

@@ -30,6 +30,7 @@ import (
 
 	"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 @@ type rollerFlags struct {
 	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 @@ func (c *cmd) RegisterFlags(ctx context.Context, cfg common.Config) ([]string, e
 	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 @@ func (r *roller) roll(ctx context.Context) error {
 	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 @@ func (r *roller) rollCommitMessage(
 	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)

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)
+	}
+}

tools/src/cts/expectations/update.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"sort"
 	"strings"
+	"time"
 
 	"dawn.googlesource.com/dawn/tools/src/container"
 	"dawn.googlesource.com/dawn/tools/src/cts/query"
@@ -337,10 +338,27 @@ func (u *updater) expectation(in Expectation, keep bool) []Expectation {
 	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 {
+			if ex := container.NewSet(in.Status...); len(ex) == 1 && ex.One() == string(result.Slow) {
-				u.diag(Note, in.Line, "all %d tests now pass", len(results))
+				// 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}