cmd/gorelease: fix tests for race builders Change-Id: I670413d295fca0ab227088d643fa1d154288fdf3 Reviewed-on: https://go-review.googlesource.com/c/exp/+/327909 Trust: Ian Cottrell <iancottrell@google.com> Trust: Jean de Klerk <deklerk@google.com> Run-TryBot: Ian Cottrell <iancottrell@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jean de Klerk <deklerk@google.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
diff --git a/cmd/gorelease/gorelease.go b/cmd/gorelease/gorelease.go index 49d7e87..9ab8f12 100644 --- a/cmd/gorelease/gorelease.go +++ b/cmd/gorelease/gorelease.go
@@ -634,9 +634,7 @@ Versions []string `json: "Versions"` } cmd := exec.CommandContext(ctx, "go", "list", "-json", "-m", "-versions", modPath) - if env, ok := ctx.Value("env").([]string); ok { - cmd.Env = env - } + cmd.Env = copyEnv(ctx, cmd.Env) cmd.Dir = modRoot out, err := cmd.Output() if err != nil { @@ -763,9 +761,7 @@ }() arg := modPath + "@" + query cmd := exec.CommandContext(ctx, "go", "list", "-m", "-f", "{{.Version}}", "--", arg) - if env, ok := ctx.Value("env").([]string); ok { - cmd.Env = env - } + cmd.Env = copyEnv(ctx, cmd.Env) cmd.Dir = tmpDir cmd.Env = append(cmd.Env, "GO111MODULE=on") out, err := cmd.Output() @@ -795,9 +791,7 @@ } }() cmd := exec.CommandContext(ctx, "go", "list", "-m", "-versions", "--", modPath) - if env, ok := ctx.Value("env").([]string); ok { - cmd.Env = env - } + cmd.Env = copyEnv(ctx, cmd.Env) cmd.Dir = tmpDir cmd.Env = append(cmd.Env, "GO111MODULE=on") out, err := cmd.Output() @@ -922,9 +916,7 @@ } defer os.Remove(tmpDir) cmd := exec.CommandContext(ctx, "go", "mod", "download", "-json", "--", m.Path+"@"+m.Version) - if env, ok := ctx.Value("env").([]string); ok { - cmd.Env = env - } + cmd.Env = copyEnv(ctx, cmd.Env) cmd.Dir = tmpDir out, err := cmd.Output() var xerr *exec.ExitError @@ -1063,9 +1055,7 @@ // Add missing requirements. cmd := exec.CommandContext(ctx, "go", "get", "-d", ".") - if env, ok := ctx.Value("env").([]string); ok { - cmd.Env = env - } + cmd.Env = copyEnv(ctx, cmd.Env) cmd.Dir = dir if _, err := cmd.Output(); err != nil { return "", nil, nil, nil, nil, fmt.Errorf("looking for missing dependencies: %w", cleanCmdError(err)) @@ -1269,9 +1259,7 @@ Dir: loadDir, Context: ctx, } - if env, ok := ctx.Value("env").([]string); ok { - cfg.Env = env - } + cfg.Env = copyEnv(ctx, cfg.Env) if len(pkgPaths) > 0 { pkgs, err = packages.Load(cfg, pkgPaths...) if err != nil { @@ -1353,9 +1341,7 @@ }() cmd := exec.CommandContext(ctx, "go", "list", "-m", "-f", "{{.Version}}", "--", modPath) - if env, ok := ctx.Value("env").([]string); ok { - cmd.Env = env - } + cmd.Env = copyEnv(ctx, cmd.Env) cmd.Dir = modDir out, err := cmd.Output() if err != nil { @@ -1363,3 +1349,13 @@ } return strings.TrimSpace(string(out)), nil } + +func copyEnv(ctx context.Context, current []string) []string { + env, ok := ctx.Value("env").([]string) + if !ok { + return current + } + clone := make([]string, len(env)) + copy(clone, env) + return clone +}
diff --git a/cmd/gorelease/gorelease_test.go b/cmd/gorelease/gorelease_test.go index 10c1080..c648816 100644 --- a/cmd/gorelease/gorelease_test.go +++ b/cmd/gorelease/gorelease_test.go
@@ -269,102 +269,104 @@ t.Cleanup(cleanup) for _, test := range tests { - test := test testName := strings.TrimSuffix(strings.TrimPrefix(filepath.ToSlash(test.testPath), "testdata/"), ".test") - t.Run(testName, func(t *testing.T) { - ctx := defaultContext + t.Run(testName, testRelease(defaultContext, tests, test)) + } +} - if test.skip != "" { - t.Skip(test.skip) +func testRelease(ctx context.Context, tests []*test, test *test) func(t *testing.T) { + return func(t *testing.T) { + if test.skip != "" { + t.Skip(test.skip) + } + + t.Parallel() + + if len(test.proxyVersions) > 0 { + var cleanup func() + var err error + ctx, cleanup, err = prepareProxy(test.proxyVersions, tests) + if err != nil { + t.Fatalf("preparing test proxy: %v", err) } + t.Cleanup(cleanup) + } - t.Parallel() + // Extract the files in the release version. They may be part of the + // test archive or in testdata/mod. + testDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatal(err) + } + if *testwork { + fmt.Fprintf(os.Stderr, "test dir: %s\n", testDir) + } else { + t.Cleanup(func() { + os.RemoveAll(testDir) + }) + } - if len(test.proxyVersions) > 0 { - var cleanup func() - ctx, cleanup, err = prepareProxy(test.proxyVersions, tests) - if err != nil { - t.Fatalf("preparing test proxy: %v", err) - } - t.Cleanup(cleanup) - } - - // Extract the files in the release version. They may be part of the - // test archive or in testdata/mod. - testDir, err := ioutil.TempDir("", "") + var arc *txtar.Archive + if test.version != "" { + arcBase := fmt.Sprintf("%s_%s.txt", strings.ReplaceAll(test.modPath, "/", "_"), test.version) + arcPath := filepath.Join("testdata/mod", arcBase) + var err error + arc, err = txtar.ParseFile(arcPath) if err != nil { t.Fatal(err) } - if *testwork { - fmt.Fprintf(os.Stderr, "test dir: %s\n", testDir) - } else { - t.Cleanup(func() { - os.RemoveAll(testDir) - }) - } + } else { + arc = &test.Archive + } + if err := extractTxtar(testDir, arc); err != nil { + t.Fatal(err) + } - var arc *txtar.Archive - if test.version != "" { - arcBase := fmt.Sprintf("%s_%s.txt", strings.ReplaceAll(test.modPath, "/", "_"), test.version) - arcPath := filepath.Join("testdata/mod", arcBase) - var err error - arc, err = txtar.ParseFile(arcPath) - if err != nil { - t.Fatal(err) - } - } else { - arc = &test.Archive + // Generate the report and compare it against the expected text. + var args []string + if test.baseVersion != "" { + args = append(args, "-base="+test.baseVersion) + } + if test.releaseVersion != "" { + args = append(args, "-version="+test.releaseVersion) + } + buf := &bytes.Buffer{} + releaseDir := filepath.Join(testDir, test.dir) + success, err := runRelease(ctx, buf, releaseDir, args) + if err != nil { + if !test.wantError { + t.Fatalf("unexpected error: %v", err) } - if err := extractTxtar(testDir, arc); err != nil { - t.Fatal(err) - } - - // Generate the report and compare it against the expected text. - var args []string - if test.baseVersion != "" { - args = append(args, "-base="+test.baseVersion) - } - if test.releaseVersion != "" { - args = append(args, "-version="+test.releaseVersion) - } - buf := &bytes.Buffer{} - releaseDir := filepath.Join(testDir, test.dir) - success, err := runRelease(ctx, buf, releaseDir, args) - if err != nil { - if !test.wantError { - t.Fatalf("unexpected error: %v", err) - } - if errMsg := []byte(err.Error()); !bytes.Equal(errMsg, bytes.TrimSpace(test.want)) { - if *updateGolden { - if err := updateTest(test, errMsg); err != nil { - t.Fatal(err) - } - } else { - t.Fatalf("got error: %s; want error: %s", errMsg, test.want) - } - } - return - } - if test.wantError { - t.Fatalf("got success; want error %s", test.want) - } - - got := bytes.TrimSpace(buf.Bytes()) - if filepath.Separator != '/' { - got = bytes.ReplaceAll(got, []byte{filepath.Separator}, []byte{'/'}) - } - if !bytes.Equal(got, test.want) { + if errMsg := []byte(err.Error()); !bytes.Equal(errMsg, bytes.TrimSpace(test.want)) { if *updateGolden { - if err := updateTest(test, got); err != nil { + if err := updateTest(test, errMsg); err != nil { t.Fatal(err) } } else { - t.Fatalf("got:\n%s\n\nwant:\n%s", got, test.want) + t.Fatalf("got error: %s; want error: %s", errMsg, test.want) } } - if success != test.wantSuccess { - t.Fatalf("got success: %v; want success %v", success, test.wantSuccess) + return + } + if test.wantError { + t.Fatalf("got success; want error %s", test.want) + } + + got := bytes.TrimSpace(buf.Bytes()) + if filepath.Separator != '/' { + got = bytes.ReplaceAll(got, []byte{filepath.Separator}, []byte{'/'}) + } + if !bytes.Equal(got, test.want) { + if *updateGolden { + if err := updateTest(test, got); err != nil { + t.Fatal(err) + } + } else { + t.Fatalf("got:\n%s\n\nwant:\n%s", got, test.want) } - }) + } + if success != test.wantSuccess { + t.Fatalf("got success: %v; want success %v", success, test.wantSuccess) + } } }