Skip to content

Commit f1b07c7

Browse files
acmoreacmore
committed
fix(cli): streamline up progress output
1 parent 1016826 commit f1b07c7

3 files changed

Lines changed: 135 additions & 26 deletions

File tree

‎internal/cli/up.go‎

Lines changed: 81 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -236,41 +236,31 @@ func newUpCmd(opts *Options) *cobra.Command {
236236
}
237237
}
238238
if enableTmux && sshMode == "embedded" {
239-
spinner := newTransientStatus(cmd.OutOrStdout(), "Checking tmux in dev container")
240-
if !spinner.enabled {
241-
ui.stepRun("tmux", "checking dev container")
242-
}
239+
ui.stepRun("tmux", "checking dev container")
243240
status, installer, err := detectEmbeddedTmux(cmd.Context(), k, ns, pod)
244241
if err != nil {
245-
spinner.stop()
246242
ui.warnf("failed to check tmux in dev container: %v", err)
247243
} else if status == "install" {
248-
if spinner.enabled {
249-
spinner.update(fmt.Sprintf("Installing tmux via %s", installer))
250-
} else {
251-
ui.stepRun("tmux", fmt.Sprintf("installing via %s", installer))
252-
}
253-
installed, detail, err := installEmbeddedTmux(cmd.Context(), k, ns, pod, installer, func(phase string) {
254-
if spinner.enabled {
255-
spinner.update(strings.TrimSpace(phase))
256-
return
257-
}
244+
ui.stepRun("tmux", fmt.Sprintf("installing via %s", installer))
245+
_, detail, err := installEmbeddedTmux(cmd.Context(), k, ns, pod, installer, func(phase string) {
258246
if strings.TrimSpace(phase) != "" {
259247
ui.stepRun("tmux", phase)
260248
}
261249
})
262-
spinner.stop()
263250
if err != nil {
264-
ui.warnf("failed to install tmux in dev container: %v", err)
265-
} else if installed {
251+
if recoveredDetail, ok := embeddedTmuxDetailIfReady(cmd.Context(), k, ns, pod); ok {
252+
ui.stepDone("tmux", recoveredDetail)
253+
} else {
254+
ui.warnf("tmux not ready in dev container: %v", err)
255+
}
256+
} else if strings.TrimSpace(detail) != "" {
266257
ui.stepDone("tmux", detail)
267258
}
268259
} else {
269-
spinner.stop()
270260
_, detail, err := interpretTmuxStatus(status, installer, "")
271261
if err != nil {
272-
ui.warnf("failed to prepare tmux in dev container: %v", err)
273-
} else {
262+
ui.warnf("tmux not ready in dev container: %v", err)
263+
} else if strings.TrimSpace(detail) != "" {
274264
ui.stepDone("tmux", detail)
275265
}
276266
}
@@ -475,7 +465,15 @@ func installEmbeddedTmux(ctx context.Context, k devShellExecutor, namespace, pod
475465
}
476466
s, inst, rawLine := parseTmuxStatus(raw.String())
477467
if s == "" {
478-
return false, "", fmt.Errorf("unexpected tmux prepare result %q", strings.TrimSpace(raw.String()))
468+
detectStatus, detectInstaller, detectErr := detectEmbeddedTmux(ctx, k, namespace, pod)
469+
if detectErr == nil {
470+
return interpretTmuxStatus(detectStatus, detectInstaller, raw.String())
471+
}
472+
trimmed := strings.TrimSpace(raw.String())
473+
if trimmed == "" {
474+
return false, "", fmt.Errorf("unexpected tmux prepare result with no status marker; inspect %s in the dev container", embeddedTmuxLogPath)
475+
}
476+
return false, "", fmt.Errorf("unexpected tmux prepare result %q; inspect %s in the dev container", trimmed, embeddedTmuxLogPath)
479477
}
480478
inst = strings.TrimSpace(inst)
481479
return interpretTmuxStatus(s, inst, rawLine)
@@ -517,6 +515,18 @@ func parseTmuxStatus(raw string) (status, installer, line string) {
517515
return status, installer, line
518516
}
519517

518+
func embeddedTmuxDetailIfReady(ctx context.Context, k devShellExecutor, namespace, pod string) (string, bool) {
519+
status, installer, err := detectEmbeddedTmux(ctx, k, namespace, pod)
520+
if err != nil {
521+
return "", false
522+
}
523+
_, detail, err := interpretTmuxStatus(status, installer, "")
524+
if err != nil || strings.TrimSpace(detail) == "" {
525+
return "", false
526+
}
527+
return detail, true
528+
}
529+
520530
func warnIfConfigNewerThanSession(opts *Options, k *kube.Client, namespace, sessionName, podName string, errOut io.Writer) error {
521531
cfgPath, err := config.ResolvePath(opts.ConfigPath)
522532
if err != nil {
@@ -539,20 +549,45 @@ func warnIfConfigNewerThanSession(opts *Options, k *kube.Client, namespace, sess
539549
}
540550

541551
type upUI struct {
542-
out io.Writer
543-
errOut io.Writer
544-
warnings []string
552+
out io.Writer
553+
errOut io.Writer
554+
warnings []string
555+
interactive bool
556+
activeStep string
557+
active *transientStatus
545558
}
546559

547560
func newUpUI(out, errOut io.Writer) *upUI {
548-
return &upUI{out: out, errOut: errOut}
561+
return &upUI{
562+
out: out,
563+
errOut: errOut,
564+
interactive: isTerminalWriter(out),
565+
}
549566
}
550567

551568
func (u *upUI) section(name string) {
569+
u.stopActive()
552570
fmt.Fprintf(u.out, "\n== %s ==\n", name)
553571
}
554572

555573
func (u *upUI) stepRun(step, detail string) {
574+
if u.interactive {
575+
message := u.stepMessage(step, detail)
576+
if u.active != nil && u.active.enabled {
577+
if u.activeStep == step {
578+
u.active.update(message)
579+
return
580+
}
581+
u.stopActive()
582+
}
583+
u.activeStep = step
584+
u.active = newTransientStatusWithMode(u.out, message, true)
585+
if u.active.enabled {
586+
return
587+
}
588+
u.active = nil
589+
u.activeStep = ""
590+
}
556591
if detail == "" {
557592
fmt.Fprintf(u.out, "… %s\n", step)
558593
return
@@ -561,13 +596,31 @@ func (u *upUI) stepRun(step, detail string) {
561596
}
562597

563598
func (u *upUI) stepDone(step, detail string) {
599+
if u.activeStep == step {
600+
u.stopActive()
601+
}
564602
if detail == "" {
565603
fmt.Fprintf(u.out, "✔ %s\n", step)
566604
return
567605
}
568606
fmt.Fprintf(u.out, "✔ %s: %s\n", step, detail)
569607
}
570608

609+
func (u *upUI) stepMessage(step, detail string) string {
610+
if strings.TrimSpace(detail) == "" {
611+
return step
612+
}
613+
return step + ": " + detail
614+
}
615+
616+
func (u *upUI) stopActive() {
617+
if u.active != nil {
618+
u.active.stop()
619+
u.active = nil
620+
}
621+
u.activeStep = ""
622+
}
623+
571624
func (u *upUI) warnf(format string, args ...any) {
572625
msg := strings.TrimSpace(fmt.Sprintf(format, args...))
573626
if msg == "" {
@@ -577,6 +630,7 @@ func (u *upUI) warnf(format string, args ...any) {
577630
}
578631

579632
func (u *upUI) printWarnings() {
633+
u.stopActive()
580634
if len(u.warnings) == 0 {
581635
return
582636
}
@@ -591,6 +645,7 @@ func (u *upUI) warnWriter() io.Writer {
591645
}
592646

593647
func (u *upUI) printReadyCard(sessionName, namespace, pod, sshSummary, syncSummary string, ports []config.PortMapping, syncPairs []syncengine.Pair, syncModeSymbol string) {
648+
u.stopActive()
594649
fmt.Fprintln(u.out, "\n== Ready ==")
595650
fmt.Fprintf(u.out, "session: %s\n", sessionName)
596651
fmt.Fprintf(u.out, "namespace: %s\n", namespace)

‎internal/cli/up_tmux_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,11 @@ func TestInstallEmbeddedTmux(t *testing.T) {
9595
wantErr string
9696
}{
9797
{name: "installed", fake: newFakeDevShellExecutor([]string{"__OKDEV_TMUX_STATUS__=installed:apt-get"}, nil), installer: "apt-get", wantInstalled: true},
98+
{name: "installed via fallback detect", fake: newFakeDevShellExecutor([]string{"", "present:none"}, nil), installer: "apt-get"},
9899
{name: "no-root", fake: newFakeDevShellExecutor([]string{"__OKDEV_TMUX_STATUS__=no-root:none"}, nil), installer: "apt-get", wantErr: "not running as root"},
99100
{name: "unavailable", fake: newFakeDevShellExecutor([]string{"__OKDEV_TMUX_STATUS__=unavailable:apk"}, nil), installer: "apk", wantErr: embeddedTmuxLogPath},
100101
{name: "exec error", fake: newFakeDevShellExecutor(nil, []error{errors.New("boom")}), installer: "apt-get", wantErr: "boom"},
102+
{name: "empty output fallback error", fake: newFakeDevShellExecutor([]string{"", ""}, []error{nil, errors.New("detect boom")}), installer: "apt-get", wantErr: embeddedTmuxLogPath},
101103
}
102104

103105
for _, tt := range tests {
@@ -131,6 +133,7 @@ func TestInstallEmbeddedTmuxDetails(t *testing.T) {
131133
}{
132134
{name: "installed with installer", fake: newFakeDevShellExecutor([]string{"__OKDEV_TMUX_STATUS__=installed:apt-get"}, nil), installer: "apt-get", wantDetail: "installed in dev container via apt-get"},
133135
{name: "installed no installer", fake: newFakeDevShellExecutor([]string{"__OKDEV_TMUX_STATUS__=installed:none"}, nil), installer: "none", wantDetail: "installed in dev container"},
136+
{name: "present via fallback detect", fake: newFakeDevShellExecutor([]string{"", "present:none"}, nil), installer: "apt-get", wantDetail: "ready in dev container"},
134137
}
135138

136139
for _, tt := range tests {
@@ -186,6 +189,26 @@ func TestInterpretTmuxStatus(t *testing.T) {
186189
}
187190
}
188191

192+
func TestEmbeddedTmuxDetailIfReady(t *testing.T) {
193+
t.Run("ready", func(t *testing.T) {
194+
fake := newFakeDevShellExecutor([]string{"present:none"}, nil)
195+
detail, ok := embeddedTmuxDetailIfReady(context.Background(), fake, "default", "okdev-test")
196+
if !ok {
197+
t.Fatal("expected ready detail")
198+
}
199+
if detail != "ready in dev container" {
200+
t.Fatalf("unexpected detail %q", detail)
201+
}
202+
})
203+
204+
t.Run("not ready", func(t *testing.T) {
205+
fake := newFakeDevShellExecutor([]string{"unavailable:none"}, nil)
206+
if detail, ok := embeddedTmuxDetailIfReady(context.Background(), fake, "default", "okdev-test"); ok || detail != "" {
207+
t.Fatalf("expected no ready detail, got %q ok=%v", detail, ok)
208+
}
209+
})
210+
}
211+
189212
func TestEmbeddedTmuxScriptsIncludeKnownPackageManagers(t *testing.T) {
190213
for _, want := range []string{
191214
"echo install:apk",

‎internal/cli/up_ui_test.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package cli
2+
3+
import (
4+
"bytes"
5+
"strings"
6+
"testing"
7+
)
8+
9+
func TestUpUIInteractiveStepRunUpdatesInPlace(t *testing.T) {
10+
var out bytes.Buffer
11+
ui := &upUI{
12+
out: &out,
13+
errOut: &out,
14+
interactive: true,
15+
}
16+
17+
ui.stepRun("pod readiness", "Pending 0/2 (ContainerCreating)")
18+
ui.stepRun("pod readiness", "Running 1/2 (-)")
19+
ui.stepDone("pod readiness", "ready")
20+
21+
got := out.String()
22+
if !strings.Contains(got, "pod readiness: Running 1/2 (-)") {
23+
t.Fatalf("expected updated transient step message, got %q", got)
24+
}
25+
if !strings.Contains(got, "✔ pod readiness: ready\n") {
26+
t.Fatalf("expected final done line, got %q", got)
27+
}
28+
if !strings.Contains(got, "\r\033[K") {
29+
t.Fatalf("expected transient clear sequence, got %q", got)
30+
}
31+
}

0 commit comments

Comments
 (0)