Regres: Categorize crashes caused by debug macros
List most common failures on the daily report.
Change-Id: Ia07f73601727f71e5f2abee7c40886e2a05209bb
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/27472
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Tested-by: Ben Clayton <bclayton@google.com>
diff --git a/tests/regres/main.go b/tests/regres/main.go
index b4e246d..ce3361c 100644
--- a/tests/regres/main.go
+++ b/tests/regres/main.go
@@ -387,16 +387,17 @@
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 @@
}
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 @@
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 @@
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 @@
{" 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 @@
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 @@
// 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 @@
"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 @@
}
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}
}
diff --git a/tests/regres/testlist/testlist.go b/tests/regres/testlist/testlist.go
index ea24dcc..b79120c 100644
--- a/tests/regres/testlist/testlist.go
+++ b/tests/regres/testlist/testlist.go
@@ -159,6 +159,14 @@
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 @@
)
// 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