OSDN Git Service

Regres: Categorize crashes caused by debug macros
authorBen Clayton <bclayton@google.com>
Mon, 18 Mar 2019 15:13:48 +0000 (15:13 +0000)
committerBen Clayton <bclayton@google.com>
Tue, 19 Mar 2019 14:18:15 +0000 (14:18 +0000)
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>
tests/regres/main.go
tests/regres/testlist/testlist.go

index b4e246d..ce3361c 100644 (file)
@@ -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}
                        }
index ea24dcc..b79120c 100644 (file)
@@ -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