From b143f3fd64d396b115ee7a4f98e64d3ef3280b77 Mon Sep 17 00:00:00 2001
From: Roger Peppe <rogpeppe@gmail.com>
Date: Tue, 11 Jun 2024 09:18:30 +0100
Subject: [PATCH] testscript: add Config.Files (#258)

This makes it possible to pass an arbitrary set of testscript
files to be run instead of just a directory, making it possible
for the testscript command to pass its command line arguments
directly.

In order to check that all the files are actually tested, we need
to make the test harness implement independent subtest failure,
and it's useful to see the name of the test too so that we
can see the name disambiguation logic at work, which
makes for changes to some of the other tests too.

Note that the name deduping logic is somewhat improved from
similar logic in cmd/testscript, in that it is always guaranteed
to produce unique names even in the presence of filenames
that look like deduped names.
---
 cmd/testscript/testdata/work.txt              |  2 +-
 testscript/testdata/big_diff.txt              |  1 +
 testscript/testdata/long_diff.txt             |  1 +
 .../testdata/testscript_explicit_files.txt    | 26 +++++++++
 testscript/testdata/testscript_logging.txt    |  4 ++
 .../testscript_stdout_stderr_error.txt        |  1 +
 testscript/testscript.go                      | 58 ++++++++++++++-----
 testscript/testscript_test.go                 | 40 +++++++++++--
 8 files changed, 111 insertions(+), 22 deletions(-)
 create mode 100644 testscript/testdata/testscript_explicit_files.txt

diff --git a/cmd/testscript/testdata/work.txt b/cmd/testscript/testdata/work.txt
index 021705cb..dfd19f04 100644
--- a/cmd/testscript/testdata/work.txt
+++ b/cmd/testscript/testdata/work.txt
@@ -13,7 +13,7 @@ stderr '^temporary work directory: \Q'$WORK'\E[/\\]\.tmp[/\\]'
 stderr '^temporary work directory for file.txt: \Q'$WORK'\E[/\\]\.tmp[/\\]'
 stderr '^temporary work directory for dir[/\\]file.txt: \Q'$WORK'\E[/\\]\.tmp[/\\]'
 expandone $WORK/.tmp/testscript*/file.txt/script.txtar
-expandone $WORK/.tmp/testscript*/file.txt1/script.txtar
+expandone $WORK/.tmp/testscript*/file.txt#1/script.txtar
 
 -- file.txt --
 >exec true
diff --git a/testscript/testdata/big_diff.txt b/testscript/testdata/big_diff.txt
index f3effc00..21efffbf 100644
--- a/testscript/testdata/big_diff.txt
+++ b/testscript/testdata/big_diff.txt
@@ -7,6 +7,7 @@ env
 cmpenv stdout stdout.golden
 
 -- stdout.golden --
+** RUN script **
 > cmp a b
 diff a b
 --- a
diff --git a/testscript/testdata/long_diff.txt b/testscript/testdata/long_diff.txt
index ea42157c..a55b86ca 100644
--- a/testscript/testdata/long_diff.txt
+++ b/testscript/testdata/long_diff.txt
@@ -110,6 +110,7 @@ cmpenv stdout stdout.golden
 >a
 >a
 -- stdout.golden --
+** RUN script **
 > cmp a b
 diff a b
 --- a
diff --git a/testscript/testdata/testscript_explicit_files.txt b/testscript/testdata/testscript_explicit_files.txt
new file mode 100644
index 00000000..1de030b7
--- /dev/null
+++ b/testscript/testdata/testscript_explicit_files.txt
@@ -0,0 +1,26 @@
+# Check that we can pass an explicit set of files to be tested.
+! testscript -files foo.txtar x/bar.txtar y/bar.txtar 'y/bar#1.txtar'
+cmpenv stdout expect-stdout
+-- expect-stdout --
+** RUN foo **
+PASS
+** RUN bar **
+PASS
+** RUN bar#1 **
+> echoandexit 1 '' 'bar#1 failure'
+[stderr]
+bar#1 failure
+FAIL: $$WORK${/}y${/}bar.txtar:1: told to exit with code 1
+** RUN bar#1#1 **
+> echoandexit 1 '' 'bar#1#1 failure'
+[stderr]
+bar#1#1 failure
+FAIL: $$WORK${/}y${/}bar#1.txtar:1: told to exit with code 1
+-- foo.txtar --
+echoandexit 0 '' 'foo failure'
+-- x/bar.txtar --
+echoandexit 0 '' 'bar failure'
+-- y/bar.txtar --
+echoandexit 1 '' 'bar#1 failure'
+-- y/bar#1.txtar --
+echoandexit 1 '' 'bar#1#1 failure'
diff --git a/testscript/testdata/testscript_logging.txt b/testscript/testdata/testscript_logging.txt
index 60975fee..f0c0777e 100644
--- a/testscript/testdata/testscript_logging.txt
+++ b/testscript/testdata/testscript_logging.txt
@@ -33,6 +33,7 @@ printargs section5
 status 1
 
 -- expect-stdout.txt --
+** RUN testscript **
 # comment 1 (0.000s)
 # comment 2 (0.000s)
 # comment 3 (0.000s)
@@ -43,6 +44,7 @@ status 1
 [exit status 1]
 FAIL: $$WORK${/}scripts${/}testscript.txt:9: unexpected command failure
 -- expect-stdout-v.txt --
+** RUN testscript **
 # comment 1 (0.000s)
 > printargs section1
 [stdout]
@@ -59,6 +61,7 @@ FAIL: $$WORK${/}scripts${/}testscript.txt:9: unexpected command failure
 [exit status 1]
 FAIL: $$WORK${/}scripts${/}testscript.txt:9: unexpected command failure
 -- expect-stdout-c.txt --
+** RUN testscript **
 # comment 1 (0.000s)
 # comment 2 (0.000s)
 # comment 3 (0.000s)
@@ -80,6 +83,7 @@ FAIL: $$WORK${/}scripts${/}testscript.txt:9: unexpected command failure
 [exit status 1]
 FAIL: $$WORK${/}scripts${/}testscript.txt:16: unexpected command failure
 -- expect-stdout-vc.txt --
+** RUN testscript **
 # comment 1 (0.000s)
 > printargs section1
 [stdout]
diff --git a/testscript/testdata/testscript_stdout_stderr_error.txt b/testscript/testdata/testscript_stdout_stderr_error.txt
index a69c7d01..172e056b 100644
--- a/testscript/testdata/testscript_stdout_stderr_error.txt
+++ b/testscript/testdata/testscript_stdout_stderr_error.txt
@@ -9,6 +9,7 @@ cmpenv stdout stdout.golden
 > printargs hello world
 > echoandexit 1 'this is stdout' 'this is stderr'
 -- stdout.golden --
+** RUN testscript **
 >  printargs hello world
 [stdout]
 ["printargs" "hello" "world"]
diff --git a/testscript/testscript.go b/testscript/testscript.go
index 45043fd0..3fe51897 100644
--- a/testscript/testscript.go
+++ b/testscript/testscript.go
@@ -22,6 +22,7 @@ import (
 	"regexp"
 	"runtime"
 	"slices"
+	"strconv"
 	"strings"
 	"sync/atomic"
 	"syscall"
@@ -136,6 +137,11 @@ type Params struct {
 	// Dir is interpreted relative to the current test directory.
 	Dir string
 
+	// Files holds a set of script filenames. If Dir is empty and this
+	// is non-nil, these files will be used instead of reading
+	// a directory.
+	Files []string
+
 	// Setup is called, if not nil, to complete any setup required
 	// for a test. The WorkDir and Vars fields will have already
 	// been initialized and all the files extracted into WorkDir,
@@ -241,24 +247,29 @@ func (t tshim) Verbose() bool {
 // RunT is like Run but uses an interface type instead of the concrete *testing.T
 // type to make it possible to use testscript functionality outside of go test.
 func RunT(t T, p Params) {
-	entries, err := os.ReadDir(p.Dir)
-	if os.IsNotExist(err) {
-		// Continue so we give a helpful error on len(files)==0 below.
-	} else if err != nil {
-		t.Fatal(err)
-	}
 	var files []string
-	for _, entry := range entries {
-		name := entry.Name()
-		if strings.HasSuffix(name, ".txtar") || strings.HasSuffix(name, ".txt") {
-			files = append(files, filepath.Join(p.Dir, name))
+	if p.Dir == "" && p.Files != nil {
+		files = p.Files
+	} else {
+		entries, err := os.ReadDir(p.Dir)
+		if os.IsNotExist(err) {
+			// Continue so we give a helpful error on len(files)==0 below.
+		} else if err != nil {
+			t.Fatal(err)
+		}
+		for _, entry := range entries {
+			name := entry.Name()
+			if strings.HasSuffix(name, ".txtar") || strings.HasSuffix(name, ".txt") {
+				files = append(files, filepath.Join(p.Dir, name))
+			}
 		}
-	}
 
-	if len(files) == 0 {
-		t.Fatal(fmt.Sprintf("no txtar nor txt scripts found in dir %s", p.Dir))
+		if len(files) == 0 {
+			t.Fatal(fmt.Sprintf("no txtar nor txt scripts found in dir %s", p.Dir))
+		}
 	}
 	testTempDir := p.WorkdirRoot
+	var err error
 	if testTempDir == "" {
 		testTempDir, err = os.MkdirTemp(os.Getenv("GOTMPDIR"), "go-test-script")
 		if err != nil {
@@ -307,10 +318,27 @@ func RunT(t T, p Params) {
 	}
 
 	refCount := int32(len(files))
+	names := make(map[string]bool)
 	for _, file := range files {
 		file := file
-		name := strings.TrimSuffix(filepath.Base(file), ".txt")
-		name = strings.TrimSuffix(name, ".txtar")
+		name := filepath.Base(file)
+		if name1, ok := strings.CutSuffix(name, ".txt"); ok {
+			name = name1
+		} else if name1, ok := strings.CutSuffix(name, ".txtar"); ok {
+			name = name1
+		}
+		// We can have duplicate names when files are passed explicitly,
+		// so disambiguate by adding a counter.
+		// Take care to handle the situation where a name with a counter-like
+		// suffix already exists, for example:
+		//	a/foo.txt
+		//	b/foo.txtar
+		//	c/foo#1.txt
+		prefix := name
+		for i := 1; names[name]; i++ {
+			name = prefix + "#" + strconv.Itoa(i)
+		}
+		names[name] = true
 		t.Run(name, func(t T) {
 			t.Parallel()
 			ts := &TestScript{
diff --git a/testscript/testscript_test.go b/testscript/testscript_test.go
index fc2e912a..9cbab8fa 100644
--- a/testscript/testscript_test.go
+++ b/testscript/testscript_test.go
@@ -164,7 +164,7 @@ func TestSetupFailure(t *testing.T) {
 		t.Fatal("test should have failed because of setup failure")
 	}
 
-	want := regexp.MustCompile(`^FAIL: .*: some failure\n$`)
+	want := regexp.MustCompile(`\nFAIL: .*: some failure\n$`)
 	if got := ft.log.String(); !want.MatchString(got) {
 		t.Fatalf("expected msg to match `%v`; got:\n%q", want, got)
 	}
@@ -226,18 +226,28 @@ func TestScripts(t *testing.T) {
 				fUniqueNames := fset.Bool("unique-names", false, "require unique names in txtar archive")
 				fVerbose := fset.Bool("v", false, "be verbose with output")
 				fContinue := fset.Bool("continue", false, "continue on error")
+				fFiles := fset.Bool("files", false, "specify files rather than a directory")
 				if err := fset.Parse(args); err != nil {
 					ts.Fatalf("failed to parse args for testscript: %v", err)
 				}
-				if fset.NArg() != 1 {
-					ts.Fatalf("testscript [-v] [-continue] [-update] [-explicit-exec] <dir>")
+				if fset.NArg() != 1 && !*fFiles {
+					ts.Fatalf("testscript [-v] [-continue] [-update] [-explicit-exec] [-files] <dir>|<file>...")
+				}
+				var files []string
+				var dir string
+				if *fFiles {
+					for _, f := range fset.Args() {
+						files = append(files, ts.MkAbs(f))
+					}
+				} else {
+					dir = ts.MkAbs(fset.Arg(0))
 				}
-				dir := fset.Arg(0)
 				t := &fakeT{verbose: *fVerbose}
 				func() {
 					defer catchAbort()
 					RunT(t, Params{
-						Dir:                 ts.MkAbs(dir),
+						Dir:                 dir,
+						Files:               files,
 						UpdateScripts:       *fUpdate,
 						RequireExplicitExec: *fExplicitExec,
 						RequireUniqueNames:  *fUniqueNames,
@@ -502,9 +512,27 @@ func (t *fakeT) FailNow() {
 }
 
 func (t *fakeT) Run(name string, f func(T)) {
-	f(t)
+	fmt.Fprintf(&t.log, "** RUN %s **\n", name)
+	defer catchAbort()
+	f(&subT{
+		fakeT: t,
+	})
 }
 
 func (t *fakeT) Verbose() bool {
 	return t.verbose
 }
+
+type subT struct {
+	*fakeT
+	failed bool
+}
+
+func (t *subT) Run(name string, f func(T)) {
+	panic("multiple test levels not supported")
+}
+
+func (t *subT) FailNow() {
+	t.failed = true
+	t.fakeT.FailNow()
+}