Skip to content

Commit aa5af29

Browse files
silv-iosilv-io
committed
Address comments
1 parent 9788b48 commit aa5af29

File tree

6 files changed

+191
-29
lines changed

6 files changed

+191
-29
lines changed

‎cmd/root.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"github.com/localstack/lstk/internal/update"
1919
"github.com/localstack/lstk/internal/version"
2020
"github.com/spf13/cobra"
21-
"github.com/spf13/viper"
2221
)
2322

2423
func NewRootCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.Command {
@@ -109,20 +108,18 @@ func runStart(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *teleme
109108
Logger: logger,
110109
}
111110

112-
notifyOpts := ui.UpdateNotifyOptions{
113-
GitHubToken: cfg.GitHubToken,
114-
UpdatePrompt: viper.GetBool("update_prompt"),
115-
PersistDisable: func() error {
116-
return config.Set("update_prompt", false)
117-
},
111+
notifyOpts := update.NotifyOptions{
112+
GitHubToken: cfg.GitHubToken,
113+
UpdatePrompt: config.IsUpdatePromptEnabled(),
114+
PersistDisable: config.DisableUpdatePrompt,
118115
}
119116

120117
if isInteractiveMode(cfg) {
121118
return ui.Run(ctx, rt, version.Version(), opts, notifyOpts)
122119
}
123120

124121
sink := output.NewPlainSink(os.Stdout)
125-
update.NotifyUpdate(ctx, sink, notifyOpts.GitHubToken, false, nil)
122+
update.NotifyUpdate(ctx, sink, update.NotifyOptions{GitHubToken: cfg.GitHubToken})
126123
return container.Start(ctx, rt, sink, opts, false)
127124
}
128125

‎internal/config/config.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,14 @@ func Set(key string, value any) error {
8181
return viper.WriteConfig()
8282
}
8383

84+
func IsUpdatePromptEnabled() bool {
85+
return viper.GetBool("update_prompt")
86+
}
87+
88+
func DisableUpdatePrompt() error {
89+
return Set("update_prompt", false)
90+
}
91+
8492
func Get() (*Config, error) {
8593
var cfg Config
8694
if err := viper.Unmarshal(&cfg); err != nil {

‎internal/ui/run.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,7 @@ func (s programSender) Send(msg any) {
2525
s.p.Send(msg)
2626
}
2727

28-
type UpdateNotifyOptions struct {
29-
GitHubToken string
30-
UpdatePrompt bool
31-
PersistDisable func() error
32-
}
33-
34-
func Run(parentCtx context.Context, rt runtime.Runtime, version string, opts container.StartOptions, notifyOpts UpdateNotifyOptions) error {
28+
func Run(parentCtx context.Context, rt runtime.Runtime, version string, opts container.StartOptions, notifyOpts update.NotifyOptions) error {
3529
ctx, cancel := context.WithCancel(parentCtx)
3630
defer cancel()
3731

@@ -53,7 +47,7 @@ func Run(parentCtx context.Context, rt runtime.Runtime, version string, opts con
5347
var err error
5448
defer func() { runErrCh <- err }()
5549
sink := output.NewTUISink(programSender{p: p})
56-
if update.NotifyUpdate(ctx, sink, notifyOpts.GitHubToken, notifyOpts.UpdatePrompt, notifyOpts.PersistDisable) {
50+
if update.NotifyUpdate(ctx, sink, notifyOpts) {
5751
p.Send(runDoneMsg{})
5852
return
5953
}

‎internal/update/notify.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ import (
1111

1212
type versionFetcher func(ctx context.Context, token string) (string, error)
1313

14+
type NotifyOptions struct {
15+
GitHubToken string
16+
UpdatePrompt bool
17+
PersistDisable func() error
18+
}
19+
1420
const checkTimeout = 500 * time.Millisecond
1521

1622
func CheckQuietly(ctx context.Context, githubToken string) (current, latest string, available bool) {
@@ -49,22 +55,22 @@ func UpdateCommandHint(info InstallInfo) string {
4955
}
5056
}
5157

52-
func NotifyUpdate(ctx context.Context, sink output.Sink, githubToken string, updatePrompt bool, persistDisable func() error) (exitAfter bool) {
53-
return notifyUpdateWithVersion(ctx, sink, githubToken, updatePrompt, persistDisable, version.Version(), fetchLatestVersion)
58+
func NotifyUpdate(ctx context.Context, sink output.Sink, opts NotifyOptions) (exitAfter bool) {
59+
return notifyUpdateWithVersion(ctx, sink, opts, version.Version(), fetchLatestVersion)
5460
}
5561

56-
func notifyUpdateWithVersion(ctx context.Context, sink output.Sink, githubToken string, updatePrompt bool, persistDisable func() error, currentVersion string, fetch versionFetcher) (exitAfter bool) {
57-
current, latest, available := checkQuietlyWithVersion(ctx, githubToken, currentVersion, fetch)
62+
func notifyUpdateWithVersion(ctx context.Context, sink output.Sink, opts NotifyOptions, currentVersion string, fetch versionFetcher) (exitAfter bool) {
63+
current, latest, available := checkQuietlyWithVersion(ctx, opts.GitHubToken, currentVersion, fetch)
5864
if !available {
5965
return false
6066
}
6167

62-
if !updatePrompt {
68+
if !opts.UpdatePrompt {
6369
output.EmitNote(sink, fmt.Sprintf("Update available: %s → %s (run %s)", current, latest, UpdateCommandHint(DetectInstallMethod())))
6470
return false
6571
}
6672

67-
return promptAndUpdate(ctx, sink, githubToken, current, latest, persistDisable)
73+
return promptAndUpdate(ctx, sink, opts.GitHubToken, current, latest, opts.PersistDisable)
6874
}
6975

7076
func promptAndUpdate(ctx context.Context, sink output.Sink, githubToken string, current, latest string, persistDisable func() error) (exitAfter bool) {

‎internal/update/notify_test.go‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func TestNotifyUpdateNoUpdateAvailable(t *testing.T) {
101101
var events []any
102102
sink := output.SinkFunc(func(event any) { events = append(events, event) })
103103

104-
exit := notifyUpdateWithVersion(context.Background(), sink, "", true, nil, "v1.0.0", testFetcher(server.URL))
104+
exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{UpdatePrompt: true}, "v1.0.0", testFetcher(server.URL))
105105
assert.False(t, exit)
106106
assert.Empty(t, events)
107107
}
@@ -113,7 +113,7 @@ func TestNotifyUpdatePromptDisabled(t *testing.T) {
113113
var events []any
114114
sink := output.SinkFunc(func(event any) { events = append(events, event) })
115115

116-
exit := notifyUpdateWithVersion(context.Background(), sink, "", false, nil, "1.0.0", testFetcher(server.URL))
116+
exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{}, "1.0.0", testFetcher(server.URL))
117117
assert.False(t, exit)
118118
assert.Len(t, events, 1)
119119
msg, ok := events[0].(output.MessageEvent)
@@ -134,7 +134,7 @@ func TestNotifyUpdatePromptSkip(t *testing.T) {
134134
}
135135
})
136136

137-
exit := notifyUpdateWithVersion(context.Background(), sink, "", true, nil, "1.0.0", testFetcher(server.URL))
137+
exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{UpdatePrompt: true}, "1.0.0", testFetcher(server.URL))
138138
assert.False(t, exit)
139139
}
140140

@@ -152,9 +152,12 @@ func TestNotifyUpdatePromptNever(t *testing.T) {
152152
}
153153
})
154154

155-
exit := notifyUpdateWithVersion(context.Background(), sink, "", true, func() error {
156-
persistCalled = true
157-
return nil
155+
exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{
156+
UpdatePrompt: true,
157+
PersistDisable: func() error {
158+
persistCalled = true
159+
return nil
160+
},
158161
}, "1.0.0", testFetcher(server.URL))
159162
assert.False(t, exit)
160163
assert.True(t, persistCalled)
@@ -172,6 +175,6 @@ func TestNotifyUpdatePromptCancelled(t *testing.T) {
172175
}
173176
})
174177

175-
exit := notifyUpdateWithVersion(context.Background(), sink, "", true, nil, "1.0.0", testFetcher(server.URL))
178+
exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{UpdatePrompt: true}, "1.0.0", testFetcher(server.URL))
176179
assert.False(t, exit)
177180
}

‎test/integration/update_test.go‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
package integration_test
22

33
import (
4+
"bytes"
5+
"context"
6+
"io"
47
"os"
58
"os/exec"
69
"path/filepath"
710
"runtime"
811
"strings"
912
"testing"
13+
"time"
1014

15+
"github.com/creack/pty"
16+
"github.com/localstack/lstk/test/integration/env"
1117
"github.com/stretchr/testify/assert"
1218
"github.com/stretchr/testify/require"
1319
)
@@ -205,6 +211,154 @@ func TestUpdateHomebrew(t *testing.T) {
205211
assert.Contains(t, updateStr, "brew upgrade", "should mention brew upgrade")
206212
}
207213

214+
func TestUpdateNotification(t *testing.T) {
215+
if runtime.GOOS == "windows" {
216+
t.Skip("PTY not supported on Windows")
217+
}
218+
219+
ctx := testContext(t)
220+
221+
// Build a fake old version to a temp location
222+
tmpBinary := filepath.Join(t.TempDir(), "lstk")
223+
repoRoot, err := filepath.Abs("../..")
224+
require.NoError(t, err)
225+
226+
buildCmd := exec.CommandContext(ctx, "go", "build",
227+
"-ldflags", "-X github.com/localstack/lstk/internal/version.version=0.0.1",
228+
"-o", tmpBinary,
229+
".",
230+
)
231+
buildCmd.Dir = repoRoot
232+
out, err := buildCmd.CombinedOutput()
233+
require.NoError(t, err, "go build failed: %s", string(out))
234+
235+
// Mock API server so license validation fails fast after the notification
236+
mockServer := createMockLicenseServer(false)
237+
defer mockServer.Close()
238+
239+
t.Run("skip", func(t *testing.T) {
240+
configFile := filepath.Join(t.TempDir(), "config.toml")
241+
require.NoError(t, os.WriteFile(configFile, []byte("update_prompt = true\n"), 0o644))
242+
243+
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
244+
defer cancel()
245+
246+
cmd := exec.CommandContext(ctx, tmpBinary, "--config", configFile)
247+
cmd.Env = env.Without(env.AuthToken).With(env.AuthToken, "fake-token").With(env.APIEndpoint, mockServer.URL)
248+
249+
ptmx, err := pty.Start(cmd)
250+
require.NoError(t, err, "failed to start command in PTY")
251+
defer func() { _ = ptmx.Close() }()
252+
253+
output := &syncBuffer{}
254+
outputCh := make(chan struct{})
255+
go func() {
256+
_, _ = io.Copy(output, ptmx)
257+
close(outputCh)
258+
}()
259+
260+
require.Eventually(t, func() bool {
261+
return bytes.Contains(output.Bytes(), []byte("new version is available"))
262+
}, 10*time.Second, 100*time.Millisecond, "update notification prompt should appear")
263+
264+
_, err = ptmx.Write([]byte("s"))
265+
require.NoError(t, err)
266+
267+
_ = cmd.Wait()
268+
<-outputCh
269+
270+
assert.Contains(t, output.String(), "Update available: 0.0.1")
271+
})
272+
273+
t.Run("never", func(t *testing.T) {
274+
configFile := filepath.Join(t.TempDir(), "config.toml")
275+
require.NoError(t, os.WriteFile(configFile, []byte("update_prompt = true\n"), 0o644))
276+
277+
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
278+
defer cancel()
279+
280+
cmd := exec.CommandContext(ctx, tmpBinary, "--config", configFile)
281+
cmd.Env = env.Without(env.AuthToken).With(env.AuthToken, "fake-token").With(env.APIEndpoint, mockServer.URL)
282+
283+
ptmx, err := pty.Start(cmd)
284+
require.NoError(t, err, "failed to start command in PTY")
285+
defer func() { _ = ptmx.Close() }()
286+
287+
output := &syncBuffer{}
288+
outputCh := make(chan struct{})
289+
go func() {
290+
_, _ = io.Copy(output, ptmx)
291+
close(outputCh)
292+
}()
293+
294+
require.Eventually(t, func() bool {
295+
return bytes.Contains(output.Bytes(), []byte("new version is available"))
296+
}, 10*time.Second, 100*time.Millisecond, "update notification prompt should appear")
297+
298+
_, err = ptmx.Write([]byte("n"))
299+
require.NoError(t, err)
300+
301+
_ = cmd.Wait()
302+
<-outputCh
303+
304+
assert.Contains(t, output.String(), "Update available: 0.0.1")
305+
306+
// Verify config was updated to disable future prompts
307+
configData, err := os.ReadFile(configFile)
308+
require.NoError(t, err)
309+
assert.Contains(t, string(configData), "update_prompt = false")
310+
})
311+
312+
t.Run("update", func(t *testing.T) {
313+
// Copy binary since it will be replaced during the update
314+
updateBinary := filepath.Join(t.TempDir(), "lstk")
315+
data, err := os.ReadFile(tmpBinary)
316+
require.NoError(t, err)
317+
require.NoError(t, os.WriteFile(updateBinary, data, 0o755))
318+
319+
configFile := filepath.Join(t.TempDir(), "config.toml")
320+
require.NoError(t, os.WriteFile(configFile, []byte("update_prompt = true\n"), 0o644))
321+
322+
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
323+
defer cancel()
324+
325+
cmd := exec.CommandContext(ctx, updateBinary, "--config", configFile)
326+
cmd.Env = env.Without(env.AuthToken).With(env.AuthToken, "fake-token").With(env.APIEndpoint, mockServer.URL)
327+
328+
ptmx, err := pty.Start(cmd)
329+
require.NoError(t, err, "failed to start command in PTY")
330+
defer func() { _ = ptmx.Close() }()
331+
332+
output := &syncBuffer{}
333+
outputCh := make(chan struct{})
334+
go func() {
335+
_, _ = io.Copy(output, ptmx)
336+
close(outputCh)
337+
}()
338+
339+
require.Eventually(t, func() bool {
340+
return bytes.Contains(output.Bytes(), []byte("new version is available"))
341+
}, 10*time.Second, 100*time.Millisecond, "update notification prompt should appear")
342+
343+
_, err = ptmx.Write([]byte("u"))
344+
require.NoError(t, err)
345+
346+
err = cmd.Wait()
347+
<-outputCh
348+
349+
out := output.String()
350+
require.NoError(t, err, "update should succeed: %s", out)
351+
assert.Contains(t, out, "Update available: 0.0.1")
352+
assert.Contains(t, out, "Updated to")
353+
354+
// Verify the binary was actually replaced
355+
verCmd := exec.CommandContext(ctx, updateBinary, "--version")
356+
verOut, err := verCmd.CombinedOutput()
357+
require.NoError(t, err)
358+
assert.NotContains(t, string(verOut), "0.0.1", "binary should no longer be the old version")
359+
})
360+
}
361+
208362
func npmPlatformPackage() string {
209363
return "lstk_" + runtime.GOOS + "_" + runtime.GOARCH
210364
}

0 commit comments

Comments
 (0)