Skip to content

Commit 0b453aa

Browse files
committed
fix(test): swap update.Action to no-op to avoid spinner-goroutine race [release]
CI on Go 1.26 darwin/arm64 caught a data race in TestRootBefore_UpdateSubcommandSuppressesNotice: runUpdate's uikit.RunWithSpinner spawns a goroutine to run its callback, and tea.Program's internal sync edge between Send(doneMsg) and the parent's Run() return isn't visible to the race detector — so when the test mocks selfupdateCheck/Update/Exec and cleanup restores them after buildRoot().Run() returns, the detector flags the read/write pair. The test only needs to verify that root's Before suppresses the notice when args target `update`. Swap update.Action to a no-op stub on the buildRoot result so Before still fires with the same Args().First() but no spinner goroutine spawns. Drop the no-longer-needed mocks. Verified locally with -count=5 -race.
1 parent 8342249 commit 0b453aa

1 file changed

Lines changed: 15 additions & 18 deletions

File tree

cmd/sci/root_test.go

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/sciminds/cli/internal/selfupdate"
1616
"github.com/sciminds/cli/internal/uikit"
1717
"github.com/sciminds/cli/internal/version"
18+
"github.com/urfave/cli/v3"
1819
)
1920

2021
// setupNoticeEnv redirects the selfupdate cache to a tempdir, pins
@@ -144,6 +145,14 @@ func TestRootBefore_JSONSuppressesNotice(t *testing.T) {
144145
// does not double-announce — the user is already running the updater.
145146
// Detection uses cmd.Args().First() because root's Before receives the
146147
// root command, not the resolved subcommand.
148+
//
149+
// The real update.Action calls uikit.RunWithSpinner which spawns a
150+
// goroutine to run its callback; tea.Program's internal sync edge isn't
151+
// visible to the race detector, so mocking selfupdateCheck/Update/Exec
152+
// and running the real Action via buildRoot triggers a flaky race in CI
153+
// (caught on Go 1.26 darwin/arm64). We don't need to exercise runUpdate
154+
// at all here — swap update.Action to a no-op so Before still fires with
155+
// the same Args().First() but no spinner goroutine spawns.
147156
func TestRootBefore_UpdateSubcommandSuppressesNotice(t *testing.T) {
148157
path := setupNoticeEnv(t)
149158
writeNoticeCache(t, path, selfupdate.CheckResult{
@@ -152,28 +161,16 @@ func TestRootBefore_UpdateSubcommandSuppressesNotice(t *testing.T) {
152161
LastCheckedAt: time.Now(),
153162
})
154163

155-
// Mock the update flow so we don't touch the network or re-exec.
156-
origCheck, origUpdate, origExec := selfupdateCheck, selfupdateUpdate, execAfterUpdate
157-
t.Cleanup(func() {
158-
selfupdateCheck = origCheck
159-
selfupdateUpdate = origUpdate
160-
execAfterUpdate = origExec
161-
})
162-
selfupdateCheck = func() selfupdate.CheckResult {
163-
return selfupdate.CheckResult{
164-
Available: true,
165-
CurrentSHA: "aaaaaaa1111111",
166-
LatestSHA: "bbbbbbb2222222",
167-
DownloadURL: "https://example.invalid/sci",
164+
root := buildRoot()
165+
for _, c := range root.Commands {
166+
if c.Name == "update" {
167+
c.Action = func(context.Context, *cli.Command) error { return nil }
168+
c.Before = nil
168169
}
169170
}
170-
selfupdateUpdate = func(_, _ string) (string, error) {
171-
return "/tmp/sci-new", nil
172-
}
173-
execAfterUpdate = func(_ string) error { return nil }
174171

175172
stderr := captureStderr(t, func() {
176-
_ = buildRoot().Run(context.Background(), []string{"sci", "update"})
173+
_ = root.Run(context.Background(), []string{"sci", "update"})
177174
})
178175

179176
if strings.Contains(stderr, "Update available") {

0 commit comments

Comments
 (0)