[tools][cts] Remove comment functionality
Removes comment functionality from the CTS roll command. Gerrit CLs
have a hard limit of 5000 comments per CL, which we're regularly
hitting at this point due to over a 1000 comments being posted per
patchset.
The majority of comments are not particularly useful (e.g. "all N tests
now pass" can be misleading due to limited data and the UPF will handle
automatic cleanup anyways). There are a few potentially useful comments
such as failures/passes/skips and the top 25 slowest tests, but these
will likely have to go away in the near future anyways since the roller
will not have the necessary information to calculate this after the
planned simplifications.
Bug: 371501714
Change-Id: I73e2680bd49f74ae451b07c5f4c383c84ff2a3c9
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/210316
Commit-Queue: Brian Sheedy <bsheedy@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Antonio Maiorano <amaiorano@google.com>
Auto-Submit: Brian Sheedy <bsheedy@google.com>
diff --git a/tools/src/cmd/cts/roll/roll.go b/tools/src/cmd/cts/roll/roll.go
index d0cf2ee..95a7855 100644
--- a/tools/src/cmd/cts/roll/roll.go
+++ b/tools/src/cmd/cts/roll/roll.go
@@ -42,7 +42,6 @@
"strconv"
"strings"
"sync"
- "text/tabwriter"
"time"
commonAuth "dawn.googlesource.com/dawn/tools/src/auth"
@@ -431,16 +430,10 @@
exInfo.results = result.Merge(exInfo.results, psResultsByExecutionMode[exInfo.executionMode])
exInfo.newExpectations = exInfo.expectations.Clone()
- diags, err := exInfo.newExpectations.Update(exInfo.results, testlist, r.flags.verbose)
+ _, err := exInfo.newExpectations.Update(exInfo.results, testlist, r.flags.verbose)
if err != nil {
return err
}
-
- // Post statistics and expectation diagnostics
- log.Printf("posting stats & diagnostics for %s...\n", exInfo.executionMode)
- if err := r.postComments(ps, exInfo.path, diags, exInfo.results); err != nil {
- return err
- }
}
// Otherwise, push the updated expectations, and try again
@@ -615,76 +608,6 @@
return msg.String()
}
-func (r *roller) postComments(ps gerrit.Patchset, path string, diags []expectations.Diagnostic, results result.List) error {
- fc := make([]gerrit.FileComment, len(diags))
- for i, d := range diags {
- var prefix string
- switch d.Severity {
- case expectations.Error:
- prefix = "🟥"
- case expectations.Warning:
- prefix = "🟨"
- case expectations.Note:
- prefix = "🟦"
- }
- fc[i] = gerrit.FileComment{
- Path: path,
- Side: gerrit.Left,
- Line: d.Line,
- Message: fmt.Sprintf("%v %v: %v", prefix, d.Severity, d.Message),
- }
- }
-
- sb := &strings.Builder{}
-
- {
- sb.WriteString("Tests by status:\n")
- counts := map[result.Status]int{}
- for _, r := range results {
- counts[r.Status] = counts[r.Status] + 1
- }
- type StatusCount struct {
- status result.Status
- count int
- }
- statusCounts := []StatusCount{}
- for s, n := range counts {
- if n > 0 {
- statusCounts = append(statusCounts, StatusCount{s, n})
- }
- }
- sort.Slice(statusCounts, func(i, j int) bool { return statusCounts[i].status < statusCounts[j].status })
- sb.WriteString("```\n")
- tw := tabwriter.NewWriter(sb, 0, 1, 0, ' ', 0)
- for _, sc := range statusCounts {
- fmt.Fprintf(tw, "%v:\t %v\n", sc.status, sc.count)
- }
- tw.Flush()
- sb.WriteString("```\n")
- }
- {
- sb.WriteString("Top 25 slowest tests:\n")
- sort.Slice(results, func(i, j int) bool {
- return results[i].Duration > results[j].Duration
- })
- const N = 25
- topN := results
- if len(topN) > N {
- topN = topN[:N]
- }
- sb.WriteString("```\n")
- for i, r := range topN {
- fmt.Fprintf(sb, "%3.1d: %v\n", i+1, r)
- }
- sb.WriteString("```\n")
- }
-
- if err := r.gerrit.Comment(ps, sb.String(), fc); err != nil {
- return fmt.Errorf("failed to post stats on change: %v", err)
- }
- return nil
-}
-
// findExistingRolls looks for all existing open CTS rolls by this user
func (r *roller) findExistingRolls() ([]gerrit.ChangeInfo, error) {
// Look for an existing gerrit change to update
diff --git a/tools/src/cts/expectations/update.go b/tools/src/cts/expectations/update.go
index 0099751..d161fb8 100644
--- a/tools/src/cts/expectations/update.go
+++ b/tools/src/cts/expectations/update.go
@@ -57,6 +57,8 @@
// Note: Validate() should be called before attempting to update the
// expectations. If Validate() returns errors, then Update() behaviour is
// undefined.
+// TODO(crbug.com/371501714): Remove the Diagnostics return value since it is
+// no longer used with the removal of commenting on CLs.
func (c *Content) Update(results result.List, testlist []query.Query, verbose bool) (Diagnostics, error) {
// Make a copy of the results. This code mutates the list.
results = append(result.List{}, results...)
