Skip to content

Commit

Permalink
cmd/testscript: add a -continue flag to not stop at the first error
Browse files Browse the repository at this point in the history
This helps with writing and running bug reproducers using testscript.
Since they are often meant to represent the desired behavior,
and not the current behavior, they are designed to fail.

However, some reproducers consist of multiple commands,
and cmd/testscript would stop at the first command to error.
When running a reproducer which is designed to fail,
we want to run the entire script and see all failures.

Add a `-continue` flag which does just that.
With it, `T.FailNow` still marks the test as failed,
but does not stop its execution via a panic.
We also make two small changes to behave correctly when we reach the end
of a script but `T.Failed` is true; we should "FAIL" rather than "PASS".
The previous code only reached that point if no errors had happened.

Note that we also altered `runT` to use pointer method receivers.
It used atomics to modify and read its `failed` field,
but without pointer receivers, `T.Failed` always returned false.
It appears that bug had been present for a long time.

Co-Authored-By: Paul Jolly <paul@myitcv.io>
  • Loading branch information
mvdan and myitcv committed Nov 23, 2022
1 parent c7b2344 commit fef0545
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 17 deletions.
50 changes: 33 additions & 17 deletions cmd/testscript/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ func mainerr() (retErr error) {
var envVars envVarsFlag
fUpdate := fs.Bool("u", false, "update archive file if a cmp fails")
fWork := fs.Bool("work", false, "print temporary work directory and do not remove when done")
fContinue := fs.Bool("continue", false, "continue running the script if an error occurs")
fVerbose := fs.Bool("v", false, "run tests verbosely")
fs.Var(&envVars, "e", "pass through environment variable to script (can appear multiple times)")
if err := fs.Parse(os.Args[1:]); err != nil {
Expand Down Expand Up @@ -101,10 +102,11 @@ func mainerr() (retErr error) {
}

tr := testRunner{
update: *fUpdate,
verbose: *fVerbose,
env: envVars.vals,
testWork: *fWork,
update: *fUpdate,
continueOnError: *fContinue,
verbose: *fVerbose,
env: envVars.vals,
testWork: *fWork,
}

dirNames := make(map[string]int)
Expand Down Expand Up @@ -139,6 +141,10 @@ type testRunner struct {
// updated in the case of any cmp failures.
update bool

// continueOnError indicates that T.FailNow should not panic, allowing the
// test script to continue running. Note that T is still marked as failed.
continueOnError bool

// verbose indicates the running of the script should be noisy.
verbose bool

Expand Down Expand Up @@ -271,8 +277,9 @@ func (tr *testRunner) run(runDir, filename string) error {
})
}

r := runT{
verbose: tr.verbose,
r := &runT{
continueOnError: tr.continueOnError,
verbose: tr.verbose,
}

func() {
Expand All @@ -286,6 +293,12 @@ func (tr *testRunner) run(runDir, filename string) error {
}
}()
testscript.RunT(r, p)

// When continueOnError is true, FailNow does not call panic(failedRun).
// We still want err to be set, as the script resulted in a failure.
if r.Failed() {
err = failedRun
}
}()

if err != nil {
Expand Down Expand Up @@ -334,44 +347,47 @@ 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
verbose bool
continueOnError bool
failed int32
}

func (r runT) Skip(is ...interface{}) {
func (r *runT) Skip(is ...interface{}) {
panic(skipRun)
}

func (r runT) Fatal(is ...interface{}) {
func (r *runT) Fatal(is ...interface{}) {
r.Log(is...)
r.FailNow()
}

func (r runT) Parallel() {
func (r *runT) Parallel() {
// No-op for now; we are currently only running a single script in a
// testscript instance.
}

func (r runT) Log(is ...interface{}) {
func (r *runT) Log(is ...interface{}) {
fmt.Print(is...)
}

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

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

func (r runT) Run(n string, f func(t testscript.T)) {
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
// will only have a single subtest.
f(r)
}

func (r runT) Verbose() bool {
func (r *runT) Verbose() bool {
return r.verbose
}
16 changes: 16 additions & 0 deletions cmd/testscript/testdata/continue.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# should support -continue
unquote file.txt

# Running with continue, the testscript command itself
# should fail, but we should see the results of executing
# both commands.
! testscript -continue file.txt
stdout 'grep banana in'
stdout 'no match for `banana` found in in'
stdout 'grep apple in'

-- file.txt --
>grep banana in
>grep apple in
>-- in --
>apple
7 changes: 7 additions & 0 deletions testscript/testscript.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,13 @@ Script:
}
ts.cmdWait(false, nil)

// If we reached here but T considers it failed, don't wipe the log and print "PASS".
// cmd/testscript behaves this way with the `-continue` flag; when turned on,
// its T.FailNow method does not panic, letting the test continue to run.
if hasFailed(ts.t) {
return
}

// Final phase ended.
rewind()
markTime()
Expand Down

0 comments on commit fef0545

Please sign in to comment.