From 5cef7b9cc562d12b89ce4a636f27daabf735909d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Wed, 23 Nov 2022 11:07:26 +0000 Subject: [PATCH] cmd/testscript: add a -continue flag to not stop at the first error 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 --- cmd/testscript/main.go | 50 ++++++++++++++++++---------- cmd/testscript/testdata/continue.txt | 16 +++++++++ testscript/testscript.go | 7 ++++ 3 files changed, 56 insertions(+), 17 deletions(-) create mode 100644 cmd/testscript/testdata/continue.txt diff --git a/cmd/testscript/main.go b/cmd/testscript/main.go index d3b1d25f..4410a189 100644 --- a/cmd/testscript/main.go +++ b/cmd/testscript/main.go @@ -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 { @@ -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) @@ -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 @@ -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() { @@ -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 { @@ -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 } diff --git a/cmd/testscript/testdata/continue.txt b/cmd/testscript/testdata/continue.txt new file mode 100644 index 00000000..78f62114 --- /dev/null +++ b/cmd/testscript/testdata/continue.txt @@ -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 diff --git a/testscript/testscript.go b/testscript/testscript.go index 2b0cd75e..6f43bd6c 100644 --- a/testscript/testscript.go +++ b/testscript/testscript.go @@ -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()