diff --git a/cmd/testscript/main_test.go b/cmd/testscript/main_test.go index ac7f63fb..a534f353 100644 --- a/cmd/testscript/main_test.go +++ b/cmd/testscript/main_test.go @@ -70,7 +70,8 @@ func dropgofrompath(ts *testscript.TestScript, neg bool, args []string) { var newPath []string for _, d := range filepath.SplitList(ts.Getenv("PATH")) { getenv := func(k string) string { - if k == "PATH" { + // Note that Windows and Plan9 use lowercase "path". + if strings.ToUpper(k) == "PATH" { return d } return ts.Getenv(k) @@ -79,7 +80,9 @@ func dropgofrompath(ts *testscript.TestScript, neg bool, args []string) { newPath = append(newPath, d) } } + ts.Logf("PATH=%s", ts.Getenv("PATH")) ts.Setenv("PATH", strings.Join(newPath, string(filepath.ListSeparator))) + ts.Logf("PATH=%s", ts.Getenv("PATH")) } func setfilegoproxy(ts *testscript.TestScript, neg bool, args []string) { diff --git a/cmd/testscript/testdata/nogo.txt b/cmd/testscript/testdata/nogo.txt index e3dd684d..f1457534 100644 --- a/cmd/testscript/testdata/nogo.txt +++ b/cmd/testscript/testdata/nogo.txt @@ -1,7 +1,7 @@ # should support skip unquote file.txt -env PATH= +dropgofrompath ! testscript -v file.txt stdout 'unknown command "go"' stderr 'error running file.txt in' diff --git a/gotooltest/script_test.go b/gotooltest/script_test.go index 47e6fe72..5beb54a7 100644 --- a/gotooltest/script_test.go +++ b/gotooltest/script_test.go @@ -5,6 +5,8 @@ package gotooltest_test import ( + "os" + "path/filepath" "testing" "github.com/rogpeppe/go-internal/gotooltest" @@ -14,7 +16,19 @@ import ( func TestSimple(t *testing.T) { p := testscript.Params{ Dir: "testdata", + Setup: func(env *testscript.Env) error { + // cover.txt will need testscript as a dependency. + // Tell it where our module is, via an absolute path. + wd, err := os.Getwd() + if err != nil { + return err + } + modPath := filepath.Dir(wd) + env.Setenv("GOINTERNAL_MODULE", modPath) + return nil + }, } + if err := gotooltest.Setup(&p); err != nil { t.Fatal(err) } diff --git a/gotooltest/testdata/cover.txt b/gotooltest/testdata/cover.txt new file mode 100644 index 00000000..b1386876 --- /dev/null +++ b/gotooltest/testdata/cover.txt @@ -0,0 +1,82 @@ +unquote scripts/exec.txt + +# The module uses testscript itself. +# Use the checkec out module, based on where the test binary ran. +go mod edit -replace=github.com/rogpeppe/go-internal=${GOINTERNAL_MODULE} +go mod tidy + +# First, a 'go test' run without coverage. +go test -vet=off +stdout 'PASS' +! stdout 'total coverage' + +# Then, a 'go test' run with -coverprofile. +# Assuming testscript works well, this results in the basic coverage being 0%, +# since the test binary does not directly run any non-test code. +# The total coverage after merging profiles should end up being 100%, +# as long as all three sub-profiles are accounted for. +# Marking all printlns as covered requires all edge cases to work well. +go test -vet=off -coverprofile=cover.out -v +stdout 'PASS' +stdout '^coverage: 0\.0%' +stdout '^total coverage: 100\.0%' +! stdout 'malformed coverage' # written by "go test" if cover.out is invalid +exists cover.out + +-- go.mod -- +module test + +go 1.15 +-- foo.go -- +package foo + +import "os" + +func foo1() int { + switch os.Args[1] { + case "1": + println("first path") + case "2": + println("second path") + default: + println("third path") + } + return 1 +} +-- foo_test.go -- +package foo + +import ( + "os" + "testing" + + "github.com/rogpeppe/go-internal/gotooltest" + "github.com/rogpeppe/go-internal/testscript" +) + +func TestMain(m *testing.M) { + os.Exit(testscript.RunMain(m, map[string] func() int{ + "foo": foo1, + })) +} + +func TestFoo(t *testing.T) { + p := testscript.Params{ + Dir: "scripts", + } + if err := gotooltest.Setup(&p); err != nil { + t.Fatal(err) + } + testscript.Run(t, p) +} +-- scripts/exec.txt -- +># Note that foo always fails, to prevent "go build" from doing anything. +> +># Running the command directly; trigger the first path. +>! foo 1 +> +># Running the command via exec; trigger the second path. +>! exec foo 2 +> +># Running the command indirectly, via toolexec; trigger the third path. +>! go build -a -toolexec=foo runtime diff --git a/testscript/cover.go b/testscript/cover.go index 09400c95..eb970a01 100644 --- a/testscript/cover.go +++ b/testscript/cover.go @@ -8,8 +8,8 @@ import ( "bufio" "fmt" "io" - "log" "os" + "path/filepath" "regexp" "strconv" "strings" @@ -83,44 +83,44 @@ func mergeCoverProfile(cover *testing.Cover, r io.Reader) error { return nil } -var ( - coverChan chan *os.File - coverDone chan testing.Cover -) - -func goCoverProfileMerge() { - if coverChan != nil { - panic("RunMain called twice!") - } - coverChan = make(chan *os.File) - coverDone = make(chan testing.Cover) - go mergeCoverProfiles() -} - -func mergeCoverProfiles() { +func finalizeCoverProfile(dir string) error { + // Merge all the coverage profiles written by test binary sub-processes, + // such as those generated by executions of commands. var cover testing.Cover - for f := range coverChan { + if err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !info.Mode().IsRegular() { + return nil + } + f, err := os.Open(path) + if err != nil { + return err + } + defer f.Close() if err := mergeCoverProfile(&cover, f); err != nil { - log.Printf("cannot merge coverage profile from %v: %v", f.Name(), err) + return fmt.Errorf("cannot merge coverage profile from %v: %v", f.Name(), err) } - f.Close() - os.Remove(f.Name()) + return nil + }); err != nil { + return errors.Wrap(err) + } + if err := os.RemoveAll(dir); err != nil { + return errors.Wrap(err) } - coverDone <- cover -} -func finalizeCoverProfile() error { + // We need to include our own top-level coverage profile too. cprof := coverProfile() - if cprof == "" { - return nil - } f, err := os.Open(cprof) if err != nil { return errors.Notef(err, nil, "cannot open existing cover profile") } - coverChan <- f - close(coverChan) - cover := <-coverDone + if err := mergeCoverProfile(&cover, f); err != nil { + return fmt.Errorf("cannot merge coverage profile from %v: %v", f.Name(), err) + } + + // Finally, write the resulting merged profile. f, err = os.Create(cprof) if err != nil { return errors.Notef(err, nil, "cannot create cover profile") diff --git a/testscript/exe.go b/testscript/exe.go index e869e55d..f0108968 100644 --- a/testscript/exe.go +++ b/testscript/exe.go @@ -6,16 +6,17 @@ package testscript import ( "flag" - "fmt" "io" + "io/ioutil" "log" "os" - "sync/atomic" + "path/filepath" + "regexp" + "runtime" + "strings" "testing" ) -var profileId int32 = 0 - // TestingM is implemented by *testing.M. It's defined as an interface // to allow testscript to co-exist with other testing frameworks // that might also wish to call M.Run. @@ -49,48 +50,71 @@ func IgnoreMissedCoverage() { // // This function returns an exit code to pass to os.Exit, after calling m.Run. func RunMain(m TestingM, commands map[string]func() int) (exitCode int) { - goCoverProfileMerge() - cmdName := os.Getenv("TESTSCRIPT_COMMAND") - if cmdName == "" { + // Depending on os.Args[0], this is either the top-level execution of + // the test binary by "go test", or the execution of one of the provided + // commands via "foo" or "exec foo". + + rxBinary := regexp.MustCompile(`go-build.*\.test(\.exe)?$`) + if rxBinary.MatchString(os.Args[0]) { + // Set up all commands in a directory, added in $PATH. + // TODO: can we reuse WorkdirRoot somehow? + // TODO: can we do better than os.Setenv? + bindir, err := ioutil.TempDir("", "testscript-bin") + if err != nil { + log.Printf("could not set up PATH binary directory: %v", err) + return 2 + } defer func() { - if err := finalizeCoverProfile(); err != nil { - log.Printf("cannot merge cover profiles: %v", err) + if err := os.RemoveAll(bindir); err != nil { + log.Printf("cannot delete bin directory: %v", err) exitCode = 2 } }() + os.Setenv("PATH", bindir+string(filepath.ListSeparator)+os.Getenv("PATH")) + + flag.Parse() + // If we are collecting a coverage profile, set up a shared + // directory for all executed test binary sub-processes to write + // their profiles to. Before finishing, we'll merge all of those + // profiles into the main profile. + if coverProfile() != "" { + // TODO: same as the points for bindir + coverdir, err := ioutil.TempDir("", "testscript-cover") + if err != nil { + log.Printf("could not set up cover directory: %v", err) + return 2 + } + os.Setenv("TESTSCRIPT_COVER_DIR", coverdir) + defer func() { + if err := finalizeCoverProfile(coverdir); err != nil { + log.Printf("cannot merge cover profiles: %v", err) + exitCode = 2 + } + }() + } + // We're not in a subcommand. for name := range commands { name := name + // Set up this command in the directory we added to $PATH. + binfile := filepath.Join(bindir, name) + if runtime.GOOS == "windows" { + binfile += ".exe" + } + if err := linkFile(os.Args[0], binfile); err != nil { + log.Printf("could not set up %s in $PATH: %v", name, err) + return 2 + } scriptCmds[name] = func(ts *TestScript, neg bool, args []string) { - path, err := os.Executable() - if err != nil { - ts.Fatalf("cannot determine path to test binary: %v", err) - } - id := atomic.AddInt32(&profileId, 1) - 1 - oldEnvLen := len(ts.env) - cprof := coverFilename(id) - ts.env = append(ts.env, - "TESTSCRIPT_COMMAND="+name, - "TESTSCRIPT_COVERPROFILE="+cprof, - ) - ts.cmdExec(neg, append([]string{path}, args...)) - ts.env = ts.env[0:oldEnvLen] - if cprof == "" { - return - } - f, err := os.Open(cprof) - if err != nil { - if ignoreMissedCoverage { - return - } - ts.Fatalf("command %s (args %q) failed to generate coverage information", name, args) - return - } - coverChan <- f + ts.cmdExec(neg, append([]string{name}, args...)) } } return m.Run() } + cmdName := filepath.Base(os.Args[0]) + if runtime.GOOS == "windows" { + cmdName = strings.TrimSuffix(cmdName, ".exe") + } mainf := commands[cmdName] if mainf == nil { log.Printf("unknown command name %q", cmdName) @@ -98,12 +122,44 @@ func RunMain(m TestingM, commands map[string]func() int) (exitCode int) { } // The command being registered is being invoked, so run it, then exit. os.Args[0] = cmdName - cprof := os.Getenv("TESTSCRIPT_COVERPROFILE") - if cprof == "" { + coverdir := os.Getenv("TESTSCRIPT_COVER_DIR") + if coverdir == "" { // No coverage, act as normal. return mainf() } - return runCoverSubcommand(cprof, mainf) + // For a command "foo", write ${TESTSCRIPT_COVER_DIR}/foo-${RANDOM}. + coverFile, err := ioutil.TempFile(coverdir, cmdName+"-*") + if err != nil { + log.Printf("could not create a coverage profile: %v", err) + return 2 + } + coverPath := coverFile.Name() + coverFile.Close() + return runCoverSubcommand(coverPath, mainf) +} + +func linkFile(from, to string) error { + // On most Unix-like systems, we can get away with just a symbolic link. + if err := os.Symlink(from, to); err == nil { + return nil + } + + // Fall back to copying the entire binary. + // This might be necessary on some Windows setups. + writer, err := os.Create(to) + if err != nil { + return err + } + defer writer.Close() + + reader, err := os.Open(from) + if err != nil { + return err + } + defer reader.Close() + + _, err = io.Copy(writer, reader) + return err } // runCoverSubcommand runs the given function, then writes any generated @@ -158,13 +214,6 @@ func runCoverSubcommand(cprof string, mainf func() int) (exitCode int) { return mainf() } -func coverFilename(id int32) string { - if cprof := coverProfile(); cprof != "" { - return fmt.Sprintf("%s_%d", cprof, id) - } - return "" -} - func coverProfileFlag() flag.Getter { f := flag.CommandLine.Lookup("test.coverprofile") if f == nil { diff --git a/testscript/testdata/readfile.txt b/testscript/testdata/readfile.txt index 110986b7..dd08b628 100644 --- a/testscript/testdata/readfile.txt +++ b/testscript/testdata/readfile.txt @@ -1,7 +1,7 @@ -echo stdout stdout +fprintargs stdout stdout testreadfile stdout -echo stderr stderr +fprintargs stderr stderr testreadfile stderr testreadfile x/somefile diff --git a/testscript/testdata/testscript_update_script.txt b/testscript/testdata/testscript_update_script.txt index 43a39883..db481ec8 100644 --- a/testscript/testdata/testscript_update_script.txt +++ b/testscript/testdata/testscript_update_script.txt @@ -4,13 +4,13 @@ testscript-update scripts cmp scripts/testscript.txt testscript-new.txt -- scripts/testscript.txt -- ->echo stdout right +>fprintargs stdout right >cmp stdout expect > >-- expect -- >wrong -- testscript-new.txt -- ->echo stdout right +>fprintargs stdout right >cmp stdout expect > >-- expect -- diff --git a/testscript/testdata/testscript_update_script_expected_not_in_archive.txt b/testscript/testdata/testscript_update_script_expected_not_in_archive.txt index 81766a65..cda575c9 100644 --- a/testscript/testdata/testscript_update_script_expected_not_in_archive.txt +++ b/testscript/testdata/testscript_update_script_expected_not_in_archive.txt @@ -6,7 +6,7 @@ cp scripts/testscript.txt unchanged cmp scripts/testscript.txt unchanged -- scripts/testscript.txt -- ->echo stdout right +>fprintargs stdout right >cp file expect >cmp stdout expect > diff --git a/testscript/testdata/testscript_update_script_quote.txt b/testscript/testdata/testscript_update_script_quote.txt index ae6b00ba..a634b8b8 100644 --- a/testscript/testdata/testscript_update_script_quote.txt +++ b/testscript/testdata/testscript_update_script_quote.txt @@ -4,13 +4,13 @@ testscript-update scripts cmp scripts/testscript.txt testscript-new.txt -- scripts/testscript.txt -- ->echo stdout '-- lookalike --' +>fprintargs stdout '-- lookalike --' >cmp stdout expect > >-- expect -- >wrong -- testscript-new.txt -- ->echo stdout '-- lookalike --' +>fprintargs stdout '-- lookalike --' >cmp stdout expect > >-- expect -- diff --git a/testscript/testdata/testscript_update_script_stderr.txt b/testscript/testdata/testscript_update_script_stderr.txt index e321631f..66882b83 100644 --- a/testscript/testdata/testscript_update_script_stderr.txt +++ b/testscript/testdata/testscript_update_script_stderr.txt @@ -4,13 +4,13 @@ testscript-update scripts cmp scripts/testscript.txt testscript-new.txt -- scripts/testscript.txt -- ->echo stderr right +>fprintargs stderr right >cmp stderr expect > >-- expect -- >wrong -- testscript-new.txt -- ->echo stderr right +>fprintargs stderr right >cmp stderr expect > >-- expect -- diff --git a/testscript/testscript.go b/testscript/testscript.go index d915fda1..361911a9 100644 --- a/testscript/testscript.go +++ b/testscript/testscript.go @@ -316,6 +316,11 @@ func (ts *TestScript) setup() string { "exe=", ) } + // If we are collecting coverage profiles for merging into the main one, + // ensure the environment variable is forwarded to sub-processes. + if coverdir := os.Getenv("TESTSCRIPT_COVER_DIR"); coverdir != "" { + env.Vars = append(env.Vars, "TESTSCRIPT_COVER_DIR="+coverdir) + } ts.cd = env.Cd // Unpack archive. a, err := txtar.ParseFile(ts.file) diff --git a/testscript/testscript_test.go b/testscript/testscript_test.go index a28e3098..0fef259c 100644 --- a/testscript/testscript_test.go +++ b/testscript/testscript_test.go @@ -26,7 +26,7 @@ func printArgs() int { return 0 } -func echo() int { +func fprintArgs() int { s := strings.Join(os.Args[2:], " ") switch os.Args[1] { case "stdout": @@ -60,7 +60,7 @@ func signalCatcher() int { func TestMain(m *testing.M) { os.Exit(RunMain(m, map[string]func() int{ "printargs": printArgs, - "echo": echo, + "fprintargs": fprintArgs, "status": exitWithStatus, "signalcatcher": signalCatcher, }))