cmd/stringer: log more information in tests

Also associate the stderr and stdout output of subprocesses more
clearly with the specific test.

For golang/go#62534.

Change-Id: I6768f2d8d60e21d4d6465208c17b542691a3f803
Reviewed-on: https://go-review.googlesource.com/c/tools/+/526172
Commit-Queue: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>

cmd/stringer/endtoend_test.go

@@ -11,11 +11,11 @@ package main
 
 import (
 	"bytes"
+	"flag"
 	"fmt"
 	"go/build"
 	"io"
 	"os"
-	"os/exec"
 	"path"
 	"path/filepath"
 	"strings"
@@ -42,6 +42,11 @@ func TestMain(m *testing.M) {
 	// command, and much less complicated and expensive to build and clean up.
 	os.Setenv("STRINGER_TEST_IS_STRINGER", "1")
 
+	flag.Parse()
+	if testing.Verbose() {
+		os.Setenv("GOPACKAGESDEBUG", "true")
+	}
+
 	os.Exit(m.Run())
 }
 
@@ -74,11 +79,12 @@ func TestEndToEnd(t *testing.T) {
 			// This file is used for tag processing in TestTags or TestConstValueChange, below.
 			continue
 		}
-		if name == "cgo.go" && !build.Default.CgoEnabled {
-			t.Logf("cgo is not enabled for %s", name)
-			continue
-		}
-		stringerCompileAndRun(t, t.TempDir(), stringer, typeName(name), name)
+		t.Run(name, func(t *testing.T) {
+			if name == "cgo.go" && !build.Default.CgoEnabled {
+				t.Skipf("cgo is not enabled for %s", name)
+			}
+			stringerCompileAndRun(t, t.TempDir(), stringer, typeName(name), name)
+		})
 	}
 }
 
