From df13fa7beb7bd19b13183d68337a0798432d6e36 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Fri, 30 Aug 2019 11:02:08 -0400 Subject: [PATCH] all: do not write to testdata directories I got tired of spurious 'git' diffs while a 'go test' was running, so I fixed the test that produced the diffs. (We need to do that anyway in order to run them in the module cache, plus it's just good hygiene not to have tests interfering with each other's sources.) Tested using: $ chmod -R ugo-w . && go test ./...; chmod -R u+w . Updates golang/go#28387 Change-Id: Ie17e31aecf0e3cb022df5503d7c443000366a5c6 Reviewed-on: https://go-review.googlesource.com/c/tools/+/192577 Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- cmd/cover/cover_test.go | 47 ++++++++++++++++++++-------------- cmd/guru/guru_test.go | 12 +++++---- refactor/eg/eg_test.go | 7 ++++- refactor/rename/rename_test.go | 44 ++++++++++++++++++++++++++----- 4 files changed, 79 insertions(+), 31 deletions(-) diff --git a/cmd/cover/cover_test.go b/cmd/cover/cover_test.go index 10a85fb54a..54c4512bff 100644 --- a/cmd/cover/cover_test.go +++ b/cmd/cover/cover_test.go @@ -23,17 +23,6 @@ import ( const ( // Data directory, also the package directory for the test. testdata = "testdata" - - // Binaries we compile. - testcover = "./testcover.exe" -) - -var ( - // Files we use. - testMain = filepath.Join(testdata, "main.go") - testTest = filepath.Join(testdata, "test.go") - coverInput = filepath.Join(testdata, "test_line.go") - coverOutput = filepath.Join(testdata, "test_cover.go") ) var debug = false // Keeps the rewritten files around if set. @@ -48,6 +37,34 @@ var debug = false // Keeps the rewritten files around if set. func TestCover(t *testing.T) { testenv.NeedsTool(t, "go") + tmpdir, err := ioutil.TempDir("", "TestCover") + if err != nil { + t.Fatal(err) + } + defer func() { + if debug { + fmt.Printf("test files left in %s\n", tmpdir) + } else { + os.RemoveAll(tmpdir) + } + }() + + testcover := filepath.Join(tmpdir, "testcover.exe") + testMain := filepath.Join(tmpdir, "main.go") + testTest := filepath.Join(tmpdir, "test.go") + coverInput := filepath.Join(tmpdir, "test_line.go") + coverOutput := filepath.Join(tmpdir, "test_cover.go") + + for _, f := range []string{testMain, testTest} { + data, err := ioutil.ReadFile(filepath.Join(testdata, filepath.Base(f))) + if err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(f, data, 0644); err != nil { + t.Fatal(err) + } + } + // Read in the test file (testTest) and write it, with LINEs specified, to coverInput. file, err := ioutil.ReadFile(testTest) if err != nil { @@ -62,18 +79,10 @@ func TestCover(t *testing.T) { t.Fatal(err) } - // defer removal of test_line.go - if !debug { - defer os.Remove(coverInput) - } - // go build -o testcover cmd := exec.Command("go", "build", "-o", testcover) run(cmd, t) - // defer removal of testcover - defer os.Remove(testcover) - // ./testcover -mode=count -var=coverTest -o ./testdata/test_cover.go testdata/test_line.go cmd = exec.Command(testcover, "-mode=count", "-var=coverTest", "-o", coverOutput, coverInput) run(cmd, t) diff --git a/cmd/guru/guru_test.go b/cmd/guru/guru_test.go index b322e9af85..0699db9ee0 100644 --- a/cmd/guru/guru_test.go +++ b/cmd/guru/guru_test.go @@ -275,13 +275,15 @@ func TestGuru(t *testing.T) { json := strings.Contains(filename, "-json/") queries := parseQueries(t, filename) golden := filename + "lden" - got := filename + "t" - gotfh, err := os.Create(got) + gotfh, err := ioutil.TempFile("", filepath.Base(filename)+"t") if err != nil { - t.Fatalf("Create(%s) failed: %s", got, err) + t.Fatal(err) } - defer os.Remove(got) - defer gotfh.Close() + got := gotfh.Name() + defer func() { + gotfh.Close() + os.Remove(got) + }() // Run the guru on each query, redirecting its output // and error (if any) to the foo.got file. diff --git a/refactor/eg/eg_test.go b/refactor/eg/eg_test.go index 985e9a7e4c..158f90957f 100644 --- a/refactor/eg/eg_test.go +++ b/refactor/eg/eg_test.go @@ -15,6 +15,7 @@ import ( "go/parser" "go/token" "go/types" + "io/ioutil" "os" "os/exec" "path/filepath" @@ -135,7 +136,11 @@ func Test(t *testing.T) { continue } - got := filename + "t" // foo.got + gotf, err := ioutil.TempFile("", filepath.Base(filename)+"t") + if err != nil { + t.Fatal(err) + } + got := gotf.Name() // foo.got golden := filename + "lden" // foo.golden // Write actual output to foo.got. diff --git a/refactor/rename/rename_test.go b/refactor/rename/rename_test.go index 176d9987a9..0369d7c76f 100644 --- a/refactor/rename/rename_test.go +++ b/refactor/rename/rename_test.go @@ -9,6 +9,7 @@ import ( "fmt" "go/build" "go/token" + "io/ioutil" "os" "os/exec" "path/filepath" @@ -1290,7 +1291,7 @@ func TestDiff(t *testing.T) { t.Skipf("plan9 diff tool doesn't support -u flag") } testenv.NeedsTool(t, DiffCmd) - testenv.NeedsTool(t, "go") // to locate golang.org/x/tools/refactor/rename + testenv.NeedsTool(t, "go") // to locate the package to be renamed defer func() { Diff = false @@ -1299,7 +1300,42 @@ func TestDiff(t *testing.T) { Diff = true stdout = new(bytes.Buffer) - if err := Main(&build.Default, "", `"golang.org/x/tools/refactor/rename".justHereForTestingDiff`, "Foo"); err != nil { + // Set up a fake GOPATH in a temporary directory, + // and ensure we're in GOPATH mode. + tmpdir, err := ioutil.TempDir("", "TestDiff") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tmpdir) + buildCtx := build.Default + buildCtx.GOPATH = tmpdir + + pkgDir := filepath.Join(tmpdir, "src", "example.com", "rename") + if err := os.MkdirAll(pkgDir, 0777); err != nil { + t.Fatal(err) + } + + prevWD, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer os.Chdir(prevWD) + + if err := os.Chdir(pkgDir); err != nil { + t.Fatal(err) + } + + const goFile = `package rename + +func justHereForTestingDiff() { + justHereForTestingDiff() +} +` + if err := ioutil.WriteFile(filepath.Join(pkgDir, "rename_test.go"), []byte(goFile), 0644); err != nil { + t.Fatal(err) + } + + if err := Main(&buildCtx, "", `"example.com/rename".justHereForTestingDiff`, "Foo"); err != nil { t.Fatal(err) } @@ -1315,10 +1351,6 @@ func TestDiff(t *testing.T) { } } -func justHereForTestingDiff() { - justHereForTestingDiff() -} - // --------------------------------------------------------------------- // Simplifying wrapper around buildutil.FakeContext for packages whose