Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
120497: roachtest: reset `failuresSuppressed` in `resetFailures` r=DarrylWong a=renatolabs

In cockroachdb#119535, we introduced a `resetFailures` method that is called after a test failure when the test runner identifies that a VM was preempted while the test was running. This function makes sure that the VM preemption error is the only one visible to the issue poster, ensuring that whenever a VM is preempted, the test failure is routed to Test Eng.

However, there was one situation where we still would not get the routing right: when the test times out. In this situation, we set the internal `failuresSuppressed` field to true; in other words, `resetFailures` followed by the VM preempmtion error would not have the desired effect of making the VM preemption error visible to the issue poster. While cockroachdb#119535 solves the most common source of test timeouts due to preemption (a bug in the monitor), tests can run arbitrary code and can timeout if a VM is preempted.

In this commit, we reset `failuresSuppressed` in `resetFailures` to make sure that the VM preemption error is taken into account when reporting failures in arbitrary test timeouts.

Fixes: cockroachdb#120381

Release note: None

Co-authored-by: Renato Costa <[email protected]>
  • Loading branch information
craig[bot] and renatolabs committed Mar 14, 2024
2 parents cf4f9e4 + fcf81d3 commit 24c998b
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions pkg/cmd/roachtest/test_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ func (t *testImpl) resetFailures() {
t.mu.Lock()
defer t.mu.Unlock()
t.mu.failures = nil
t.mu.failuresSuppressed = false
}

// We take the "squashed" error that contains information of all the errors for each failure.
Expand Down

0 comments on commit 24c998b

Please sign in to comment.