Regres: Categorize crashes caused by debug macros

tests/regres/main.go

@@ -387,16 +387,17 @@ func (r *regres) updateTestLists(client *gerrit.Client) error {
 	test := r.newTest(headHash)
 	defer test.cleanup()
 
+	// Always need to checkout the change.
+	if err := test.checkout(); err != nil {
+		return cause.Wrap(err, "Failed to checkout '%s'", headHash)
+	}
+
+	// Load the test lists.
 	testLists, err := test.loadTestLists(fullTestListRelPath)
 	if err != nil {
 		return cause.Wrap(err, "Failed to load full test lists for '%s'", headHash)
 	}
 
-	// Couldn't load cached results. Have to build them.
-	if err := test.checkout(); err != nil {
-		return cause.Wrap(err, "Failed to checkout '%s'", headHash)
-	}
-
 	// Build the change.
 	if err := test.build(); err != nil {
 		return cause.Wrap(err, "Failed to build '%s'", headHash)
@@ -421,23 +422,17 @@ func (r *regres) updateTestLists(client *gerrit.Client) error {
 	}
 
 	log.Println("Checking for existing test list")
-	changes, _, err := client.Changes.QueryChanges(&gerrit.QueryChangeOptions{
-		QueryOptions: gerrit.QueryOptions{
-			Query: []string{fmt.Sprintf(`status:open+owner:"%v"`, r.gerritEmail)},
-			Limit: 1,
-		},
-	})
+	existingChange, err := r.findTestListChange(client)
 	if err != nil {
-		return cause.Wrap(err, "Failed to checking for existing test list")
+		return err
 	}
 
 	commitMsg := strings.Builder{}
 	commitMsg.WriteString(consts.TestListUpdateCommitSubjectPrefix + headHash.String()[:8])
-	if results != nil && len(*changes) > 0 {
+	if existingChange != nil {
 		// Reuse gerrit change ID if there's already a change up for review.
-		id := (*changes)[0].ChangeID
 		commitMsg.WriteString("\n\n")
-		commitMsg.WriteString("Change-Id: " + id)
+		commitMsg.WriteString("Change-Id: " + existingChange.ChangeID + "\n")
 	}
 
 	if err := git.Commit(test.srcDir, commitMsg.String(), git.CommitFlags{
@@ -460,9 +455,86 @@ func (r *regres) updateTestLists(client *gerrit.Client) error {
 		log.Println("Test results posted for review")
 	}
 
+	change, err := r.findTestListChange(client)
+	if err != nil {
+		return err
+	}
+
+	if err := r.postMostCommonFailures(client, change, results); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// postMostCommonFailures posts the most common failure cases as a review
+// comment on the given change.
+func (r *regres) postMostCommonFailures(client *gerrit.Client, change *gerrit.ChangeInfo, results *CommitTestResults) error {
+	const limit = 25
+
+	failures := results.commonFailures()
+	if len(failures) > limit {
+		failures = failures[:limit]
+	}
+	sb := strings.Builder{}
+	sb.WriteString(fmt.Sprintf("Top %v most common failures:\n", len(failures)))
+	for _, f := range failures {
+		lines := strings.Split(f.error, "\n")
+		if len(lines) == 1 {
+			line := lines[0]
+			if line != "" {
+				sb.WriteString(fmt.Sprintf("  %d occurrences: %v: %v\n", f.count, f.status, line))
+			} else {
+				sb.WriteString(fmt.Sprintf("  %d occurrences: %v\n", f.count, f.status))
+			}
+		} else {
+			sb.WriteString(fmt.Sprintf("  %d occurrences: %v:\n", f.count, f.status))
+			for _, l := range lines {
+				sb.WriteString("    > ")
+				sb.WriteString(l)
+				sb.WriteString("\n")
+			}
+		}
+	}
+	msg := sb.String()
+
+	if r.dryRun {
+		log.Printf("DRY RUN: add most common failures to '%v':\n%v\n", change.ChangeID, msg)
+	} else {
+		log.Printf("Posting most common failures to '%s'\n", change.ChangeID)
+		_, _, err := client.Changes.SetReview(change.ChangeID, change.CurrentRevision, &gerrit.ReviewInput{
+			Message: msg,
+			Tag:     "autogenerated:regress",
+		})
+		if err != nil {
+			return cause.Wrap(err, "Failed to post comments on change '%s'", change.ChangeID)
+		}
+	}
 	return nil
 }
 
+func (r *regres) findTestListChange(client *gerrit.Client) (*gerrit.ChangeInfo, error) {
+	log.Println("Checking for existing test list change")
+	changes, _, err := client.Changes.QueryChanges(&gerrit.QueryChangeOptions{
+		QueryOptions: gerrit.QueryOptions{
+			Query: []string{fmt.Sprintf(`status:open+owner:"%v"`, r.gerritEmail)},
+			Limit: 1,
+		},
+		ChangeOptions: gerrit.ChangeOptions{
+			AdditionalFields: []string{"CURRENT_REVISION"},
+		},
+	})
+	if err != nil {
+		return nil, cause.Wrap(err, "Failed to checking for existing test list")
+	}
+	if len(*changes) > 0 {
+		// TODO: This currently assumes that only change changes from
+		// gerritEmail are test lists updates. This may not always be true.
+		return &(*changes)[0], nil
+	}
+	return nil, nil
+}
+
 // changeInfo holds the important information about a single, open change in
 // gerrit.
 type changeInfo struct {
@@ -827,6 +899,33 @@ func (r *CommitTestResults) save(path string) error {
 	return nil
 }
 
+type testStatusAndError struct {
+	status testlist.Status
+	error  string
+}
+
+type commonFailure struct {
+	count int
+	testStatusAndError
+}
+
+func (r *CommitTestResults) commonFailures() []commonFailure {
+	failures := map[testStatusAndError]int{}
+	for _, test := range r.Tests {
+		if !test.Status.Failing() {
+			continue
+		}
+		key := testStatusAndError{test.Status, test.Err}
+		failures[key] = failures[key] + 1
+	}
+	out := make([]commonFailure, 0, len(failures))
+	for failure, count := range failures {
+		out = append(out, commonFailure{count, failure})
+	}
+	sort.Slice(out, func(i, j int) bool { return out[i].count > out[j].count })
+	return out
+}
+
 // compare returns a string describing all differences between two
 // CommitTestResults. This string is used as the report message posted to the
 // gerrit code review.
@@ -903,6 +1002,10 @@ func compare(old, new *CommitTestResults) string {
 		{"                 Pass", testlist.Pass},
 		{"                 Fail", testlist.Fail},
 		{"              Timeout", testlist.Timeout},
+		{"      UNIMPLEMENTED()", testlist.Unimplemented},
+		{"        UNREACHABLE()", testlist.Unreachable},
+		{"             ASSERT()", testlist.Assert},
+		{"              ABORT()", testlist.Abort},
 		{"                Crash", testlist.Crash},
 		{"        Not Supported", testlist.NotSupported},
 		{"Compatibility Warning", testlist.CompatibilityWarning},
@@ -978,8 +1081,18 @@ func (r TestResult) String() string {
 	return fmt.Sprintf("%s: %s", r.Test, r.Status)
 }
 
-// Regular expression to parse the output of a dEQP test.
-var parseRE = regexp.MustCompile(`(Fail|Pass|NotSupported|CompatibilityWarning|QualityWarning) \(([\s\S]*)\)`)
+var (
+	// Regular expression to parse the output of a dEQP test.
+	deqpRE = regexp.MustCompile(`(Fail|Pass|NotSupported|CompatibilityWarning|QualityWarning) \(([^\)]*)\)`)
+	// Regular expression to parse a test that failed due to UNIMPLEMENTED()
+	unimplementedRE = regexp.MustCompile(`[^\n]*UNIMPLEMENTED:[^\n]*`)
+	// Regular expression to parse a test that failed due to UNREACHABLE()
+	unreachableRE = regexp.MustCompile(`[^\n]*UNREACHABLE:[^\n]*`)
+	// Regular expression to parse a test that failed due to ASSERT()
+	assertRE = regexp.MustCompile(`[^\n]*ASSERT\([^\)]*\)[^\n]*`)
+	// Regular expression to parse a test that failed due to ABORT()
+	abortRE = regexp.MustCompile(`[^\n]*ABORT:[^\n]*`)
+)
 
 // deqpTestRoutine repeatedly runs the dEQP test executable exe with the tests
 // taken from tests. The output of the dEQP test is parsed, and the test result
@@ -987,6 +1100,7 @@ var parseRE = regexp.MustCompile(`(Fail|Pass|NotSupported|CompatibilityWarning|Q
 // deqpTestRoutine only returns once the tests chan has been closed.
 // deqpTestRoutine does not close the results chan.
 func (t *test) deqpTestRoutine(exe string, tests <-chan string, results chan<- TestResult) {
+nextTest:
 	for name := range tests {
 		// log.Printf("Running test '%s'\n", name)
 		env := []string{
@@ -996,24 +1110,43 @@ func (t *test) deqpTestRoutine(exe string, tests <-chan string, results chan<- T
 			"LIBC_FATAL_STDERR_=1", // Put libc explosions into logs.
 		}
 
-		out, err := shell.Exec(testTimeout, exe, filepath.Dir(exe), env, "--deqp-surface-type=pbuffer", "-n="+name)
+		outRaw, err := shell.Exec(testTimeout, exe, filepath.Dir(exe), env, "--deqp-surface-type=pbuffer", "-n="+name)
+		out := string(outRaw)
+		out = strings.ReplaceAll(out, t.srcDir, "<SwiftShader>")
+		out = strings.ReplaceAll(out, exe, "<dEQP>")
 		switch err.(type) {
 		default:
+			for _, test := range []struct {
+				re *regexp.Regexp
+				s  testlist.Status
+			}{
+				{unimplementedRE, testlist.Unimplemented},
+				{unreachableRE, testlist.Unreachable},
+				{assertRE, testlist.Assert},
+				{abortRE, testlist.Abort},
+			} {
+				if s := test.re.FindString(out); s != "" {
+					results <- TestResult{
+						Test:   name,
+						Status: test.s,
+						Err:    s,
+					}
+					continue nextTest
+				}
+			}
 			results <- TestResult{
 				Test:   name,
 				Status: testlist.Crash,
-				Err:    cause.Wrap(err, string(out)).Error(),
 			}
 		case shell.ErrTimeout:
 			results <- TestResult{
 				Test:   name,
 				Status: testlist.Timeout,
-				Err:    cause.Wrap(err, string(out)).Error(),
 			}
 		case nil:
-			toks := parseRE.FindStringSubmatch(string(out))
+			toks := deqpRE.FindStringSubmatch(out)
 			if len(toks) < 3 {
-				err := fmt.Sprintf("Couldn't parse test '%v' output:\n%s", name, string(out))
+				err := fmt.Sprintf("Couldn't parse test '%v' output:\n%s", name, out)
 				log.Println("Warning: ", err)
 				results <- TestResult{Test: name, Status: testlist.Fail, Err: err}
 				continue
@@ -1034,7 +1167,7 @@ func (t *test) deqpTestRoutine(exe string, tests <-chan string, results chan<- T
 				}
 				results <- TestResult{Test: name, Status: testlist.Fail, Err: err}
 			default:
-				err := fmt.Sprintf("Couldn't parse test output:\n%s", string(out))
+				err := fmt.Sprintf("Couldn't parse test output:\n%s", out)
 				log.Println("Warning: ", err)
 				results <- TestResult{Test: name, Status: testlist.Fail, Err: err}
 			}

tests/regres/testlist/testlist.go

@@ -159,6 +159,14 @@ const (
 	Timeout = Status("TIMEOUT")
 	// Crash is the status of a test that crashed.
 	Crash = Status("CRASH")
+	// Unimplemented is the status of a test that failed with UNIMPLEMENTED().
+	Unimplemented = Status("UNIMPLEMENTED")
+	// Unreachable is the status of a test that failed with UNREACHABLE().
+	Unreachable = Status("UNREACHABLE")
+	// Assert is the status of a test that failed with ASSERT() or ASSERT_MSG().
+	Assert = Status("ASSERT")
+	// Abort is the status of a test that failed with ABORT().
+	Abort = Status("ABORT")
 	// NotSupported is the status of a test feature not supported by the driver.
 	NotSupported = Status("NOT_SUPPORTED")
 	// CompatibilityWarning is the status passing test with a warning.
@@ -168,12 +176,24 @@ const (
 )
 
 // Statuses is the full list of status types
-var Statuses = []Status{Pass, Fail, Timeout, Crash, NotSupported, CompatibilityWarning, QualityWarning}
+var Statuses = []Status{
+	Pass,
+	Fail,
+	Timeout,
+	Crash,
+	Unimplemented,
+	Unreachable,
+	Assert,
+	Abort,
+	NotSupported,
+	CompatibilityWarning,
+	QualityWarning,
+}
 
 // Failing returns true if the task status requires fixing.
 func (s Status) Failing() bool {
 	switch s {
-	case Fail, Timeout, Crash:
+	case Fail, Timeout, Crash, Unimplemented, Unreachable, Assert:
 		return true
 	default:
 		return false