Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testscript: print background command output on failure #148

Merged
merged 1 commit into from Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions cmd/testscript/main.go
Expand Up @@ -13,6 +13,7 @@ import (
"os/exec"
"path/filepath"
"strings"
"sync/atomic"

"github.com/rogpeppe/go-internal/goproxytest"
"github.com/rogpeppe/go-internal/gotooltest"
Expand Down Expand Up @@ -335,6 +336,7 @@ func renderFilename(filename string) string {
// runT implements testscript.T and is used in the call to testscript.Run
type runT struct {
verbose bool
failed int32
}

func (r runT) Skip(is ...interface{}) {
Expand All @@ -356,9 +358,14 @@ func (r runT) Log(is ...interface{}) {
}

func (r runT) FailNow() {
atomic.StoreInt32(&r.failed, 1)
panic(failedRun)
}

func (r runT) Failed() bool {
return atomic.LoadInt32(&r.failed) != 0
}

func (r runT) Run(n string, f func(t testscript.T)) {
// For now we we don't top/tail the run of a subtest. We are currently only
// running a single script in a testscript instance, which means that we
Expand Down
6 changes: 6 additions & 0 deletions testscript/cmd.go
Expand Up @@ -455,7 +455,10 @@ func (ts *TestScript) cmdWait(neg bool, args []string) {
if len(args) > 0 {
ts.Fatalf("usage: wait")
}
ts.waitBackground(true, neg)
}

func (ts *TestScript) waitBackground(checkStatus bool, neg bool) {
var stdouts, stderrs []string
for _, bg := range ts.background {
<-bg.wait
Expand All @@ -475,6 +478,9 @@ func (ts *TestScript) cmdWait(neg bool, args []string) {
stderrs = append(stderrs, cmdStderr)
}

if !checkStatus {
continue
}
if bg.cmd.ProcessState.Success() {
if bg.neg {
ts.Fatalf("unexpected command success")
Expand Down
25 changes: 22 additions & 3 deletions testscript/testscript.go
Expand Up @@ -174,6 +174,12 @@ type T interface {
Verbose() bool
}

// TFailed holds optional extra methods implemented on T.
// It's defined as a separate type for backward compatibility reasons.
type TFailed interface {
Failed() bool
}

type tshim struct {
*testing.T
}
Expand Down Expand Up @@ -387,10 +393,16 @@ func (ts *TestScript) run() {
for _, bg := range ts.background {
interruptProcess(bg.cmd.Process)
}
for _, bg := range ts.background {
<-bg.wait
if ts.t.Verbose() || hasFailed(ts.t) {
// In verbose mode or on test failure, we want to see what happened in the background
// processes too.
ts.waitBackground(false, false)
} else {
for _, bg := range ts.background {
<-bg.wait
}
ts.background = nil
}
ts.background = nil

markTime()
// Flush testScript log to testing.T log.
Expand Down Expand Up @@ -516,6 +528,13 @@ Script:
}
}

func hasFailed(t T) bool {
if t, ok := t.(TFailed); ok {
return t.Failed()
}
return false
}

func (ts *TestScript) applyScriptUpdates() {
if len(ts.scriptUpdates) == 0 {
return
Expand Down
6 changes: 6 additions & 0 deletions testscript/testscript_test.go
Expand Up @@ -389,6 +389,7 @@ type fakeT struct {
log bytes.Buffer
failMsgs []string
verbose bool
failed bool
}

var errAbort = errors.New("abort test")
Expand All @@ -398,6 +399,7 @@ func (t *fakeT) Skip(args ...interface{}) {
}

func (t *fakeT) Fatal(args ...interface{}) {
t.failed = true
t.failMsgs = append(t.failMsgs, fmt.Sprint(args...))
panic(errAbort)
}
Expand All @@ -419,3 +421,7 @@ func (t *fakeT) Run(name string, f func(T)) {
func (t *fakeT) Verbose() bool {
return t.verbose
}

func (t *fakeT) Failed() bool {
return t.failed
}