tools/cts: Bunch of fixes for 'cts roll'

• Strip 'release-x64' tags.
  These are always paired with 'release', and all
  other tests just have the 'release' tag. This
  means that results with the 'release' +
  'release-x64' tags are a subset of just the
  'release' tags, leading to many broken
  assumptions in the expectation update.

• Don't consider tests with the error reason
  'asyncio.exceptions.TimeoutError' as slow, but
  as a failure. The CTS expectation updater will
  already mark tests exceeding the config
  threshold as slow. This reduces the amount of
  data requested from resultdb, and prevents
  failed rolls when results are actually
  deadlocking.

• Fix inferring of patchset in GetResults().
  LatestPatchest() was called but was assigned to
  a nested scope.

Bug: dawn:1401
Change-Id: I57b2c9a029b31430d63a056f7b13c4bf1bc5b437
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/88450
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>

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

@@ -53,10 +53,16 @@ type Config struct {
 	// Builders is a map of builder name (as displayed in the UI) to buildbucket
 	// builder information.
 	Builders map[string]buildbucket.Builder
-	// TagAliases is a list of tags which are treated as equivalent.
-	// For example, some GPUs are similar enough to be considered equivalent.
-	// See crbug.com/dawn/1387 for more information.
-	TagAliases [][]string
+	// Tags holds configuration data for cleaning result tags before processing
+	Tag struct {
+		// TagAliases is a list of tags which are treated as equivalent.
+		// For example, some GPUs are similar enough to be considered equivalent.
+		// See crbug.com/dawn/1387 for more information.
+		Aliases [][]string
+		// Remove holds tags that should be removed before processing.
+		// See crbug.com/dawn/1401 for more information.
+		Remove []string
+	}
 	// Sheets holds information about the Google Sheets document used for
 	// tracking CTS statistics.
 	Sheets struct {

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

@@ -112,7 +112,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)
@@ -220,12 +220,6 @@ func GetResults(
 			}
 		}
 
-		if fr := rpb.GetFailureReason(); fr != nil {
-			if strings.Contains(fr.GetPrimaryErrorMessage(), "asyncio.exceptions.TimeoutError") {
-				status = result.Slow
-			}
-		}
-
 		duration := rpb.GetDuration().AsDuration()
 		if status == result.Pass && duration > cfg.Test.SlowThreshold {
 			status = result.Slow
@@ -247,11 +241,8 @@ func GetResults(
 		return nil, err
 	}
 
-	// Expand any aliased tags
-	ExpandAliasedTags(cfg, results)
-
-	// Remove any duplicates from the results.
-	results = results.ReplaceDuplicates(result.Deduplicate)
+	// Expand aliased tags, remove specific tags
+	CleanTags(cfg, &results)
 
 	results.Sort()
 	return results, err
@@ -329,21 +320,38 @@ func MostRecentResultsForChange(
 	return nil, gerrit.Patchset{}, fmt.Errorf("no builds found for change %v", change)
 }
 
-// ExpandAliasedTags modifies each result so that tags which are found in
+// CleanTags modifies each result so that tags which are found in
 // cfg.TagAliases are expanded to include all the tag aliases.
-// This is bodge for crbug.com/dawn/1387.
-func ExpandAliasedTags(cfg Config, results result.List) {
-	// Build the result sets
-	sets := make([]result.Tags, len(cfg.TagAliases))
-	for i, l := range cfg.TagAliases {
-		sets[i] = result.NewTags(l...)
+// Tags in cfg.Tag.Remove are also removed.
+// Finally, duplicate results are removed by erring towards Failure.
+// See: crbug.com/dawn/1387, crbug.com/dawn/1401
+func CleanTags(cfg Config, results *result.List) {
+	remove := result.NewTags(cfg.Tag.Remove...)
+	aliases := make([]result.Tags, len(cfg.Tag.Aliases))
+	for i, l := range cfg.Tag.Aliases {
+		aliases[i] = result.NewTags(l...)
 	}
 	// Expand the result tags for the aliased tag sets
-	for _, r := range results {
-		for _, set := range sets {
+	for _, r := range *results {
+		for _, set := range aliases {
 			if r.Tags.ContainsAny(set) {
 				r.Tags.AddAll(set)
 			}
 		}
+		r.Tags.RemoveAll(remove)
 	}
+	*results = results.ReplaceDuplicates(func(s result.Statuses) result.Status {
+		// If all results have the same status, then use that.
+		if len(s) == 1 {
+			return s.One()
+		}
+		// Mixed statuses. Replace with something appropriate.
+		switch {
+		case s.Contains(result.Crash):
+			return result.Crash
+		case s.Contains(result.Abort):
+			return result.Abort
+		}
+		return result.Failure
+	})
 }

tools/src/cmd/cts/config.json

@@ -39,13 +39,19 @@
       "Builder": "linux-dawn-rel"
     }
   },
-  "TagAliases": [
-    [
-      // crbug.com/dawn/1387
-      "intel-0x5912",
-      "intel-0x3e92"
-    ]
-  ],
+  "Tag": {
+    "Aliases": [
+      [
+        // crbug.com/dawn/1387
+        "intel-0x5912",
+        "intel-0x3e92"
+      ]
+    ],
+    "Remove": [
+      // crbug.com/dawn/1401
+      "release-x64"
+    ],
+  },
   "Sheets": {
     // Spreadsheet to export results data to
     // https://docs.google.com/spreadsheets/d/1OFsh-r_njG5pKDwjL1HOvLJKDRC4FgO-LE9Kw7WPQcc

tools/src/resultsdb/resultsdb.go

@@ -72,7 +72,7 @@ func (r *ResultsDB) QueryTestResults(
 				TestIdRegexp: filterRegex,
 			},
 			ReadMask: &fieldmaskpb.FieldMask{Paths: []string{
-				"test_id", "status", "tags", "failure_reason", "duration",
+				"test_id", "status", "tags", "duration",
 			}},
 			PageSize:  1000, // Maximum page size.
 			PageToken: pageToken,