diff --git a/README.md b/README.md index 0f44d03..b6e549c 100644 --- a/README.md +++ b/README.md @@ -604,8 +604,10 @@ provide: - `script/build` — build the `secret` binary into the repo root, stamping the version (`VERSION` from the environment, else `git describe`) and the git commit -- `script/test` — run `go vet` and the test suite (verbose rerun on failure), - every test on every run, never a result from Go's test cache +- `script/test` — run `go vet`, then the test suite with the race detector, a + 30-second timeout per package and coverage, every test on every run, never a + result from Go's test cache; on failure it reruns the tests verbosely for the + details and fails even when the rerun passes - `script/lint` — run `golangci-lint` in docker only: builds `Dockerfile.lint`, where the linter is a build step that runs on every call, also on an unchanged tree diff --git a/TODO.md b/TODO.md index e386516..fb587e5 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,15 @@ https://git.eeqj.de/sneak/secret/milestone/12 # Completed Steps +- 2026-10-06: `script/test` runs the tests with the race detector, a 30-second + timeout per package and coverage, as `REPO_POLICIES.md` requires + (https://git.eeqj.de/sneak/secret/issues/32). When they fail, it reruns them + verbosely for the details and then fails anyway, so a test that fails once and + passes on the retry no longer gives a green build. `go vet` still runs first, + and every `go test` keeps `-count=1`. The tests that gave the `secret` binary + a minute now give it 10 seconds, and the PGP unlocker test's 30-second timer + is 10 seconds, so a test that hangs fails with its own message before the + package's 30-second timeout ends every test in it. - 2026-10-06: `make test` in `script/cibuild` no longer compiles the standard library and every dependency from nothing on every build (https://git.eeqj.de/sneak/secret/issues/124). The `Dockerfile` runs it and diff --git a/internal/cli/entry_test.go b/internal/cli/entry_test.go index 0e036cd..3889936 100644 --- a/internal/cli/entry_test.go +++ b/internal/cli/entry_test.go @@ -8,7 +8,6 @@ import ( "path/filepath" "strings" "testing" - "time" "github.com/awnumar/memguard" "github.com/stretchr/testify/assert" @@ -52,7 +51,7 @@ func TestInterruptExitsThroughMemguard(t *testing.T) { const waitingForValue = "Reading secret value from stdin" - ctx, cancel := context.WithTimeout(t.Context(), time.Minute) + ctx, cancel := context.WithTimeout(t.Context(), commandWait) defer cancel() wd, err := filepath.Abs("../..") diff --git a/internal/cli/integration_test.go b/internal/cli/integration_test.go index df140a8..3507aa6 100644 --- a/internal/cli/integration_test.go +++ b/internal/cli/integration_test.go @@ -32,6 +32,12 @@ const ( // testMnemonic is a standard BIP39 mnemonic used for testing //nolint:dupword // BIP39 test mnemonic intentionally repeats a word testMnemonic = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about" + + // commandWait is how long a test lets the secret binary run before it + // kills it and fails. It stays well under the 30 seconds script/test + // gives the whole package, so a command that hangs fails the test with + // the test's own message instead of Go's timeout panic. + commandWait = 10 * time.Second ) // errEmptyValue indicates a concurrent reader received an empty secret value. @@ -2583,7 +2589,7 @@ func TestRemoveWithoutTerminalFailsAtOnce(t *testing.T) { _ = stdin.Close() }() - ctx, cancel := context.WithTimeout(t.Context(), time.Minute) + ctx, cancel := context.WithTimeout(t.Context(), commandWait) defer cancel() cmd, secretDir := secretRmCommand(ctx, t) @@ -2602,9 +2608,10 @@ func TestRemoveWithoutTerminalFailsAtOnce(t *testing.T) { // the answer is read from. pty.Open returns the two ends of a new terminal: // tty is the end a program uses as its terminal, and ptmx the end the test // reads what the terminal shows from and types into. Reading ptmx stops at -// the context's deadline, when secret rm is killed too, so a terminal that -// stays open fails the test then instead of hanging it. The deadline works -// only while ptmx stays non-blocking, as pty.Open of the github.com/creack/pty +// the context's deadline, commandWait (10 seconds) after the test starts, +// when secret rm is killed too, so a terminal that stays open fails the test +// then with its own message instead of hanging it. The deadline works only +// while ptmx stays non-blocking, as pty.Open of the github.com/creack/pty // commit in go.mod leaves it: calling ptmx.Fd() or going back to v1.1.24 // makes the read ignore the deadline, without any error. @@ -2614,7 +2621,7 @@ func TestRemoveWithoutTerminalFailsAtOnce(t *testing.T) { func TestRemoveIgnoresTerminalOnStdout(t *testing.T) { t.Parallel() - ctx, cancel := context.WithTimeout(t.Context(), time.Minute) + ctx, cancel := context.WithTimeout(t.Context(), commandWait) defer cancel() cmd, secretDir := secretRmCommand(ctx, t) @@ -2638,12 +2645,13 @@ func TestRemoveIgnoresTerminalOnStdout(t *testing.T) { // The read ends once secret rm has exited and so closed the terminal. shown, err := io.ReadAll(ptmx) require.NotErrorIs(t, err, os.ErrDeadlineExceeded, - "the terminal was still open a minute after secret rm started: %s", - shown) + "the terminal was still open %s after secret rm started: %s", + commandWait, shown) err = cmd.Wait() - require.NoError(t, ctx.Err(), "secret rm did not exit within a minute") + require.NoError(t, ctx.Err(), "secret rm did not exit within %s", + commandWait) require.Error(t, err) assert.Contains(t, string(shown), "pass --force") assert.DirExists(t, secretDir) @@ -2654,7 +2662,7 @@ func TestRemoveIgnoresTerminalOnStdout(t *testing.T) { func TestRemoveAsksAtTerminalOnStdin(t *testing.T) { t.Parallel() - ctx, cancel := context.WithTimeout(t.Context(), time.Minute) + ctx, cancel := context.WithTimeout(t.Context(), commandWait) defer cancel() cmd, secretDir := secretRmCommand(ctx, t) diff --git a/internal/secret/pgpunlock_test.go b/internal/secret/pgpunlock_test.go index 72d1e30..cd2b955 100644 --- a/internal/secret/pgpunlock_test.go +++ b/internal/secret/pgpunlock_test.go @@ -317,8 +317,8 @@ func testCreatePGPUnlocker( t.Helper() // Set a limited test timeout to avoid hanging - timer := time.AfterFunc(30*time.Second, func() { - t.Fatalf("Test timed out after 30 seconds") + timer := time.AfterFunc(10*time.Second, func() { + t.Fatalf("Test timed out after 10 seconds") }) defer timer.Stop() diff --git a/script/test b/script/test index 7a48c8b..f2a4ff4 100755 --- a/script/test +++ b/script/test @@ -1,5 +1,6 @@ #!/bin/sh -# script/test: run the test suite (vet first, verbose rerun on failure). +# script/test: run the test suite (vet first, then the tests with the race +# detector; a verbose rerun on failure). set -eu ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" @@ -10,8 +11,12 @@ main() { export CGO_ENABLED=1 go vet ./... # -count=1: run every test, never take a result from Go's test cache, - # which the Dockerfile keeps between builds - go test -count=1 ./... || go test -count=1 -v ./... + # which the Dockerfile keeps between builds. The rerun only prints + # details: `exit 1` keeps the script failing even if a flaky test + # passes on the second attempt. + go test -count=1 -timeout 30s -race -cover ./... || \ + { echo "--- Rerunning with -v for details ---"; \ + go test -count=1 -timeout 30s -race -v ./...; exit 1; } } main "$@"