Commit 665ee574 by Ben Clayton

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/+/27169Tested-by: 's avatarBen Clayton <bclayton@google.com> Reviewed-by: 's avatarNicolas Capens <nicolascapens@google.com>
parent 24cb99d4
...@@ -248,6 +248,7 @@ func (r *regres) run() error { ...@@ -248,6 +248,7 @@ func (r *regres) run() error {
if err != nil { if err != nil {
log.Println(cause.Wrap(err, "Failed to test changelist '%s'", change.latest)) log.Println(cause.Wrap(err, "Failed to test changelist '%s'", change.latest))
time.Sleep(time.Minute) time.Sleep(time.Minute)
change.pending = false
continue continue
} }
...@@ -336,14 +337,8 @@ func (r *regres) testLatest(change *changeInfo) (*CommitTestResults, testlist.Li ...@@ -336,14 +337,8 @@ func (r *regres) testLatest(change *changeInfo) (*CommitTestResults, testlist.Li
return results, testlists, nil // Use cached results return results, testlists, nil // Use cached results
} }
if err := test.build(); err != nil { // Build the change and test it.
return nil, nil, cause.Wrap(err, "Failed to build '%s'", change.latest) results := test.buildAndRun(testlists)
}
results, err := test.run(testlists)
if err != nil {
return nil, nil, cause.Wrap(err, "Failed to test '%s'", change.latest)
}
// Cache the results for future tests // Cache the results for future tests
if err := results.save(cachePath); err != nil { if err := results.save(cachePath); err != nil {
...@@ -369,16 +364,8 @@ func (r *regres) testParent(change *changeInfo, testlists testlist.Lists) (*Comm ...@@ -369,16 +364,8 @@ func (r *regres) testParent(change *changeInfo, testlists testlist.Lists) (*Comm
return nil, cause.Wrap(err, "Failed to checkout '%s'", change.parent) return nil, cause.Wrap(err, "Failed to checkout '%s'", change.parent)
} }
// Build the parent change. // Build the parent change and test it.
if err := test.build(); err != nil { results := test.buildAndRun(testlists)
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)
}
// Store the results of the parent change to the cache. // Store the results of the parent change to the cache.
if err := results.save(cachePath); err != nil { if err := results.save(cachePath); err != nil {
...@@ -624,6 +611,27 @@ func (t *test) checkout() error { ...@@ -624,6 +611,27 @@ func (t *test) checkout() error {
return nil 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. // build builds the SwiftShader source into t.buildDir.
func (t *test) build() error { func (t *test) build() error {
log.Printf("Building '%s'\n", t.commit) log.Printf("Building '%s'\n", t.commit)
...@@ -704,7 +712,6 @@ func (t *test) run(testLists testlist.Lists) (*CommitTestResults, error) { ...@@ -704,7 +712,6 @@ func (t *test) run(testLists testlist.Lists) (*CommitTestResults, error) {
out := CommitTestResults{ out := CommitTestResults{
Version: dataVersion, Version: dataVersion,
Built: true,
Tests: map[string]TestResult{}, Tests: map[string]TestResult{},
} }
...@@ -776,7 +783,7 @@ func (t *test) resultsCachePath(testLists testlist.Lists) string { ...@@ -776,7 +783,7 @@ func (t *test) resultsCachePath(testLists testlist.Lists) string {
// results. // results.
type CommitTestResults struct { type CommitTestResults struct {
Version int Version int
Built bool Error string
Tests map[string]TestResult Tests map[string]TestResult
Duration time.Duration Duration time.Duration
} }
...@@ -820,13 +827,11 @@ func (r *CommitTestResults) save(path string) error { ...@@ -820,13 +827,11 @@ func (r *CommitTestResults) save(path string) error {
// CommitTestResults. This string is used as the report message posted to the // CommitTestResults. This string is used as the report message posted to the
// gerrit code review. // gerrit code review.
func compare(old, new *CommitTestResults) string { func compare(old, new *CommitTestResults) string {
switch { if old.Error != "" {
case !old.Built && !new.Built: return old.Error
return "Build continues to be broken." }
case old.Built && !new.Built: if new.Error != "" {
return "Build broken." return new.Error
case !old.Built && !new.Built:
return "Build now fixed. Cannot compare against broken parent."
} }
oldStatusCounts, newStatusCounts := map[testlist.Status]int{}, map[testlist.Status]int{} oldStatusCounts, newStatusCounts := map[testlist.Status]int{}, map[testlist.Status]int{}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment