Skip to content

Commit

Permalink
cmd/testscript: remove redundant use of Failed (#257)
Browse files Browse the repository at this point in the history
The call to `T.Failed` and its associated comment are a
legacy of previous implementation. The comment isn't accurate and the
call isn't necessary (`FailNow` is always called even when `ContinueOnError`
is set). Deprecate the associated `TFailed` type rather than removing it.
  • Loading branch information
rogpeppe authored Jun 10, 2024
1 parent ee2fcaa commit 8300480
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 19 deletions.
13 changes: 0 additions & 13 deletions cmd/testscript/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"os/exec"
"path/filepath"
"strings"
"sync/atomic"

"github.com/rogpeppe/go-internal/goproxytest"
"github.com/rogpeppe/go-internal/gotooltest"
Expand Down Expand Up @@ -293,12 +292,6 @@ 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 @@ -348,7 +341,6 @@ 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 @@ -370,14 +362,9 @@ 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
3 changes: 1 addition & 2 deletions testscript/testscript.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,7 @@ type T interface {
Verbose() bool
}

// TFailed holds optional extra methods implemented on T.
// It's defined as a separate type for backward compatibility reasons.
// Deprecated: this type is unused.
type TFailed interface {
Failed() bool
}
Expand Down
4 changes: 0 additions & 4 deletions testscript/testscript_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,3 @@ 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
}

0 comments on commit 8300480

Please sign in to comment.