@@ -122,7 +128,7 @@ func TestTags(t *testing.T) {
 	// - Versions of Go earlier than Go 1.11, do not support absolute directories as a pattern.
 	// - When the current directory is inside a go module, the path will not be considered
 	//   a valid path to a package.
-	err := runInDir(dir, stringer, "-type", "Const", ".")
+	err := runInDir(t, dir, stringer, "-type", "Const", ".")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -137,7 +143,7 @@ func TestTags(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	err = runInDir(dir, stringer, "-type", "Const", "-tags", "tag", ".")
+	err = runInDir(t, dir, stringer, "-type", "Const", "-tags", "tag", ".")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -162,12 +168,12 @@ func TestConstValueChange(t *testing.T) {
 	}
 	stringSource := filepath.Join(dir, "day_string.go")
 	// Run stringer in the directory that contains the package files.
-	err = runInDir(dir, stringer, "-type", "Day", "-output", stringSource)
+	err = runInDir(t, dir, stringer, "-type", "Day", "-output", stringSource)
 	if err != nil {
 		t.Fatal(err)
 	}
 	// Run the binary in the temporary directory as a sanity check.
-	err = run("go", "run", stringSource, source)
+	err = run(t, "go", "run", stringSource, source)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -185,8 +191,8 @@ func TestConstValueChange(t *testing.T) {
 	// output. An alternative might be to check that the error output
 	// matches a set of possible error strings emitted by known
 	// Go compilers.
-	fmt.Fprintf(os.Stderr, "Note: the following messages should indicate an out-of-bounds compiler error\n")
-	err = run("go", "build", stringSource, source)
+	t.Logf("Note: the following messages should indicate an out-of-bounds compiler error\n")
+	err = run(t, "go", "build", stringSource, source)
 	if err == nil {
 		t.Fatal("unexpected compiler success")
 	}
@@ -213,7 +219,6 @@ func stringerPath(t *testing.T) string {
 // stringerCompileAndRun runs stringer for the named file and compiles and
 // runs the target binary in directory dir. That binary will panic if the String method is incorrect.
 func stringerCompileAndRun(t *testing.T, dir, stringer, typeName, fileName string) {
-	t.Helper()
 	t.Logf("run: %s %s\n", fileName, typeName)
 	source := filepath.Join(dir, path.Base(fileName))
 	err := copy(source, filepath.Join("testdata", fileName))
@@ -222,12 +227,12 @@ func stringerCompileAndRun(t *testing.T, dir, stringer, typeName, fileName strin
 	}
 	stringSource := filepath.Join(dir, typeName+"_string.go")
 	// Run stringer in temporary directory.
-	err = run(stringer, "-type", typeName, "-output", stringSource, source)
+	err = run(t, stringer, "-type", typeName, "-output", stringSource, source)
 	if err != nil {
 		t.Fatal(err)
 	}
 	// Run the binary in the temporary directory.
-	err = run("go", "run", stringSource, source)
+	err = run(t, "go", "run", stringSource, source)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -251,17 +256,24 @@ func copy(to, from string) error {
 
 // run runs a single command and returns an error if it does not succeed.
 // os/exec should have this function, to be honest.
-func run(name string, arg ...string) error {
-	return runInDir(".", name, arg...)
+func run(t testing.TB, name string, arg ...string) error {
+	t.Helper()
+	return runInDir(t, ".", name, arg...)
 }
 
 // runInDir runs a single command in directory dir and returns an error if
 // it does not succeed.
-func runInDir(dir, name string, arg ...string) error {
-	cmd := exec.Command(name, arg...)
+func runInDir(t testing.TB, dir, name string, arg ...string) error {
+	t.Helper()
+	cmd := testenv.Command(t, name, arg...)
 	cmd.Dir = dir
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
 	cmd.Env = append(os.Environ(), "GO111MODULE=auto")
-	return cmd.Run()
+	out, err := cmd.CombinedOutput()
+	if len(out) > 0 {
+		t.Logf("%s", out)
+	}
+	if err != nil {
+		return fmt.Errorf("%v: %v", cmd, err)
+	}
+	return nil
 }

cmd/stringer/golden_test.go

@@ -453,28 +453,32 @@ func TestGolden(t *testing.T) {
 
 	dir := t.TempDir()
 	for _, test := range golden {
-		g := Generator{
-			trimPrefix:  test.trimPrefix,
-			lineComment: test.lineComment,
-		}
-		input := "package test\n" + test.input
-		file := test.name + ".go"
-		absFile := filepath.Join(dir, file)
-		err := os.WriteFile(absFile, []byte(input), 0644)
-		if err != nil {
-			t.Error(err)
-		}
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			g := Generator{
+				trimPrefix:  test.trimPrefix,
+				lineComment: test.lineComment,
+				logf:        t.Logf,
+			}
+			input := "package test\n" + test.input
+			file := test.name + ".go"
+			absFile := filepath.Join(dir, file)
+			err := os.WriteFile(absFile, []byte(input), 0644)
+			if err != nil {
+				t.Fatal(err)
+			}
 
-		g.parsePackage([]string{absFile}, nil)
-		// Extract the name and type of the constant from the first line.
-		tokens := strings.SplitN(test.input, " ", 3)
-		if len(tokens) != 3 {
-			t.Fatalf("%s: need type declaration on first line", test.name)
-		}
-		g.generate(tokens[1])
-		got := string(g.format())
-		if got != test.output {
-			t.Errorf("%s: got(%d)\n====\n%q====\nexpected(%d)\n====%q", test.name, len(got), got, len(test.output), test.output)
-		}
+			g.parsePackage([]string{absFile}, nil)
+			// Extract the name and type of the constant from the first line.
+			tokens := strings.SplitN(test.input, " ", 3)
+			if len(tokens) != 3 {
+				t.Fatalf("%s: need type declaration on first line", test.name)
+			}
+			g.generate(tokens[1])
+			got := string(g.format())
+			if got != test.output {
+				t.Errorf("%s: got(%d)\n====\n%q====\nexpected(%d)\n====%q", test.name, len(got), got, len(test.output), test.output)
+			}
+		})
 	}
 }

cmd/stringer/stringer.go

@@ -188,6 +188,8 @@ type Generator struct {
 
 	trimPrefix  string
 	lineComment bool
+
+	logf func(format string, args ...interface{}) // test logging hook; nil when not testing
 }
 
 func (g *Generator) Printf(format string, args ...interface{}) {
@@ -221,13 +223,14 @@ func (g *Generator) parsePackage(patterns []string, tags []string) {
 		// in a separate pass? For later.
 		Tests:      false,
 		BuildFlags: []string{fmt.Sprintf("-tags=%s", strings.Join(tags, " "))},
+		Logf:       g.logf,
 	}
 	pkgs, err := packages.Load(cfg, patterns...)
 	if err != nil {
 		log.Fatal(err)
 	}
 	if len(pkgs) != 1 {
-		log.Fatalf("error: %d packages found", len(pkgs))
+		log.Fatalf("error: %d packages matching %v", len(pkgs), strings.Join(patterns, " "))
 	}
 	g.addPackage(pkgs[0])
 }