zip: expand logging and use t.TempDir and t.Cleanup in test helpers
Fixes golang/go#55250 (for now).
Change-Id: I7286c459937e666d3e88fe56a9af1a2ff22f0186
Reviewed-on: https://go-review.googlesource.com/c/mod/+/432475
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
diff --git a/zip/zip_test.go b/zip/zip_test.go
index 8436666..89d5555 100644
--- a/zip/zip_test.go
+++ b/zip/zip_test.go
@@ -38,6 +38,16 @@
sync.Once
}
+func init() {
+ if os.Getenv("GO_BUILDER_NAME") != "" || os.Getenv("GIT_TRACE_CURL") == "1" {
+ // Enable extra Git logging to diagnose networking issues.
+ // (These environment variables will be inherited by subprocesses.)
+ os.Setenv("GIT_TRACE_CURL", "1")
+ os.Setenv("GIT_TRACE_CURL_NO_DATA", "1")
+ os.Setenv("GIT_REDACT_COOKIES", "o,SSO,GSSO_Uberproxy")
+ }
+}
+
// gitPath returns the path to a usable "git" command,
// or a non-nil error.
func gitPath() (string, error) {
@@ -56,7 +66,7 @@
return gitOnce.path, gitOnce.err
}
-func mustHaveGit(t *testing.T) {
+func mustHaveGit(t testing.TB) {
if _, err := gitPath(); err != nil {
t.Helper()
t.Skipf("skipping: %v", err)
@@ -111,33 +121,24 @@
return test, nil
}
-func extractTxtarToTempDir(arc *txtar.Archive) (dir string, cleanup func(), err error) {
- dir, err = ioutil.TempDir("", "zip_test-*")
- if err != nil {
- return "", func() {}, err
- }
- cleanup = func() {
- os.RemoveAll(dir)
- }
- defer func() {
- if err != nil {
- cleanup()
- }
- }()
+func extractTxtarToTempDir(t testing.TB, arc *txtar.Archive) (dir string, err error) {
+ dir = t.TempDir()
for _, f := range arc.Files {
filePath := filepath.Join(dir, f.Name)
if err := os.MkdirAll(filepath.Dir(filePath), 0777); err != nil {
- return "", func() {}, err
+ return "", err
}
if err := ioutil.WriteFile(filePath, f.Data, 0666); err != nil {
- return "", func() {}, err
+ return "", err
}
}
- return dir, cleanup, nil
+ return dir, nil
}
-func extractTxtarToTempZip(arc *txtar.Archive) (zipPath string, err error) {
- zipFile, err := ioutil.TempFile("", "zip_test-*.zip")
+func extractTxtarToTempZip(t *testing.T, arc *txtar.Archive) (zipPath string, err error) {
+ zipPath = filepath.Join(t.TempDir(), "txtar.zip")
+
+ zipFile, err := os.Create(zipPath)
if err != nil {
return "", err
}
@@ -145,10 +146,8 @@
if cerr := zipFile.Close(); err == nil && cerr != nil {
err = cerr
}
- if err != nil {
- os.Remove(zipFile.Name())
- }
}()
+
zw := zip.NewWriter(zipFile)
for _, f := range arc.Files {
zf, err := zw.Create(f.Name)
@@ -305,11 +304,10 @@
break
}
}
- tmpDir, cleanup, err := extractTxtarToTempDir(test.archive)
+ tmpDir, err := extractTxtarToTempDir(t, test.archive)
if err != nil {
t.Fatal(err)
}
- defer cleanup()
// Check the directory.
cf, err := modzip.CheckDir(tmpDir)
@@ -366,15 +364,10 @@
break
}
}
- tmpZipPath, err := extractTxtarToTempZip(test.archive)
+ tmpZipPath, err := extractTxtarToTempZip(t, test.archive)
if err != nil {
t.Fatal(err)
}
- defer func() {
- if err := os.Remove(tmpZipPath); err != nil {
- t.Errorf("removing temp zip file: %v", err)
- }
- }()
// Check the zip.
m := module.Version{Path: test.path, Version: test.version}
@@ -493,11 +486,10 @@
}
// Write files to a temporary directory.
- tmpDir, cleanup, err := extractTxtarToTempDir(test.archive)
+ tmpDir, err := extractTxtarToTempDir(t, test.archive)
if err != nil {
t.Fatal(err)
}
- defer cleanup()
// Create zip from the directory.
tmpZip, err := ioutil.TempFile("", "TestCreateFromDir-*.zip")
@@ -632,15 +624,10 @@
}
// Convert txtar to temporary zip file.
- tmpZipPath, err := extractTxtarToTempZip(test.archive)
+ tmpZipPath, err := extractTxtarToTempZip(t, test.archive)
if err != nil {
t.Fatal(err)
}
- defer func() {
- if err := os.Remove(tmpZipPath); err != nil {
- t.Errorf("removing temp zip file: %v", err)
- }
- }()
// Extract to a temporary directory.
tmpDir, err := ioutil.TempDir("", "TestUnzip")
@@ -1263,8 +1250,7 @@
}
t.Parallel()
- repo, dl, cleanup, err := downloadVCSZip(test.vcs, test.url, test.rev, test.subdir)
- defer cleanup()
+ repo, dl, err := downloadVCSZip(t, test.vcs, test.url, test.rev, test.subdir)
if err != nil {
// This may fail if there's a problem with the network or upstream
// repository. The package being tested doesn't directly interact with
@@ -1331,7 +1317,7 @@
files = append(files, zipFile{name: name, f: f})
}
if !haveLICENSE && subdir != "" {
- license, err := downloadVCSFile(test.vcs, repo, test.rev, "LICENSE")
+ license, err := downloadVCSFile(t, test.vcs, repo, test.rev, "LICENSE")
if err != nil {
t.Fatal(err)
}
@@ -1378,41 +1364,31 @@
}
}
-func downloadVCSZip(vcs, url, rev, subdir string) (repoDir string, dl *os.File, cleanup func(), err error) {
- var cleanups []func()
- cleanup = func() {
- for i := len(cleanups) - 1; i >= 0; i-- {
- cleanups[i]()
- }
- }
- repoDir, err = ioutil.TempDir("", "downloadVCSZip")
- if err != nil {
- return "", nil, cleanup, err
- }
- cleanups = append(cleanups, func() { os.RemoveAll(repoDir) })
+func downloadVCSZip(t testing.TB, vcs, url, rev, subdir string) (repoDir string, dl *os.File, err error) {
+ repoDir = t.TempDir()
switch vcs {
case "git":
// Create a repository and download the revision we want.
- if err := run(repoDir, "git", "init", "--bare"); err != nil {
- return "", nil, cleanup, err
+ if _, err := run(t, repoDir, "git", "init", "--bare"); err != nil {
+ return "", nil, err
}
if err := os.MkdirAll(filepath.Join(repoDir, "info"), 0777); err != nil {
- return "", nil, cleanup, err
+ return "", nil, err
}
attrFile, err := os.OpenFile(filepath.Join(repoDir, "info", "attributes"), os.O_CREATE|os.O_APPEND|os.O_RDWR, 0666)
if err != nil {
- return "", nil, cleanup, err
+ return "", nil, err
}
if _, err := attrFile.Write([]byte("\n* -export-subst -export-ignore\n")); err != nil {
attrFile.Close()
- return "", nil, cleanup, err
+ return "", nil, err
}
if err := attrFile.Close(); err != nil {
- return "", nil, cleanup, err
+ return "", nil, err
}
- if err := run(repoDir, "git", "remote", "add", "origin", "--", url); err != nil {
- return "", nil, cleanup, err
+ if _, err := run(t, repoDir, "git", "remote", "add", "origin", "--", url); err != nil {
+ return "", nil, err
}
var refSpec string
if strings.HasPrefix(rev, "v") {
@@ -1420,86 +1396,105 @@
} else {
refSpec = fmt.Sprintf("%s:refs/dummy", rev)
}
- if err := run(repoDir, "git", "fetch", "-f", "--depth=1", "origin", refSpec); err != nil {
- return "", nil, cleanup, err
+ if _, err := run(t, repoDir, "git", "fetch", "-f", "--depth=1", "origin", refSpec); err != nil {
+ return "", nil, err
}
// Create an archive.
- tmpZipFile, err := ioutil.TempFile("", "downloadVCSZip-*.zip")
+ zipPath := filepath.Join(t.TempDir(), "vcs.zip")
+ tmpZipFile, err := os.Create(zipPath)
if err != nil {
- return "", nil, cleanup, err
+ return "", nil, err
}
- cleanups = append(cleanups, func() {
- name := tmpZipFile.Name()
- tmpZipFile.Close()
- os.Remove(name)
- })
+ t.Cleanup(func() { tmpZipFile.Close() })
subdirArg := subdir
if subdir == "" {
subdirArg = "."
}
+
cmd := exec.Command("git", "-c", "core.autocrlf=input", "-c", "core.eol=lf", "archive", "--format=zip", "--prefix=prefix/", rev, "--", subdirArg)
cmd.Dir = repoDir
cmd.Stdout = tmpZipFile
- if err := cmd.Run(); err != nil {
- return "", nil, cleanup, err
+ stderr := new(strings.Builder)
+ cmd.Stderr = stderr
+
+ err = cmd.Run()
+ if stderr.Len() > 0 && (err != nil || testing.Verbose()) {
+ t.Logf("%v: %v\n%s", err, cmd, stderr)
+ } else if err != nil {
+ t.Logf("%v: %v", err, cmd)
+ } else {
+ t.Logf("%v", cmd)
}
+ if err != nil {
+ return "", nil, err
+ }
+
if _, err := tmpZipFile.Seek(0, 0); err != nil {
- return "", nil, cleanup, err
+ return "", nil, err
}
- return repoDir, tmpZipFile, cleanup, nil
+ return repoDir, tmpZipFile, nil
case "hg":
// Clone the whole repository.
- if err := run(repoDir, "hg", "clone", "-U", "--", url, "."); err != nil {
- return "", nil, cleanup, err
+ if _, err := run(t, repoDir, "hg", "clone", "-U", "--", url, "."); err != nil {
+ return "", nil, err
}
// Create an archive.
tmpZipFile, err := ioutil.TempFile("", "downloadVCSZip-*.zip")
if err != nil {
- return "", nil, cleanup, err
+ return "", nil, err
}
tmpZipPath := tmpZipFile.Name()
tmpZipFile.Close()
- cleanups = append(cleanups, func() { os.Remove(tmpZipPath) })
+ t.Cleanup(func() { os.Remove(tmpZipPath) })
args := []string{"archive", "-t", "zip", "--no-decode", "-r", rev, "--prefix=prefix/"}
if subdir != "" {
args = append(args, "-I", subdir+"/**")
}
args = append(args, "--", tmpZipPath)
- if err := run(repoDir, "hg", args...); err != nil {
- return "", nil, cleanup, err
+ if _, err := run(t, repoDir, "hg", args...); err != nil {
+ return "", nil, err
}
if tmpZipFile, err = os.Open(tmpZipPath); err != nil {
- return "", nil, cleanup, err
+ return "", nil, err
}
- cleanups = append(cleanups, func() { tmpZipFile.Close() })
- return repoDir, tmpZipFile, cleanup, err
+ t.Cleanup(func() { tmpZipFile.Close() })
+ return repoDir, tmpZipFile, err
default:
- return "", nil, cleanup, fmt.Errorf("vcs %q not supported", vcs)
+ return "", nil, fmt.Errorf("vcs %q not supported", vcs)
}
}
-func downloadVCSFile(vcs, repo, rev, file string) ([]byte, error) {
+func downloadVCSFile(t testing.TB, vcs, repo, rev, file string) ([]byte, error) {
+ t.Helper()
switch vcs {
case "git":
- cmd := exec.Command("git", "cat-file", "blob", rev+":"+file)
- cmd.Dir = repo
- return cmd.Output()
+ return run(t, repo, "git", "cat-file", "blob", rev+":"+file)
default:
return nil, fmt.Errorf("vcs %q not supported", vcs)
}
}
-func run(dir string, name string, args ...string) error {
+func run(t testing.TB, dir string, name string, args ...string) ([]byte, error) {
+ t.Helper()
+
cmd := exec.Command(name, args...)
cmd.Dir = dir
- if err := cmd.Run(); err != nil {
- return fmt.Errorf("%s: %v", strings.Join(args, " "), err)
+ stderr := new(strings.Builder)
+ cmd.Stderr = stderr
+
+ out, err := cmd.Output()
+ if stderr.Len() > 0 && (err != nil || testing.Verbose()) {
+ t.Logf("%v: %v\n%s", err, cmd, stderr)
+ } else if err != nil {
+ t.Logf("%v: %v", err, cmd)
+ } else {
+ t.Logf("%v", cmd)
}
- return nil
+ return out, err
}
type zipFile struct {
@@ -1515,7 +1510,7 @@
mustHaveGit(t)
// Write files to a temporary directory.
- tmpDir, cleanup, err := extractTxtarToTempDir(txtar.Parse([]byte(`-- go.mod --
+ tmpDir, err := extractTxtarToTempDir(t, txtar.Parse([]byte(`-- go.mod --
module example.com/foo/bar
go 1.12
@@ -1541,7 +1536,6 @@
if err != nil {
t.Fatal(err)
}
- defer cleanup()
gitInit(t, tmpDir)
gitCommit(t, tmpDir)
@@ -1612,7 +1606,7 @@
mustHaveGit(t)
// Write files to a temporary directory.
- tmpDir, cleanup, err := extractTxtarToTempDir(txtar.Parse([]byte(`-- go.mod --
+ tmpDir, err := extractTxtarToTempDir(t, txtar.Parse([]byte(`-- go.mod --
module example.com/foo/bar
go 1.12
@@ -1645,7 +1639,6 @@
if err != nil {
t.Fatal(err)
}
- defer cleanup()
gitInit(t, tmpDir)
gitCommit(t, tmpDir)
@@ -1695,7 +1688,7 @@
mustHaveGit(t)
// Write files to a temporary directory.
- tmpDir, cleanup, err := extractTxtarToTempDir(txtar.Parse([]byte(`-- go.mod --
+ tmpDir, err := extractTxtarToTempDir(t, txtar.Parse([]byte(`-- go.mod --
module example.com/foo/bar
go 1.12
@@ -1707,7 +1700,6 @@
if err != nil {
t.Fatal(err)
}
- defer cleanup()
// Create zip from the directory.
tmpZip, err := ioutil.TempFile("", "TestCreateFromDir-*.zip")
@@ -1738,7 +1730,7 @@
mustHaveGit(t)
// Write files to a temporary directory.
- tmpDir, cleanup, err := extractTxtarToTempDir(txtar.Parse([]byte(`-- go.mod --
+ tmpDir, err := extractTxtarToTempDir(t, txtar.Parse([]byte(`-- go.mod --
module example.com/foo/bar
go 1.12
@@ -1750,7 +1742,6 @@
if err != nil {
t.Fatal(err)
}
- defer cleanup()
gitInit(t, tmpDir)
@@ -1776,47 +1767,29 @@
//
// Note: some environments - and trybots - don't have git installed. This
// function will cause the calling test to be skipped if that's the case.
-func gitInit(t *testing.T, dir string) {
+func gitInit(t testing.TB, dir string) {
t.Helper()
mustHaveGit(t)
- cmd := exec.Command("git", "init")
- cmd.Dir = dir
- cmd.Stderr = os.Stderr
- if err := cmd.Run(); err != nil {
+ if _, err := run(t, dir, "git", "init"); err != nil {
t.Fatal(err)
}
-
- cmd = exec.Command("git", "config", "user.email", "testing@golangtests.com")
- cmd.Dir = dir
- cmd.Stderr = os.Stderr
- if err := cmd.Run(); err != nil {
+ if _, err := run(t, dir, "git", "config", "user.name", "Go Gopher"); err != nil {
t.Fatal(err)
}
-
- cmd = exec.Command("git", "config", "user.name", "This is the zip Go tests")
- cmd.Dir = dir
- cmd.Stderr = os.Stderr
- if err := cmd.Run(); err != nil {
+ if _, err := run(t, dir, "git", "config", "user.email", "gopher@golang.org"); err != nil {
t.Fatal(err)
}
}
-func gitCommit(t *testing.T, dir string) {
+func gitCommit(t testing.TB, dir string) {
t.Helper()
mustHaveGit(t)
- cmd := exec.Command("git", "add", "-A")
- cmd.Dir = dir
- cmd.Stderr = os.Stderr
- if err := cmd.Run(); err != nil {
+ if _, err := run(t, dir, "git", "add", "-A"); err != nil {
t.Fatal(err)
}
-
- cmd = exec.Command("git", "commit", "-m", "some commit")
- cmd.Dir = dir
- cmd.Stderr = os.Stderr
- if err := cmd.Run(); err != nil {
+ if _, err := run(t, dir, "git", "commit", "-m", "some commit"); err != nil {
t.Fatal(err)
}
}
