From 1db6e1e230e9279cd4007a9360e470d22b9aa860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Tue, 2 Feb 2021 00:30:07 +0000 Subject: [PATCH] make -coverprofile include toolexec processes (#216) testscript already included magic to also account for commands in the total code coverage. That does not happen with plain tests, since those only include coverage from the main test process. The main problem was that, before, indirectly executed commands did not properly save their coverage profile anywhere for testscript to collect it at the end. In other words, we only collected coverage from direct garble executions like "garble -help", but not indirect ones like "go build -toolexec=garble". $ go test -coverprofile=cover.out PASS coverage: 3.6% of statements total coverage: 16.6% of statements ok mvdan.cc/garble 6.453s After the delicate changes to testscript, any direct or indirect executions of commands all go through $PATH and properly count towards the total coverage: $ go test -coverprofile=cover.out PASS coverage: 3.6% of statements total coverage: 90.5% of statements ok mvdan.cc/garble 33.258s Note that we can also get rid of our code to set up $PATH, since testscript now does it for us. goversion.txt needed minor tweaks, since we no longer set up $WORK/.bin. Finally, note that we disable the reuse of $GOCACHE when collecting coverage information. This is to do "full builds", as otherwise the cached package builds would result in lower coverage. Fixes #35. --- go.mod | 2 +- go.sum | 4 ++-- main_test.go | 38 ++++++---------------------------- testdata/scripts/basic.txt | 2 +- testdata/scripts/goversion.txt | 2 ++ 5 files changed, 12 insertions(+), 36 deletions(-) diff --git a/go.mod b/go.mod index 8f9b5c7..a17ac2d 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.15 require ( github.com/Binject/debug v0.0.0-20210101210738-1b03ff50b8a5 github.com/google/go-cmp v0.5.4 - github.com/rogpeppe/go-internal v1.6.3-0.20201011174404-9f985d550aa7 + github.com/rogpeppe/go-internal v1.7.1-0.20210131190821-dc4b49510d96 golang.org/x/mod v0.4.1 golang.org/x/tools v0.0.0-20210115202250-e0d201561e39 ) diff --git a/go.sum b/go.sum index 2a6d4ac..d1bc749 100644 --- a/go.sum +++ b/go.sum @@ -7,8 +7,8 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= -github.com/rogpeppe/go-internal v1.6.3-0.20201011174404-9f985d550aa7 h1:gsBBcK+87059lM2pDrOQWXGoVx9qYgoBasBMJ74eCYI= -github.com/rogpeppe/go-internal v1.6.3-0.20201011174404-9f985d550aa7/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= +github.com/rogpeppe/go-internal v1.7.1-0.20210131190821-dc4b49510d96 h1:E/aJC0lqUPDEdRwu6g+J/Egnf7hPxvgcaAffAvw2qkA= +github.com/rogpeppe/go-internal v1.7.1-0.20210131190821-dc4b49510d96/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= diff --git a/main_test.go b/main_test.go index e190753..384f69a 100644 --- a/main_test.go +++ b/main_test.go @@ -10,7 +10,6 @@ import ( "go/ast" "go/printer" "go/token" - "io" "math" mathrand "math/rand" "os" @@ -67,21 +66,13 @@ func TestScripts(t *testing.T) { "GOFLAGS=-mod=readonly", // TODO(mvdan): remove once we switch to Go 1.16 "gofullversion="+runtime.Version(), ) - bindir := filepath.Join(env.WorkDir, ".bin") - if err := os.Mkdir(bindir, 0o777); err != nil { - return err - } - binfile := filepath.Join(bindir, "garble") - if runtime.GOOS == "windows" { - binfile += ".exe" - } - if err := os.Symlink(os.Args[0], binfile); err != nil { - if err := copyFile(os.Args[0], binfile); err != nil { // Fallback to copy if symlink failed. Useful for Windows not elevated processes - return err - } + + if os.Getenv("TESTSCRIPT_COVER_DIR") != "" { + // Don't reuse the build cache if we want to collect + // code coverage. Otherwise, many toolexec calls would + // be avoided and the coverage would be incomplete. + env.Vars = append(env.Vars, "GOCACHE="+filepath.Join(env.WorkDir, "go-cache-tmp")) } - env.Vars = append(env.Vars, fmt.Sprintf("PATH=%s%c%s", bindir, filepath.ListSeparator, os.Getenv("PATH"))) - env.Vars = append(env.Vars, "TESTSCRIPT_COMMAND=garble") return nil }, Cmds: map[string]func(ts *testscript.TestScript, neg bool, args []string){ @@ -99,23 +90,6 @@ func TestScripts(t *testing.T) { testscript.Run(t, p) } -func copyFile(from, to string) error { - 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 -} - type binaryCache struct { name string modtime time.Time diff --git a/testdata/scripts/basic.txt b/testdata/scripts/basic.txt index 3b42789..915ddad 100644 --- a/testdata/scripts/basic.txt +++ b/testdata/scripts/basic.txt @@ -27,7 +27,7 @@ stdout 'unknown' [short] stop # checking that the build is reproducible is slow -# Check that we fail if the user used "go build -toolexec garble" instead of "garble build" +# Check that we fail if the user used "go build -toolexec garble" instead of "garble build" ! exec go build -a -toolexec=garble main.go stderr 'not running "garble \[command\]"' diff --git a/testdata/scripts/goversion.txt b/testdata/scripts/goversion.txt index c4c3a0b..f998ecb 100644 --- a/testdata/scripts/goversion.txt +++ b/testdata/scripts/goversion.txt @@ -1,7 +1,9 @@ # TODO: Place file direct to .bin directory +mkdir .bin cp go.sh .bin/go cp go.bat .bin/go.bat chmod 777 .bin/go +env PATH=.bin${:}${PATH} # Check incorrect go install env GO_VERSION=''