testscript: merge coverage from all test binary executions

.github/workflows/test.yml

@@ -11,7 +11,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        go-version: [1.14.x, 1.15.x, 1.16.0-beta1]
+        go-version: [1.14.x, 1.15.x, 1.16.0-rc1]
         os: [ubuntu-latest, macos-latest, windows-latest]
     runs-on: ${{ matrix.os }}
     steps:

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)

cmd/testscript/testdata/nogo.txt

@@ -1,7 +1,10 @@
 # should support skip
 unquote file.txt
 
-env PATH=
+# We can't just set PATH to empty because we need the part of it that
+# contains the command names, so use a special builtin instead.
+dropgofrompath
+
 ! testscript -v file.txt
 stdout 'unknown command "go"'
 stderr 'error running file.txt in'

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)
 	}

gotooltest/testdata/cover.txt

@@ -0,0 +1,82 @@
+unquote scripts/exec.txt
+
+# The module uses testscript itself.
+# Use the checked 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

testscript/cover.go

@@ -8,8 +8,8 @@ import (
 	"bufio"
 	"fmt"
 	"io"
-	"log"
 	"os"
+	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
@@ -22,8 +22,13 @@ import (
 // mergeCoverProfile merges the coverage information in f into
 // cover. It assumes that the coverage information in f is
 // always produced from the same binary for every call.
-func mergeCoverProfile(cover *testing.Cover, r io.Reader) error {
-	scanner, err := newProfileScanner(r)
+func mergeCoverProfile(cover *testing.Cover, path string) error {
+	f, err := os.Open(path)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+	scanner, err := newProfileScanner(f)
 	if err != nil {
 		return errors.Wrap(err)
 	}
@@ -83,45 +88,45 @@ 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 := mergeCoverProfile(&cover, f); err != nil {
-			log.Printf("cannot merge coverage profile from %v: %v", f.Name(), err)
+	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
+		}
+		if err := mergeCoverProfile(&cover, path); err != nil {
+			return fmt.Errorf("cannot merge coverage profile from %v: %v", path, err)
 		}
-		f.Close()
-		os.Remove(f.Name())
+		return nil
+	}); err != nil {
+		return errors.Wrap(err)
+	}
+	if err := os.RemoveAll(dir); err != nil {
+		// The RemoveAll seems to fail very rarely, with messages like
+		// "directory not empty". It's unclear why.
+		// For now, if it happens again, try to print a bit more info.
+		filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
+			if err == nil && !info.IsDir() {
+				fmt.Fprintln(os.Stderr, "non-directory found after RemoveAll:", path)
+			}
+			return 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
+	if err := mergeCoverProfile(&cover, cprof); err != nil {
+		return fmt.Errorf("cannot merge coverage profile from %v: %v", cprof, err)
 	}
-	f, err := os.Open(cprof)
-	if err != nil {
-		return errors.Notef(err, nil, "cannot open existing cover profile")
-	}
-	coverChan <- f
-	close(coverChan)
-	cover := <-coverDone
-	f, err = os.Create(cprof)
+
+	// Finally, write the resulting merged profile.
+	f, err := os.Create(cprof)
 	if err != nil {
 		return errors.Notef(err, nil, "cannot create cover profile")
 	}