diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5887242..46f40c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -53,8 +53,13 @@ jobs: - name: Vet run: go vet ./... + # -race and -shuffle because the suite shares package state (the cobra + # tree, swappable package vars, time.Local) and both a race on it and a + # test that only passed after another one went unseen until #55. A + # shuffle failure prints its -test.shuffle seed: rerun with that seed to + # reproduce the order. - name: Test - run: go test ./... + run: go test -race -shuffle=on ./... # The release builds four targets from this same source. A build that # only works on the runner's own platform is worth knowing about before diff --git a/cmd/automations_test.go b/cmd/automations_test.go index af5cdef..561f369 100644 --- a/cmd/automations_test.go +++ b/cmd/automations_test.go @@ -617,9 +617,18 @@ func TestBrowserSessionRequiredSaysWhy(t *testing.T) { } // execute runs the real command tree, as main does, and returns how it -// would exit. +// would exit. The tree is package state: its flags start at their defaults +// and go back to them when the test ends, so no test sees what another one +// parsed, whatever order they run in. func execute(t *testing.T, args ...string) (int, string) { t.Helper() + resetCommandFlags(rootCmd) + t.Cleanup(func() { + resetCommandFlags(rootCmd) + rootCmd.SetArgs(nil) + rootCmd.SetOut(nil) + rootCmd.SetErr(nil) + }) var out bytes.Buffer rootCmd.SetArgs(args) rootCmd.SetOut(&out) diff --git a/cmd/byok_test.go b/cmd/byok_test.go index d5cfa86..b8b3bb4 100644 --- a/cmd/byok_test.go +++ b/cmd/byok_test.go @@ -11,7 +11,6 @@ import ( "path/filepath" "strings" "testing" - "time" "github.com/nxssie/nan-cli/internal/api" ) @@ -237,9 +236,6 @@ func checkGolden(t *testing.T, rel, got string) { func TestBYOKGoldenViews(t *testing.T) { resetPhase2Flags() defer resetPhase2Flags() - loc := time.Local - time.Local = time.UTC - defer func() { time.Local = loc }() ctx := context.Background() for _, tc := range []struct { diff --git a/cmd/main_test.go b/cmd/main_test.go new file mode 100644 index 0000000..08862ef --- /dev/null +++ b/cmd/main_test.go @@ -0,0 +1,27 @@ +package cmd + +import ( + "os" + "testing" + "time" +) + +// TestMain pins the clock's zone before any test or goroutine starts. +// +// Rendered times (runs ls, golden views) go through t.Local(), so the tests +// need one fixed zone. Swapping time.Local inside a test races with every +// goroutine that calls time.Now() meanwhile, including net/http servers left +// over from earlier tests; set once here, nothing reads it concurrently. +func TestMain(m *testing.M) { + time.Local = time.UTC + os.Exit(m.Run()) +} + +// swap sets *p to v for the rest of the test and puts the old value back +// when it ends, whatever order the tests run in. +func swap[T any](t testing.TB, p *T, v T) { + t.Helper() + old := *p + *p = v + t.Cleanup(func() { *p = old }) +} diff --git a/cmd/review_test.go b/cmd/review_test.go index f649769..815856e 100644 --- a/cmd/review_test.go +++ b/cmd/review_test.go @@ -236,14 +236,7 @@ func TestJSONOutputNeutralisesALoneC1Byte(t *testing.T) { func TestContradictoryWorktreeFlagsSayWhatIsWrong(t *testing.T) { t.Setenv("HOME", t.TempDir()) - rootCmd.SetArgs([]string{"run", "--worktree", "--no-worktree", "x"}) - rootCmd.SetOut(io.Discard) - rootCmd.SetErr(io.Discard) - cmd, err := rootCmd.ExecuteC() - code, msg := exitCodeFor(cmd, err) - runOpts = runOptions{agent: "pi", timeout: 30 * time.Minute} - _ = runCmd.Flags().Set("worktree", "false") - _ = runCmd.Flags().Set("no-worktree", "false") + code, msg := execute(t, "run", "--worktree", "--no-worktree", "x") if code != exitUsage || !strings.Contains(msg, "pick one") { t.Errorf("exit %d: %s", code, msg) } diff --git a/cmd/root_test.go b/cmd/root_test.go index 29b4d15..4c4678f 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,7 +1,6 @@ package cmd import ( - "bytes" "testing" "github.com/nxssie/nan-cli/internal/tui" @@ -10,24 +9,18 @@ import ( // `nan --version` and `nan -v` print one line and exit 0, without opening // the dashboard. func TestVersionPrintsOneLine(t *testing.T) { - old := openTUI - defer func() { openTUI = old }() - openTUI = func() error { + swap(t, &openTUI, func() error { t.Error("--version opened the dashboard") return nil - } + }) for _, flag := range []string{"--version", "-v"} { - var out bytes.Buffer - rootCmd.SetArgs([]string{flag}) - rootCmd.SetOut(&out) - rootCmd.SetErr(&out) - if err := rootCmd.Execute(); err != nil { - t.Fatalf("%s: %v", flag, err) + code, out := execute(t, flag) + if code != 0 { + t.Fatalf("%s: exit %d: %s", flag, code, out) } - if got, want := out.String(), "nan "+tui.Version+"\n"; got != want { + if got, want := out, "nan "+tui.Version+"\n"; got != want { t.Errorf("%s printed %q, want %q", flag, got, want) } - _ = rootCmd.Flags().Set("version", "false") } } diff --git a/cmd/run_test.go b/cmd/run_test.go index c81a42f..313600d 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -665,17 +665,9 @@ func TestCobraRefusalsExit64OnRunCommands(t *testing.T) { {"runs", "show"}, {"runs", "logs", "a", "b"}, } { - rootCmd.SetArgs(args) - rootCmd.SetOut(io.Discard) - rootCmd.SetErr(io.Discard) - cmd, err := rootCmd.ExecuteC() - code, _ := exitCodeFor(cmd, err) - if err == nil || code != exitUsage { - t.Errorf("%v: exit %d (%v)", args, code, err) - } - runOpts = runOptions{agent: "pi", timeout: 30 * time.Minute} - _ = runCmd.Flags().Set("worktree", "false") - _ = runCmd.Flags().Set("no-worktree", "false") + if code, msg := execute(t, args...); code != exitUsage { + t.Errorf("%v: exit %d (%s)", args, code, msg) + } } if code, _ := exitCodeFor(meCmd, errors.New("boom")); code != 1 { t.Errorf("an ordinary command's failure exits %d", code) diff --git a/cmd/writeapi_test.go b/cmd/writeapi_test.go index 93bf811..77d916d 100644 --- a/cmd/writeapi_test.go +++ b/cmd/writeapi_test.go @@ -14,7 +14,6 @@ import ( "strings" "sync" "testing" - "time" "github.com/nxssie/nan-cli/internal/api" @@ -313,21 +312,13 @@ func runWrite(t *testing.T, fake *fakeWrite, setup writeSetup, args ...string) w home := fakeHome(t, setup.session) t.Setenv(tokenEnvVar, setup.env) - oldBase, oldIn, oldTTY, oldOut, oldErr, oldAttempts := runsBaseURL, writeStdin, writeStdinTTY, writeStdout, writeStderr, createAttempts - t.Cleanup(func() { - runsBaseURL, writeStdin, writeStdinTTY, writeStdout, writeStderr, createAttempts = oldBase, oldIn, oldTTY, oldOut, oldErr, oldAttempts - resetCommandFlags(rootCmd) - }) - runsBaseURL = srv.URL - loc := time.Local - time.Local = time.UTC - t.Cleanup(func() { time.Local = loc }) - writeStdin = strings.NewReader(setup.stdin) + swap(t, &runsBaseURL, srv.URL) + swap(t, &writeStdin, io.Reader(strings.NewReader(setup.stdin))) tty := setup.tty - writeStdinTTY = func() bool { return tty } + swap(t, &writeStdinTTY, func() bool { return tty }) var out, errOut bytes.Buffer - writeStdout, writeStderr = &out, &errOut - resetCommandFlags(rootCmd) + swap(t, &writeStdout, io.Writer(&out)) + swap(t, &writeStderr, io.Writer(&errOut)) code, msg := execute(t, args...) return writeRun{code: code, msg: msg, stdout: out.String(), stderr: errOut.String(), home: home} @@ -434,14 +425,13 @@ func TestSecretsSetReadsTheValueExactly(t *testing.T) { "abc": "abc", " a b c \n": " a b c ", } { - writeStdin = strings.NewReader(in) - writeStdinTTY = func() bool { return false } + swap(t, &writeStdin, io.Reader(strings.NewReader(in))) + swap(t, &writeStdinTTY, func() bool { return false }) got, err := readSecretValue("", true) if err != nil || got != want { t.Errorf("%q: got %q, %v", in, got, err) } } - writeStdin, writeStdinTTY = os.Stdin, func() bool { return isTerminal(os.Stdin) } } func TestSecretsSetRefusals(t *testing.T) { @@ -803,8 +793,7 @@ func TestReadBodyFile(t *testing.T) { t.Errorf("%s: %v", name, err) } } - writeStdinTTY = func() bool { return true } - defer func() { writeStdinTTY = func() bool { return isTerminal(os.Stdin) } }() + swap(t, &writeStdinTTY, func() bool { return true }) if _, err := readBodyFile("-"); exitCode(err) != exitUsage || !strings.Contains(err.Error(), "terminal") { t.Errorf("tty stdin: %v", err) } @@ -1051,7 +1040,7 @@ func TestSecretsSetWithoutRetryFailsOnALostAnswer(t *testing.T) { fake := newFakeWrite() fake.dropFirstCreate = true file := writeValueFile(t, testValue) - createAttempts = 1 + swap(t, &createAttempts, 1) r := runWrite(t, fake, writeSetup{session: cliSession}, "secrets", "set", "NEW_TOKEN", "--value-file", file) if r.code == 0 || len(fake.sent("POST", "/v1/secrets")) != 1 { t.Fatalf("exit %d, posts %v", r.code, fake.sent("POST", "/v1/secrets")) @@ -1144,9 +1133,8 @@ func TestSecretWriteNotFound(t *testing.T) { } func TestSecretValueSizeLimit(t *testing.T) { - defer func() { writeStdin, writeStdinTTY = os.Stdin, func() bool { return isTerminal(os.Stdin) } }() - writeStdinTTY = func() bool { return false } - writeStdin = strings.NewReader(strings.Repeat("a", maxSecretValue) + "\n") + swap(t, &writeStdinTTY, func() bool { return false }) + swap(t, &writeStdin, io.Reader(strings.NewReader(strings.Repeat("a", maxSecretValue)+"\n"))) if v, err := readSecretValue("", true); err != nil || len(v) != maxSecretValue { t.Errorf("16 KiB: %d %v", len(v), err) }