Skip to content

Commit

Permalink
configurable depth
Browse files Browse the repository at this point in the history
separate out context cancellation from addFailure
  • Loading branch information
Miral Gadani committed Feb 7, 2023
1 parent 8a4d0ad commit d83340b
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
21 changes: 12 additions & 9 deletions pkg/cmd/roachtest/test_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,44 +288,47 @@ func collectErrors(args []interface{}) []error {
// ATTENTION: Since this calls panic(errTestFatal), it should only be called
// from a test's closure. The test runner itself should never call this.
func (t *testImpl) Fatal(args ...interface{}) {
t.addFailureAndCancel("", args...)
t.addFailureAndCancel(1, "", args...)
panic(errTestFatal)
}

// Fatalf is like Fatal, but takes a format string.
func (t *testImpl) Fatalf(format string, args ...interface{}) {
t.addFailureAndCancel(format, args...)
t.addFailureAndCancel(1, format, args...)
panic(errTestFatal)
}

// FailNow implements the TestingT interface.
func (t *testImpl) FailNow() {
t.addFailureAndCancel("FailNow called")
t.addFailureAndCancel(1, "FailNow called")
panic(errTestFatal)
}

// Error implements the TestingT interface
func (t *testImpl) Error(args ...interface{}) {
t.addFailureAndCancel("", args...)
t.addFailureAndCancel(1, "", args...)
}

// Errorf implements the TestingT interface.
func (t *testImpl) Errorf(format string, args ...interface{}) {
t.addFailureAndCancel(format, args...)
t.addFailureAndCancel(1, format, args...)
}

func (t *testImpl) addFailureAndCancel(format string, args ...interface{}) {
t.addFailure(format, args...)
func (t *testImpl) addFailureAndCancel(depth int, format string, args ...interface{}) {
t.addFailure(depth+1, format, args...)
if t.mu.cancel != nil {
t.mu.cancel()
}
}

func (t *testImpl) addFailure(format string, args ...interface{}) {
// addFailure depth indicates how many stack frames to skip when reporting the
// site of the failure in logs. `0` will report the caller of addFailure, `1` the
// caller of the caller of addFailure, etc.
func (t *testImpl) addFailure(depth int, format string, args ...interface{}) {
if format == "" {
format = strings.Repeat(" %v", len(args))[1:]
}
reportFailure := newFailure(errors.NewWithDepthf(2, format, args...), collectErrors(args))
reportFailure := newFailure(errors.NewWithDepthf(depth+1, format, args...), collectErrors(args))

t.mu.Lock()
defer t.mu.Unlock()
Expand Down
10 changes: 4 additions & 6 deletions pkg/cmd/roachtest/test_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -984,10 +984,9 @@ func (r *testRunner) runTest(
}
t.L().Printf("tearing down after %s; see teardown.log", s)
case <-time.After(timeout):
// NB: we're intentionally not failing the test if it hasn't
// already. This will be done at the very end of this method,
// after we've collected artifacts.
t.addFailure("test timed out (%s)", timeout)
// NB: We're adding the timeout failure intentionally without cancelling the context
// to capture as much state as possible during artifact collection.
t.addFailure(0, "test timed out (%s)", timeout)
timedOut = true
}

Expand Down Expand Up @@ -1104,8 +1103,7 @@ func (r *testRunner) teardownTest(
// around so someone can poke at it.
_ = c.StopE(ctx, t.L(), option.DefaultStopOpts(), c.All())

// We add a failure without cancelling the context to allow for artifact collection.
// Now we can cancel the context.
// We previously added a timeout failure without cancellation, so we cancel here.
if t.mu.cancel != nil {
t.mu.cancel()
}
Expand Down

0 comments on commit d83340b

Please sign in to comment.