Regres: Don't attempt to build failing changes forever.
If a test failed to build, an error was logged to stdout, but the error was not posted to the change. This meant the change would be picked up by regres again, and the process would repeat ad-infinitum.
Posting of the build error used to work, but was broken by bfaf4e825b.
Change-Id: I1e59f0d2e5ee26eb002aa2c596dc5491e48a9c87
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/27169
Tested-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
diff --git a/tests/regres/main.go b/tests/regres/main.go
index 9766f09..7db4876 100644
--- a/tests/regres/main.go
+++ b/tests/regres/main.go
@@ -248,6 +248,7 @@
if err != nil {
log.Println(cause.Wrap(err, "Failed to test changelist '%s'", change.latest))
time.Sleep(time.Minute)
+ change.pending = false
continue
}
@@ -336,14 +337,8 @@
return results, testlists, nil // Use cached results
}
- if err := test.build(); err != nil {
- return nil, nil, cause.Wrap(err, "Failed to build '%s'", change.latest)
- }
-
- results, err := test.run(testlists)
- if err != nil {
- return nil, nil, cause.Wrap(err, "Failed to test '%s'", change.latest)
- }
+ // Build the change and test it.
+ results := test.buildAndRun(testlists)
// Cache the results for future tests
if err := results.save(cachePath); err != nil {
@@ -369,16 +364,8 @@
return nil, cause.Wrap(err, "Failed to checkout '%s'", change.parent)
}
- // Build the parent change.
- if err := test.build(); err != nil {
- return nil, cause.Wrap(err, "Failed to build '%s'", change.parent)
- }
-
- // Run the tests on the parent change.
- results, err := test.run(testlists)
- if err != nil {
- return nil, cause.Wrap(err, "Failed to test '%s'", change.parent)
- }
+ // Build the parent change and test it.
+ results := test.buildAndRun(testlists)
// Store the results of the parent change to the cache.
if err := results.save(cachePath); err != nil {
@@ -624,6 +611,27 @@
return nil
}
+// buildAndRun calls t.build() followed by t.run(). Errors are logged and
+// reported in the returned CommitTestResults.Error field.
+func (t *test) buildAndRun(testLists testlist.Lists) *CommitTestResults {
+ // Build the parent change.
+ if err := t.build(); err != nil {
+ msg := fmt.Sprintf("Failed to build '%s'", t.commit)
+ log.Println(cause.Wrap(err, msg))
+ return &CommitTestResults{Error: msg}
+ }
+
+ // Run the tests on the parent change.
+ results, err := t.run(testLists)
+ if err != nil {
+ msg := fmt.Sprintf("Failed to test change '%s'", t.commit)
+ log.Println(cause.Wrap(err, msg))
+ return &CommitTestResults{Error: msg}
+ }
+
+ return results
+}
+
// build builds the SwiftShader source into t.buildDir.
func (t *test) build() error {
log.Printf("Building '%s'\n", t.commit)
@@ -704,7 +712,6 @@
out := CommitTestResults{
Version: dataVersion,
- Built: true,
Tests: map[string]TestResult{},
}
@@ -776,7 +783,7 @@
// results.
type CommitTestResults struct {
Version int
- Built bool
+ Error string
Tests map[string]TestResult
Duration time.Duration
}
@@ -820,13 +827,11 @@
// CommitTestResults. This string is used as the report message posted to the
// gerrit code review.
func compare(old, new *CommitTestResults) string {
- switch {
- case !old.Built && !new.Built:
- return "Build continues to be broken."
- case old.Built && !new.Built:
- return "Build broken."
- case !old.Built && !new.Built:
- return "Build now fixed. Cannot compare against broken parent."
+ if old.Error != "" {
+ return old.Error
+ }
+ if new.Error != "" {
+ return new.Error
}
oldStatusCounts, newStatusCounts := map[testlist.Status]int{}, map[testlist.Status]int{}
