From e24308709d15f94d0a4291267dda07cbd0cb1c73 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Sat, 15 Aug 2026 16:17:18 +0200 Subject: [PATCH 1/4] refactor(display): rebuild the TTY progress renderer on a model/layout/screen split The TTY renderer accumulated display-corruption fixes (truncation of details, then progress sizes, then task ids; timer alignment; rune-based measurement) that each patched one symptom of the same structural gap: nothing guaranteed a rendered line fits the terminal, and once a line wraps, cursor arithmetic desyncs and the block corrupts. Replace it with three separable units: - tty_model.go: pure event reducer with an injected clock, preserving first-parent-wins updates, monotonic progress and header counters - tty_layout.go: pure (model, size, now) -> lines function; all widths are measured in terminal cells (go-runewidth, so CJK is correct) on plain text before coloring, and every line is clipped to the terminal width by construction, status text included - tty_screen.go: diff-based repaint; unchanged rows are skipped, identical frames write nothing, a frame is a single Write; a shrinking terminal abandons the block instead of moving the cursor over reflowed rows The writer coordinates them behind a mutex and stops the refresh goroutine through context cancellation, so Done cannot block when the operation context was cancelled first (Ctrl-C during pull). The spinner frame is derived from the clock instead of advancing on every call, and truncation can no longer split multi-byte runes. Visual output is unchanged: the snapshot test reproduces the previous renderer's golden output character for character. Co-Authored-By: Claude Fable 5 Signed-off-by: Nicolas De Loof --- cmd/compose/compose_progress_test.go | 6 +- cmd/display/spinner.go | 70 --- cmd/display/tty.go | 755 ++++-------------------- cmd/display/tty_layout.go | 353 ++++++++++++ cmd/display/tty_model.go | 132 +++++ cmd/display/tty_screen.go | 88 +++ cmd/display/tty_test.go | 834 ++++++--------------------- go.mod | 2 +- 8 files changed, 850 insertions(+), 1390 deletions(-) delete mode 100644 cmd/display/spinner.go create mode 100644 cmd/display/tty_layout.go create mode 100644 cmd/display/tty_model.go create mode 100644 cmd/display/tty_screen.go diff --git a/cmd/compose/compose_progress_test.go b/cmd/compose/compose_progress_test.go index 5756782f113..6f5ede4f4b5 100644 --- a/cmd/compose/compose_progress_test.go +++ b/cmd/compose/compose_progress_test.go @@ -92,7 +92,7 @@ func TestSelectEventProcessor_AutoMode(t *testing.T) { name: "stderr TTY, stdout piped -> Full", errIsTTY: true, ansi: "auto", - wantType: "*display.ttyWriter", + wantType: "*display.termWriter", wantMode: display.ModeTTY, }, { @@ -107,7 +107,7 @@ func TestSelectEventProcessor_AutoMode(t *testing.T) { outIsTTY: true, errIsTTY: true, ansi: "auto", - wantType: "*display.ttyWriter", + wantType: "*display.termWriter", wantMode: display.ModeTTY, }, { @@ -154,7 +154,7 @@ func TestSelectEventProcessor_ExplicitMode(t *testing.T) { progress: display.ModeTTY, ansi: "auto", wantMode: display.ModeTTY, - wantType: "*display.ttyWriter", + wantType: "*display.termWriter", }, { name: "progress=tty with ansi=never is rejected", diff --git a/cmd/display/spinner.go b/cmd/display/spinner.go deleted file mode 100644 index e476deae80f..00000000000 --- a/cmd/display/spinner.go +++ /dev/null @@ -1,70 +0,0 @@ -/* - Copyright 2020 Docker Compose CLI authors - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. -*/ - -package display - -import ( - "runtime" - "time" -) - -type Spinner struct { - time time.Time - index int - chars []string - stop bool - done string -} - -func NewSpinner() *Spinner { - chars := []string{ - "⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏", - } - done := "⠿" - - if runtime.GOOS == "windows" { - chars = []string{"-"} - done = "-" - } - - return &Spinner{ - index: 0, - time: time.Now(), - chars: chars, - done: done, - } -} - -func (s *Spinner) String() string { - if s.stop { - return s.done - } - - d := time.Since(s.time) - if d.Milliseconds() > 100 { - s.index = (s.index + 1) % len(s.chars) - } - - return s.chars[s.index] -} - -func (s *Spinner) Stop() { - s.stop = true -} - -func (s *Spinner) Restart() { - s.stop = false -} diff --git a/cmd/display/tty.go b/cmd/display/tty.go index 9f379c13d42..d858db88f96 100644 --- a/cmd/display/tty.go +++ b/cmd/display/tty.go @@ -20,716 +20,173 @@ import ( "context" "fmt" "io" - "iter" - "slices" - "strings" "sync" "time" - "unicode/utf8" "github.com/buger/goterm" - "github.com/docker/go-units" - "github.com/morikuni/aec" "github.com/docker/compose/v5/pkg/api" - "github.com/docker/compose/v5/pkg/utils" ) -// Full creates an EventProcessor that render advanced UI within a terminal. -// On Start, TUI lists task with a progress timer -func Full(out io.Writer, info io.Writer, detached bool) api.EventProcessor { - return &ttyWriter{ +// Full creates the terminal EventProcessor, built on a model/layout/screen +// split: +// +// - tty_model.go — event reducer, pure data, injected clock +// - tty_layout.go — pure (model, size, now) → lines, every line ≤ width +// - tty_screen.go — diff-based repaint of the block, single Write per frame +// +// The writer itself only coordinates: a mutex guards the model and the +// screen, and the refresh goroutine is stopped through context cancellation +// so no lifecycle transition can block (Done after Ctrl-C included). +func Full(out io.Writer, info io.Writer, detached bool, opts ...TermOption) api.EventProcessor { + w := &termWriter{ out: out, info: info, - tasks: map[string]*task{}, - done: newDoneSignal(), - mtx: &sync.Mutex{}, detached: detached, + tree: newTaskTree(), + scr: screen{out: out}, + size: termSize, + now: time.Now, } + for _, opt := range opts { + opt(w) + } + return w } -// doneSignal is a channel that's safe to close more than once: a shared bus -// can go through more than one sequential Start/Done cycle in its lifetime -// (e.g. `docker compose rm --stop` runs a "stop" cycle before its own -// "remove" cycle), and a bare channel can only ever be closed once. -type doneSignal struct { - ch chan bool - once sync.Once -} +// TermOption customizes the Full EventProcessor. +type TermOption func(*termWriter) -func newDoneSignal() *doneSignal { - return &doneSignal{ch: make(chan bool)} +// WithDryRun prefixes every row with the dry-run marker. +func WithDryRun() TermOption { + return func(w *termWriter) { w.dryRun = true } } -func (d *doneSignal) close() { - d.once.Do(func() { close(d.ch) }) -} +type termWriter struct { + mu sync.Mutex + out io.Writer + info io.Writer + detached bool + dryRun bool -type ttyWriter struct { - out io.Writer - ids []string // tasks ids ordered as first event appeared - tasks map[string]*task - repeated bool - numLines int - done *doneSignal // (re)created by Start for each Start/Done cycle - mtx *sync.Mutex - dryRun bool // FIXME(ndeloof) (re)implement support for dry-run + tree taskTree + scr screen operation string - ticker *time.Ticker suspended bool - info io.Writer - detached bool -} + stopTicks context.CancelFunc -type task struct { - ID string - parent string // the resource this task receives updates from - other parents will be ignored - parents utils.Set[string] // all resources to depend on this task - startTime time.Time - endTime time.Time - text string - details string - status api.EventStatus - current int64 - percent int - total int64 - spinner *Spinner + // injected for tests + size func() (width, height int) + now func() time.Time } -func newTask(e api.Resource) task { - t := task{ - ID: e.ID, - parents: utils.NewSet[string](), - startTime: time.Now(), - text: e.Text, - details: e.Details, - status: e.Status, - current: e.Current, - percent: e.Percent, - total: e.Total, - spinner: NewSpinner(), - } - if e.ParentID != "" { - t.parent = e.ParentID - t.parents.Add(e.ParentID) +func termSize() (int, int) { + width, height := goterm.Width(), goterm.Height() + if width <= 0 { + width = 80 } - if e.Status == api.Done || e.Status == api.Error { - t.stop() + if height <= 0 { + height = 24 } - return t + return width, height } -// update adjusts task state based on last received event -func (t *task) update(e api.Resource) { - if e.ParentID != "" { - t.parents.Add(e.ParentID) - // we may receive same event from distinct parents (typically: images sharing layers) - // to avoid status to flicker, only accept updates from our first declared parent - if t.parent != e.ParentID { +func (w *termWriter) Start(ctx context.Context, operation string) { + w.mu.Lock() + defer w.mu.Unlock() + w.operation = operation + // The refresh goroutine is bound to a derived context: parent + // cancellation (Ctrl-C) and Done both stop it by cancelling, which can + // never block — there is no channel handshake to miss. + tickCtx, cancel := context.WithCancel(ctx) + w.stopTicks = cancel + go w.refresh(tickCtx) +} + +func (w *termWriter) refresh(ctx context.Context) { + ticker := time.NewTicker(tickInterval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): return + case <-ticker.C: + w.mu.Lock() + w.repaint() + w.mu.Unlock() } } - - // update task based on received event - switch e.Status { - case api.Done, api.Error, api.Warning: - if t.status != e.Status { - t.stop() - } - case api.Working: - t.spinner.Restart() - } - t.status = e.Status - t.text = e.Text - t.details = e.Details - // progress can only go up - if e.Total > t.total { - t.total = e.Total - } - if e.Current > t.current { - t.current = e.Current - } - if e.Percent > t.percent { - t.percent = e.Percent - } -} - -func (t *task) stop() { - t.endTime = time.Now() - t.spinner.Stop() -} - -func (t *task) Completed() bool { - switch t.status { - case api.Done, api.Error, api.Warning: - return true - default: - return false - } -} - -func (w *ttyWriter) Start(ctx context.Context, operation string) { - w.mtx.Lock() - // If a previous cycle is still open (Start called again before its - // matching Done, e.g. nested Start/Done misuse), close it out first so - // its render goroutine exits instead of leaking until ctx is done. - if w.done != nil { - w.done.close() - } - if w.ticker != nil { - w.ticker.Stop() - } - done := newDoneSignal() - ticker := time.NewTicker(100 * time.Millisecond) - w.done = done - w.ticker = ticker - w.operation = operation - w.mtx.Unlock() - - // done and ticker are this cycle's own, captured locally: a later Start - // (a new Start/Done cycle on the same writer) reassigns w.done/w.ticker, - // but that never affects this goroutine's own wait. - go func() { - for { - select { - case <-ctx.Done(): - // interrupted - ticker.Stop() - return - case <-done.ch: - return - case <-ticker.C: - w.print() - } - } - }() } -func (w *ttyWriter) Done(operation string, success bool) { - w.print() - w.mtx.Lock() - done := w.done - ticker := w.ticker - w.mtx.Unlock() - - // close never blocks, unlike a send, so this returns even if the render - // goroutine already exited via ctx.Done(). - done.close() - - w.mtx.Lock() - defer w.mtx.Unlock() - if ticker != nil { - ticker.Stop() +func (w *termWriter) Done(string, bool) { + w.mu.Lock() + defer w.mu.Unlock() + if w.stopTicks != nil { + w.stopTicks() + w.stopTicks = nil } + w.repaint() // leave the final state on screen w.operation = "" + // The tree and the screen survive: a follow-up operation on the same + // writer (`up` chains create and start) extends the same block. } -func (w *ttyWriter) On(events ...api.Resource) { - w.mtx.Lock() - defer w.mtx.Unlock() +func (w *termWriter) On(events ...api.Resource) { + w.mu.Lock() + defer w.mu.Unlock() for _, e := range events { - if e.ID == "Compose" { + if e.ID == api.ResourceCompose { _, _ = fmt.Fprintln(w.info, ErrorColor(e.Details)) continue } - if w.operation != "start" && (e.Text == api.StatusStarted || e.Text == api.StatusStarting) && !w.detached { - // skip those events to avoid mix with container logs + // skip those events to avoid mixing with container logs continue } - w.event(e) + w.handle(e) } } -func (w *ttyWriter) event(e api.Resource) { - // Suspend print while a build is in progress, to avoid collision with buildkit Display - if w.ticker != nil { - if e.Text == api.StatusBuilding { - w.ticker.Stop() - w.suspended = true - } else if w.suspended { - w.ticker.Reset(100 * time.Millisecond) - w.suspended = false - } +func (w *termWriter) handle(e api.Resource) { + // Buildkit paints its own UI on the same stream: stay silent while a + // build is in flight, and once it's over start a fresh block below + // whatever buildkit wrote instead of repainting over it. + if e.Text == api.StatusBuilding { + w.suspended = true + } else if w.suspended { + w.suspended = false + w.scr.reset() } - if last, ok := w.tasks[e.ID]; ok { - last.update(e) - } else { - t := newTask(e) - w.tasks[e.ID] = &t - w.ids = append(w.ids, e.ID) - } - w.printEvent(e) -} + w.tree.apply(e, w.now()) -func (w *ttyWriter) printEvent(e api.Resource) { - if w.operation != "" { - // event will be displayed by progress UI on ticker's ticks - return - } - - var color colorFunc - switch e.Status { - case api.Working: - color = SuccessColor - case api.Done: - color = SuccessColor - case api.Warning: - color = WarningColor - case api.Error: - color = ErrorColor - } - _, _ = fmt.Fprintf(w.out, "%s %s %s\n", e.ID, color(e.Text), e.Details) -} - -func (w *ttyWriter) parentTasks() iter.Seq[*task] { - return func(yield func(*task) bool) { - for _, id := range w.ids { // iterate on ids to enforce a consistent order - t := w.tasks[id] - if len(t.parents) == 0 { - yield(t) - } - } + if w.operation == "" { + // outside any operation: degrade to one plain line per event + _, _ = fmt.Fprintf(w.out, "%s %s %s\n", e.ID, plainEventColor(e.Status)(e.Text), e.Details) } } -func (w *ttyWriter) childrenTasks(parent string) iter.Seq[*task] { - return func(yield func(*task) bool) { - for _, id := range w.ids { // iterate on ids to enforce a consistent order - t := w.tasks[id] - if t.parents.Has(parent) { - yield(t) - } - } - } -} - -// lineData holds pre-computed formatting for a task line -type lineData struct { - spinner string // rendered spinner with color - prefix string // dry-run prefix if any - taskID string // possibly abbreviated - progress string // progress bar and (optionally) size info appended - progressSizeBytes int // byte length of the trailing size suffix in progress, 0 if none - status string // rendered status with color - details string // possibly abbreviated - timer string // rendered timer with color - statusPad int // padding before status to align - timerPad int // padding before timer to align - statusColor colorFunc -} - -func (w *ttyWriter) print() { - terminalWidth := goterm.Width() - terminalHeight := goterm.Height() - if terminalWidth <= 0 { - terminalWidth = 80 - } - if terminalHeight <= 0 { - terminalHeight = 24 - } - w.printWithDimensions(terminalWidth, terminalHeight) -} - -func (w *ttyWriter) printWithDimensions(terminalWidth, terminalHeight int) { - w.mtx.Lock() - defer w.mtx.Unlock() - if len(w.tasks) == 0 { +func (w *termWriter) repaint() { + if w.suspended || w.operation == "" || len(w.tree.nodes) == 0 { return } - - up := w.numLines + 1 - if !w.repeated { - up-- - w.repeated = true - } - b := aec.NewBuilder( - aec.Hide, // Hide the cursor while we are printing - aec.Up(uint(up)), - aec.Column(0), - ) - _, _ = fmt.Fprint(w.out, b.ANSI) - defer func() { - _, _ = fmt.Fprint(w.out, aec.Show) - }() - - firstLine := fmt.Sprintf("[+] %s %d/%d", w.operation, numDone(w.tasks), len(w.tasks)) - _, _ = fmt.Fprintln(w.out, firstLine) - - // Collect parent tasks in original order - allTasks := slices.Collect(w.parentTasks()) - - // Available lines: terminal height - 2 (header line + potential "more" line) - maxLines := max(terminalHeight-2, 1) - - showMore := len(allTasks) > maxLines - tasksToShow := allTasks - if showMore { - tasksToShow = allTasks[:maxLines-1] // Reserve one line for "more" message - } - - // collect line data and compute timerLen - lines := make([]lineData, len(tasksToShow)) - var timerLen int - for i, t := range tasksToShow { - lines[i] = w.prepareLineData(t) - if len(lines[i].timer) > timerLen { - timerLen = len(lines[i].timer) - } - } - - // pad timers so they all have the same visible width - for i := range lines { - l := &lines[i] - if l.timer == "" { - continue - } - timerWidth := utf8.RuneCountInString(l.timer) - if timerWidth < timerLen { - // Left-pad so the timer's right edge stays aligned on the terminal. - // This also prevents stale suffix characters from visually “sticking” - // when a previously-rendered timer was wider (e.g. "10.6s" -> "0.0s"). - l.timer = strings.Repeat(" ", timerLen-timerWidth) + l.timer - } - } - - // shorten details/taskID to fit terminal width - w.adjustLineWidth(lines, timerLen, terminalWidth) - - // compute padding - w.applyPadding(lines, terminalWidth, timerLen) - - // Render lines - numLines := 0 - for _, l := range lines { - _, _ = fmt.Fprint(w.out, lineText(l)) - numLines++ - } - - if showMore { - moreCount := len(allTasks) - len(tasksToShow) - moreText := fmt.Sprintf(" ... %d more", moreCount) - pad := max(terminalWidth-len(moreText), 0) - _, _ = fmt.Fprintf(w.out, "%s%s\n", moreText, strings.Repeat(" ", pad)) - numLines++ - } - - // Clear any remaining lines from previous render - for i := numLines; i < w.numLines; i++ { - _, _ = fmt.Fprintln(w.out, strings.Repeat(" ", terminalWidth)) - numLines++ - } - w.numLines = numLines + width, height := w.size() + lines := layoutFrame(&w.tree, w.operation, layoutOpts{ + width: width, + height: height, + dryRun: w.dryRun, + now: w.now(), + }) + w.scr.paint(lines, width) } -func (w *ttyWriter) applyPadding(lines []lineData, terminalWidth int, timerLen int) { - var maxBeforeStatus int - for i := range lines { - l := &lines[i] - // Width before statusPad: space(1) + spinner(1) + prefix + space(1) + taskID + progress - beforeStatus := 3 + lenAnsi(l.prefix) + utf8.RuneCountInString(l.taskID) + lenAnsi(l.progress) - if beforeStatus > maxBeforeStatus { - maxBeforeStatus = beforeStatus - } - } - - for i, l := range lines { - // Position before statusPad: space(1) + spinner(1) + prefix + space(1) + taskID + progress - beforeStatus := 3 + lenAnsi(l.prefix) + utf8.RuneCountInString(l.taskID) + lenAnsi(l.progress) - // statusPad aligns status; lineText adds 1 more space after statusPad - l.statusPad = maxBeforeStatus - beforeStatus - - // Format: beforeStatus + statusPad + space(1) + status - lineLen := beforeStatus + l.statusPad + 1 + utf8.RuneCountInString(l.status) - if l.details != "" { - lineLen += 1 + utf8.RuneCountInString(l.details) - } - l.timerPad = max(terminalWidth-lineLen-timerLen, 1) - lines[i] = l - - } -} - -func (w *ttyWriter) adjustLineWidth(lines []lineData, timerLen int, terminalWidth int) { - const minIDLen = 10 - maxStatusLen := maxStatusLength(lines) - - // Iteratively truncate until all lines fit - for range 100 { // safety limit - maxBeforeStatus := maxBeforeStatusWidth(lines) - overflow := computeOverflow(lines, maxBeforeStatus, maxStatusLen, timerLen, terminalWidth) - - if overflow <= 0 { - break - } - - // Drop ancillary content (details, progress size info) before touching the taskID. - if !truncateDetails(lines, overflow) && !truncateProgressSize(lines) && !truncateLongestTaskID(lines, overflow, minIDLen) { - break // Can't truncate further - } - } -} - -// maxStatusLength returns the maximum status text length across all lines. -func maxStatusLength(lines []lineData) int { - var maxLen int - for i := range lines { - if len(lines[i].status) > maxLen { - maxLen = len(lines[i].status) - } - } - return maxLen -} - -// maxBeforeStatusWidth computes the maximum width before statusPad across all lines. -// This is: space(1) + spinner(1) + prefix + space(1) + taskID + progress -func maxBeforeStatusWidth(lines []lineData) int { - var maxWidth int - for i := range lines { - l := &lines[i] - width := 3 + lenAnsi(l.prefix) + utf8.RuneCountInString(l.taskID) + lenAnsi(l.progress) - if width > maxWidth { - maxWidth = width - } - } - return maxWidth -} - -// computeOverflow calculates how many characters the widest line exceeds the terminal width. -// Returns 0 or negative if all lines fit. -func computeOverflow(lines []lineData, maxBeforeStatus, maxStatusLen, timerLen, terminalWidth int) int { - var maxOverflow int - for i := range lines { - l := &lines[i] - detailsLen := len(l.details) - if detailsLen > 0 { - detailsLen++ // space before details - } - // Line width: maxBeforeStatus + space(1) + status + details + minTimerPad(1) + timer - lineWidth := maxBeforeStatus + 1 + maxStatusLen + detailsLen + 1 + timerLen - overflow := lineWidth - terminalWidth - if overflow > maxOverflow { - maxOverflow = overflow - } - } - return maxOverflow -} - -// truncateProgressSize drops the trailing "X.XMB / Y.YMB" size info from the -// line currently driving maxBeforeStatusWidth — only that line's shrink can -// reduce overflow. Returns true if any line was modified. -func truncateProgressSize(lines []lineData) bool { - maxIdx := -1 - var maxWidth int - for i := range lines { - l := &lines[i] - if l.progressSizeBytes == 0 { - continue - } - w := lenAnsi(l.prefix) + utf8.RuneCountInString(l.taskID) + lenAnsi(l.progress) - if maxIdx < 0 || w > maxWidth { - maxWidth = w - maxIdx = i - } - } - if maxIdx < 0 { - return false - } - l := &lines[maxIdx] - l.progress = l.progress[:len(l.progress)-l.progressSizeBytes] - l.progressSizeBytes = 0 - return true -} - -// truncateDetails tries to truncate the first line's details to reduce overflow. -// Returns true if any truncation was performed. -func truncateDetails(lines []lineData, overflow int) bool { - for i := range lines { - l := &lines[i] - if len(l.details) > 3 { - reduction := min(overflow, len(l.details)-3) - l.details = l.details[:len(l.details)-reduction-3] + "..." - return true - } else if l.details != "" { - l.details = "" - return true - } - } - return false -} - -// truncateLongestTaskID truncates the longest taskID to reduce overflow. -// Returns true if truncation was performed. Lengths and slicing are in runes -// to avoid emitting invalid UTF-8 when taskID contains multi-byte chars. -func truncateLongestTaskID(lines []lineData, overflow, minIDLen int) bool { - longestIdx := -1 - longestLen := minIDLen - for i := range lines { - if utf8.RuneCountInString(lines[i].taskID) > longestLen { - longestLen = utf8.RuneCountInString(lines[i].taskID) - longestIdx = i - } - } - - if longestIdx < 0 { - return false - } - - l := &lines[longestIdx] - reduction := overflow + 3 // account for "..." - newLen := max(longestLen-reduction, minIDLen-3) - runes := []rune(l.taskID) - l.taskID = string(runes[:newLen]) + "..." - return true -} - -func (w *ttyWriter) prepareLineData(t *task) lineData { - endTime := time.Now() - if t.status != api.Working { - endTime = t.startTime - if (t.endTime != time.Time{}) { - endTime = t.endTime - } - } - - prefix := "" - if w.dryRun { - prefix = PrefixColor(DRYRUN_PREFIX) - } - - elapsed := endTime.Sub(t.startTime).Seconds() - - var ( - hideDetails bool - total int64 - current int64 - completion []string - ) - - // only show the aggregated progress while the root operation is in-progress - if t.status == api.Working { - for child := range w.childrenTasks(t.ID) { - if child.status == api.Working && child.total == 0 { - hideDetails = true - } - total += child.total - current += child.current - r := len(percentChars) - 1 - p := min(child.percent, 100) - completion = append(completion, percentChars[r*p/100]) - } - } - - if total == 0 { - hideDetails = true - } - - var progress string - var progressSizeBytes int - if len(completion) > 0 { - progress = " [" + SuccessColor(strings.Join(completion, "")) + "]" - if !hideDetails { - sizeInfo := fmt.Sprintf(" %7s / %-7s", units.HumanSize(float64(current)), units.HumanSize(float64(total))) - progress += sizeInfo - progressSizeBytes = len(sizeInfo) - } - } - - return lineData{ - spinner: spinner(t), - prefix: prefix, - taskID: t.ID, - progress: progress, - progressSizeBytes: progressSizeBytes, - status: t.text, - statusColor: colorFn(t.status), - details: t.details, - timer: fmt.Sprintf("%.1fs", elapsed), - } -} - -func lineText(l lineData) string { - var sb strings.Builder - sb.WriteString(" ") - sb.WriteString(l.spinner) - sb.WriteString(l.prefix) - sb.WriteString(" ") - sb.WriteString(l.taskID) - sb.WriteString(l.progress) - sb.WriteString(strings.Repeat(" ", l.statusPad)) - sb.WriteString(" ") - sb.WriteString(l.statusColor(l.status)) - if l.details != "" { - sb.WriteString(" ") - sb.WriteString(l.details) - } - sb.WriteString(strings.Repeat(" ", l.timerPad)) - sb.WriteString(TimerColor(l.timer)) - sb.WriteString("\n") - return sb.String() -} - -var ( - spinnerDone = "✔" - spinnerWarning = "!" - spinnerError = "✘" -) - -func spinner(t *task) string { - switch t.status { - case api.Done: - return SuccessColor(spinnerDone) - case api.Warning: - return WarningColor(spinnerWarning) - case api.Error: - return ErrorColor(spinnerError) - default: - return CountColor(t.spinner.String()) - } -} - -func colorFn(s api.EventStatus) colorFunc { +func plainEventColor(s api.EventStatus) colorFunc { switch s { - case api.Done: - return SuccessColor case api.Warning: return WarningColor case api.Error: return ErrorColor default: - return nocolor - } -} - -func numDone(tasks map[string]*task) int { - i := 0 - for _, t := range tasks { - if t.status != api.Working { - i++ - } - } - return i -} - -// lenAnsi count of user-perceived characters in ANSI string. -func lenAnsi(s string) int { - length := 0 - ansiCode := false - for _, r := range s { - if r == '\x1b' { - ansiCode = true - continue - } - if ansiCode && r == 'm' { - ansiCode = false - continue - } - if !ansiCode { - length++ - } + return SuccessColor } - return length } - -var percentChars = strings.Split("⠀⡀⣀⣄⣤⣦⣶⣷⣿", "") diff --git a/cmd/display/tty_layout.go b/cmd/display/tty_layout.go new file mode 100644 index 00000000000..08da3df36ff --- /dev/null +++ b/cmd/display/tty_layout.go @@ -0,0 +1,353 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package display + +import ( + "fmt" + "runtime" + "strings" + "time" + + "github.com/docker/go-units" + "github.com/mattn/go-runewidth" + + "github.com/docker/compose/v5/pkg/api" +) + +// The layout is a pure function of (taskTree, operation, layoutOpts): +// no clocks, no terminal, no mutation. Its single hard invariant, enforced +// by renderSegs, is that every returned line occupies at most o.width +// terminal cells. This is what makes screen cursor arithmetic sound: a line +// that never wraps is a line the cursor can reliably move back over. +// +// All width computations happen on plain text, in terminal cells (via +// go-runewidth, so CJK and other wide runes count as 2); colors are applied +// only when a segment's final text is known. ANSI sequences therefore never +// enter any measurement. + +// seg is a piece of a row: plain text plus the color to apply once its +// geometry is final. +type seg struct { + text string + color colorFunc +} + +type layoutOpts struct { + width, height int + dryRun bool + now time.Time +} + +const ( + tickInterval = 100 * time.Millisecond + minIDCells = 10 + // left margin: space + spinner + space + marginCells = 3 + // widest status text worth reserving column space for; longer ones are + // truncated rather than allowed to squeeze the id column + maxStatusReserveCells = 20 +) + +// layoutFrame renders the whole progress block, one string per terminal row. +func layoutFrame(t *taskTree, operation string, o layoutOpts) []string { + done, total := t.counts() + lines := []string{fmt.Sprintf("[+] %s %d/%d", operation, done, total)} + + rows := buildRows(t, o) + maxRows := max(o.height-2, 1) + var more int + if len(rows) > maxRows { + more = len(rows) - (maxRows - 1) + rows = rows[:maxRows-1] + } + + cols := computeColumns(rows, o.width) + for i := range rows { + lines = append(lines, renderSegs(rowSegs(&rows[i], cols, o.width), o.width)) + } + if more > 0 { + lines = append(lines, renderSegs([]seg{{text: fmt.Sprintf(" ... %d more", more)}}, o.width)) + } + return lines +} + +// row is the pre-rendered content of one line, before column fitting. +type row struct { + spin seg + prefix string // dry-run prefix + id string + barStrip string // aggregated braille strip, one glyph per child task + sizes string // droppable progress suffix appended after the bar + status string + statusColor colorFunc + details string + timer string +} + +func buildRows(t *taskTree, o layoutOpts) []row { + var rows []row + for _, root := range t.roots() { + rows = append(rows, makeRow(t, root, o)) + } + return rows +} + +func makeRow(t *taskTree, n *node, o layoutOpts) row { + r := row{ + spin: spinGlyph(n, o.now), + id: n.id, + status: n.text, + statusColor: colorFn(n.status), + details: n.details, + timer: fmt.Sprintf("%.1fs", nodeElapsed(n, o.now).Seconds()), + } + if o.dryRun { + r.prefix = DRYRUN_PREFIX + } + r.barStrip, r.sizes = aggregateProgress(t, n) + return r +} + +func nodeElapsed(n *node, now time.Time) time.Duration { + switch { + case n.status == api.Working: + return now.Sub(n.startedAt) + case !n.endedAt.IsZero(): + return n.endedAt.Sub(n.startedAt) + default: + return 0 + } +} + +// aggregateProgress compresses the children of a root task into a braille +// strip (one glyph per child) and a "current / total" size suffix. +func aggregateProgress(t *taskTree, root *node) (strip, sizes string) { + if root.status != api.Working { + return "", "" + } + var ( + total, current int64 + hideSizes bool + glyphs []string + ) + for _, child := range t.children(root.id) { + if child.status == api.Working && child.total == 0 { + hideSizes = true + } + total += child.total + current += child.current + r := len(percentChars) - 1 + p := min(child.percent, 100) + glyphs = append(glyphs, percentChars[r*p/100]) + } + if len(glyphs) == 0 { + return "", "" + } + if total == 0 { + hideSizes = true + } + if !hideSizes { + sizes = fmt.Sprintf(" %7s / %-7s", units.HumanSize(float64(current)), units.HumanSize(float64(total))) + } + return strings.Join(glyphs, ""), sizes +} + +// columns holds the shared geometry aligning all rows of a frame. +type columns struct { + left int // indent + prefix + id + bar + sizes + timer int +} + +func computeColumns(rows []row, width int) columns { + var c columns + var status int + for i := range rows { + r := &rows[i] + c.timer = max(c.timer, runewidth.StringWidth(r.timer)) + c.left = max(c.left, leftWidth(r, true)) + status = max(status, runewidth.StringWidth(r.status)) + } + // keep room for the widest (capped) status and the filler before the timer + reserve := min(status, maxStatusReserveCells) + 2 + c.left = min(c.left, max(width-marginCells-reserve-c.timer, minIDCells)) + return c +} + +// leftWidth measures the left block of a row in cells. +func leftWidth(r *row, withSizes bool) int { + w := runewidth.StringWidth(r.prefix) + runewidth.StringWidth(r.id) + if r.barStrip != "" { + w += 3 + runewidth.StringWidth(r.barStrip) // " [" + strip + "]" + } + if withSizes { + w += runewidth.StringWidth(r.sizes) + } + return w +} + +// rowSegs lays one row out into segments. Content degrades in a fixed order +// when space is short: drop the sizes suffix, truncate the id, truncate +// status, truncate then drop details. renderSegs is the final hard guarantee. +func rowSegs(r *row, c columns, width int) []seg { + var segs []seg + used := 0 + add := func(text string, color colorFunc) { + if text == "" { + return + } + segs = append(segs, seg{text: text, color: color}) + used += runewidth.StringWidth(text) + } + + add(" ", nil) + add(r.spin.text, r.spin.color) + add(r.prefix, PrefixColor) + add(" ", nil) + + // left block: id + bar + sizes, fitted to the shared left column + sizes := r.sizes + if leftWidth(r, true) > c.left { + sizes = "" // drop the size suffix first + } + if lw := leftWidth(r, false); lw > c.left { + idBudget := max(c.left-(lw-runewidth.StringWidth(r.id)), minIDCells-3) + r.id = truncateCells(r.id, idBudget) + } + add(r.id, nil) + if r.barStrip != "" { + add(" [", nil) + add(r.barStrip, SuccessColor) + add("]", nil) + } + add(sizes, nil) + if pad := marginCells + c.left - used; pad > 0 { + add(strings.Repeat(" ", pad), nil) + } + + // status, then details, each within what remains before the timer + if budget := width - used - c.timer - 2; budget > 0 { + add(" ", nil) + add(truncateCells(r.status, budget), r.statusColor) + } + if budget := width - used - c.timer - 2; r.details != "" && budget >= 5 { + add(" ", nil) + add(truncateCells(r.details, budget-1), nil) + } + + // right-aligned timer + if fill := width - used - c.timer; fill > 0 { + add(strings.Repeat(" ", fill), nil) + } + add(strings.Repeat(" ", max(c.timer-runewidth.StringWidth(r.timer), 0)), nil) + add(r.timer, TimerColor) + return segs +} + +// renderSegs assembles segments into the final styled line, enforcing the +// package invariant: the rendered line occupies at most maxCells terminal +// cells. Truncation happens on plain text before coloring, so the clip is +// both ANSI-safe and UTF-8-safe. +func renderSegs(segs []seg, maxCells int) string { + var sb strings.Builder + cells := 0 + for _, s := range segs { + w := runewidth.StringWidth(s.text) + if cells+w > maxCells { + s.text = runewidth.Truncate(s.text, maxCells-cells, "") + w = runewidth.StringWidth(s.text) + } + if s.text != "" { + if s.color != nil { + sb.WriteString(s.color(s.text)) + } else { + sb.WriteString(s.text) + } + cells += w + } + if cells >= maxCells { + break + } + } + return sb.String() +} + +// truncateCells shortens s to the given number of terminal cells, appending +// "..." when something was actually cut and space allows. +func truncateCells(s string, cells int) string { + if cells <= 0 { + return "" + } + if runewidth.StringWidth(s) <= cells { + return s + } + if cells <= 3 { + return runewidth.Truncate(s, cells, "") + } + return runewidth.Truncate(s, cells, "...") +} + +var ( + spinnerDone = "✔" + spinnerWarning = "!" + spinnerError = "✘" + + // percentChars maps a completion ratio to a braille glyph for the + // aggregated per-image strip + percentChars = strings.Split("⠀⡀⣀⣄⣤⣦⣶⣷⣿", "") + + termSpinnerFrames = spinnerFrames() +) + +func colorFn(s api.EventStatus) colorFunc { + switch s { + case api.Done: + return SuccessColor + case api.Warning: + return WarningColor + case api.Error: + return ErrorColor + default: + return nocolor + } +} + +func spinnerFrames() []string { + if runtime.GOOS == "windows" { + return []string{"-"} + } + return []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} +} + +// spinGlyph derives the per-row glyph from status and time. The animation +// frame is a pure function of the clock, so its speed doesn't depend on how +// often the layout runs. +func spinGlyph(n *node, now time.Time) seg { + switch n.status { + case api.Done: + return seg{text: spinnerDone, color: SuccessColor} + case api.Warning: + return seg{text: spinnerWarning, color: WarningColor} + case api.Error: + return seg{text: spinnerError, color: ErrorColor} + default: + frame := int(now.Sub(n.startedAt)/tickInterval) % len(termSpinnerFrames) + if frame < 0 { + frame = 0 + } + return seg{text: termSpinnerFrames[frame], color: CountColor} + } +} diff --git a/cmd/display/tty_model.go b/cmd/display/tty_model.go new file mode 100644 index 00000000000..0c1a981b457 --- /dev/null +++ b/cmd/display/tty_model.go @@ -0,0 +1,132 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package display + +import ( + "time" + + "github.com/docker/compose/v5/pkg/api" + "github.com/docker/compose/v5/pkg/utils" +) + +// node is the rendered state of a single task, fed exclusively by +// taskTree.apply. It holds no rendering concern: no spinner, no strings +// destined for the screen, no clock reads. +type node struct { + id string + anchor string // first parent seen; only events carrying it update the node + parents utils.Set[string] + text string + details string + status api.EventStatus + current int64 + total int64 + percent int + startedAt time.Time + endedAt time.Time +} + +func (n *node) completed() bool { + switch n.status { + case api.Done, api.Error, api.Warning: + return true + default: + return false + } +} + +// taskTree stores tasks in arrival order and resolves parent/child links. +// It is a plain data structure: all mutation goes through apply, all time is +// injected, so its behavior is exhaustively testable without a terminal. +type taskTree struct { + order []string + nodes map[string]*node +} + +func newTaskTree() taskTree { + return taskTree{nodes: map[string]*node{}} +} + +// apply is the single state transition of the model. +func (t *taskTree) apply(e api.Resource, now time.Time) { + n, ok := t.nodes[e.ID] + if !ok { + n = &node{ + id: e.ID, + anchor: e.ParentID, + parents: utils.NewSet[string](), + startedAt: now, + } + t.nodes[e.ID] = n + t.order = append(t.order, e.ID) + } + if e.ParentID != "" { + n.parents.Add(e.ParentID) + // Layers shared by several images receive the same events once per + // image. Accept updates from the first declared parent only, so the + // rendered state doesn't flicker between concurrent pull streams. + if n.anchor != e.ParentID { + return + } + } + + wasCompleted := n.completed() + n.status = e.Status + n.text = e.Text + n.details = e.Details + // progress is monotonic: out-of-order events must not move bars backwards + n.total = max(n.total, e.Total) + n.current = max(n.current, e.Current) + n.percent = max(n.percent, e.Percent) + if n.completed() && !wasCompleted { + n.endedAt = now + } +} + +// roots returns the top-level tasks in arrival order. +func (t *taskTree) roots() []*node { + var roots []*node + for _, id := range t.order { + if n := t.nodes[id]; len(n.parents) == 0 { + roots = append(roots, n) + } + } + return roots +} + +// children returns the tasks attached to parent, in arrival order. A layer +// shared by several images is listed under each of them. +func (t *taskTree) children(parent string) []*node { + var children []*node + for _, id := range t.order { + if n := t.nodes[id]; n.parents.Has(parent) { + children = append(children, n) + } + } + return children +} + +// counts returns completed and total task counts, children included, to +// preserve the historical "[+] pull 3/15" header semantics. +func (t *taskTree) counts() (done, total int) { + for _, n := range t.nodes { + if n.completed() { + done++ + } + } + return done, len(t.nodes) +} diff --git a/cmd/display/tty_screen.go b/cmd/display/tty_screen.go new file mode 100644 index 00000000000..bd52b718c2a --- /dev/null +++ b/cmd/display/tty_screen.go @@ -0,0 +1,88 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package display + +import ( + "io" + "slices" + "strings" + + "github.com/morikuni/aec" +) + +// screen owns the terminal region the progress block is painted on. +// +// It relies on a single contract with the layout: every line fits the +// terminal width, so one string is one terminal row and the cursor can move +// back over the block with plain relative moves. Repainting diffs against +// the previous frame: unchanged rows are skipped (a bare newline), changed +// rows are erased and rewritten, leftover rows from a taller previous frame +// are blanked. Each frame is flushed as a single Write. +type screen struct { + out io.Writer + prev []string + prevWidth int +} + +// reset forgets the painted block: the next paint starts fresh at the +// current cursor position. Used when foreign output (buildkit, a resize +// reflow) may have invalidated our notion of where the block lives. +func (s *screen) reset() { + s.prev = nil + s.prevWidth = 0 +} + +func (s *screen) paint(rows []string, width int) { + if width < s.prevWidth { + // The terminal shrank: previously painted rows may have wrapped and + // the terminal may have reflowed them, so cursor arithmetic against + // the old block is meaningless. Leave it behind and start a new one. + s.reset() + } + redrawAll := width != s.prevWidth + if !redrawAll && slices.Equal(rows, s.prev) { + return // nothing changed, write nothing + } + + var b strings.Builder + b.WriteString(aec.Hide.String()) + if n := len(s.prev); n > 0 { + b.WriteString(aec.Up(uint(n)).String()) + } + b.WriteString(aec.Column(0).String()) + for i, row := range rows { + if !redrawAll && i < len(s.prev) && s.prev[i] == row { + b.WriteString("\n") + continue + } + b.WriteString(aec.EraseLine(aec.EraseModes.All).String()) + b.WriteString(row) + b.WriteString("\n") + } + if extra := len(s.prev) - len(rows); extra > 0 { + for range extra { + b.WriteString(aec.EraseLine(aec.EraseModes.All).String()) + b.WriteString("\n") + } + b.WriteString(aec.Up(uint(extra)).String()) + } + b.WriteString(aec.Show.String()) + _, _ = io.WriteString(s.out, b.String()) + + s.prev = rows + s.prevWidth = width +} diff --git a/cmd/display/tty_test.go b/cmd/display/tty_test.go index 3e6b870d6b0..49b3c081b0e 100644 --- a/cmd/display/tty_test.go +++ b/cmd/display/tty_test.go @@ -20,356 +20,17 @@ import ( "bytes" "context" "fmt" - "io" "strings" - "sync" "testing" "time" "unicode/utf8" - "go.uber.org/goleak" + "github.com/mattn/go-runewidth" "gotest.tools/v3/assert" "github.com/docker/compose/v5/pkg/api" ) -func newTestWriter() (*ttyWriter, *bytes.Buffer) { - var buf bytes.Buffer - w := &ttyWriter{ - out: &buf, - info: &buf, - tasks: map[string]*task{}, - done: newDoneSignal(), - mtx: &sync.Mutex{}, - operation: "pull", - } - return w, &buf -} - -func addTask(w *ttyWriter, id, text, details string, status api.EventStatus) { - t := &task{ - ID: id, - parents: make(map[string]struct{}), - startTime: time.Now(), - text: text, - details: details, - status: status, - spinner: NewSpinner(), - } - w.tasks[id] = t - w.ids = append(w.ids, id) -} - -// extractLines parses the output buffer and returns lines without ANSI control sequences -func extractLines(buf *bytes.Buffer) []string { - content := buf.String() - // Split by newline - rawLines := strings.Split(content, "\n") - var lines []string - for _, line := range rawLines { - // Skip empty lines and lines that are just ANSI codes - if lenAnsi(line) > 0 { - lines = append(lines, line) - } - } - return lines -} - -func TestPrintWithDimensions_LinesFitTerminalWidth(t *testing.T) { - testCases := []struct { - name string - taskID string - status string - details string - terminalWidth int - }{ - { - name: "short task fits wide terminal", - taskID: "Image foo", - status: "Pulling", - details: "layer abc123", - terminalWidth: 100, - }, - { - name: "long details truncated to fit", - taskID: "Image foo", - status: "Pulling", - details: "downloading layer sha256:abc123def456789xyz0123456789abcdef", - terminalWidth: 50, - }, - { - name: "long taskID truncated to fit", - taskID: "very-long-image-name-that-exceeds-terminal-width", - status: "Pulling", - details: "", - terminalWidth: 40, - }, - { - name: "both long taskID and details", - taskID: "my-very-long-service-name-here", - status: "Downloading", - details: "layer sha256:abc123def456789xyz0123456789", - terminalWidth: 50, - }, - { - name: "narrow terminal", - taskID: "service-name", - status: "Pulling", - details: "some details", - terminalWidth: 35, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - w, buf := newTestWriter() - addTask(w, tc.taskID, tc.status, tc.details, api.Working) - - w.printWithDimensions(tc.terminalWidth, 24) - - lines := extractLines(buf) - for i, line := range lines { - lineLen := lenAnsi(line) - assert.Assert(t, lineLen <= tc.terminalWidth, - "line %d has length %d which exceeds terminal width %d: %q", - i, lineLen, tc.terminalWidth, line) - } - }) - } -} - -func TestPrintWithDimensions_MultipleTasksFitTerminalWidth(t *testing.T) { - w, buf := newTestWriter() - - // Add multiple tasks with varying lengths - addTask(w, "Image nginx", "Pulling", "layer sha256:abc123", api.Working) - addTask(w, "Image postgres-database", "Pulling", "downloading", api.Working) - addTask(w, "Image redis", "Pulled", "", api.Done) - - terminalWidth := 60 - w.printWithDimensions(terminalWidth, 24) - - lines := extractLines(buf) - for i, line := range lines { - lineLen := lenAnsi(line) - assert.Assert(t, lineLen <= terminalWidth, - "line %d has length %d which exceeds terminal width %d: %q", - i, lineLen, terminalWidth, line) - } -} - -func TestPrintWithDimensions_VeryNarrowTerminal(t *testing.T) { - w, buf := newTestWriter() - addTask(w, "Image nginx", "Pulling", "details", api.Working) - - terminalWidth := 30 - w.printWithDimensions(terminalWidth, 24) - - lines := extractLines(buf) - for i, line := range lines { - lineLen := lenAnsi(line) - assert.Assert(t, lineLen <= terminalWidth, - "line %d has length %d which exceeds terminal width %d: %q", - i, lineLen, terminalWidth, line) - } -} - -func TestPrintWithDimensions_TaskWithProgress(t *testing.T) { - w, buf := newTestWriter() - - // Create parent task - parent := &task{ - ID: "Image nginx", - parents: make(map[string]struct{}), - startTime: time.Now(), - text: "Pulling", - status: api.Working, - spinner: NewSpinner(), - } - w.tasks["Image nginx"] = parent - w.ids = append(w.ids, "Image nginx") - - // Create child tasks to trigger progress display - for i := range 3 { - child := &task{ - ID: "layer" + string(rune('a'+i)), - parents: map[string]struct{}{"Image nginx": {}}, - startTime: time.Now(), - text: "Downloading", - status: api.Working, - total: 1000, - current: 500, - percent: 50, - spinner: NewSpinner(), - } - w.tasks[child.ID] = child - w.ids = append(w.ids, child.ID) - } - - terminalWidth := 80 - w.printWithDimensions(terminalWidth, 24) - - lines := extractLines(buf) - for i, line := range lines { - lineLen := lenAnsi(line) - assert.Assert(t, lineLen <= terminalWidth, - "line %d has length %d which exceeds terminal width %d: %q", - i, lineLen, terminalWidth, line) - } -} - -func TestAdjustLineWidth_DetailsCorrectlyTruncated(t *testing.T) { - w := &ttyWriter{} - lines := []lineData{ - { - taskID: "Image foo", - status: "Pulling", - details: "downloading layer sha256:abc123def456789xyz", - }, - } - - terminalWidth := 50 - timerLen := 5 - w.adjustLineWidth(lines, timerLen, terminalWidth) - - // Verify the line fits - detailsLen := len(lines[0].details) - if detailsLen > 0 { - detailsLen++ // space before details - } - // widthWithoutDetails = 5 + prefix(0) + taskID(9) + progress(0) + status(7) + timer(5) = 26 - lineWidth := 5 + len(lines[0].taskID) + len(lines[0].status) + detailsLen + timerLen - - assert.Assert(t, lineWidth <= terminalWidth, - "line width %d should not exceed terminal width %d (taskID=%q, details=%q)", - lineWidth, terminalWidth, lines[0].taskID, lines[0].details) - - // Verify details were truncated (not removed entirely) - assert.Assert(t, lines[0].details != "", "details should be truncated, not removed") - assert.Assert(t, strings.HasSuffix(lines[0].details, "..."), "truncated details should end with ...") -} - -func TestAdjustLineWidth_TaskIDCorrectlyTruncated(t *testing.T) { - w := &ttyWriter{} - lines := []lineData{ - { - taskID: "very-long-image-name-that-exceeds-minimum-length", - status: "Pulling", - details: "", - }, - } - - terminalWidth := 40 - timerLen := 5 - w.adjustLineWidth(lines, timerLen, terminalWidth) - - lineWidth := 5 + len(lines[0].taskID) + 7 + timerLen - - assert.Assert(t, lineWidth <= terminalWidth, - "line width %d should not exceed terminal width %d (taskID=%q)", - lineWidth, terminalWidth, lines[0].taskID) - - assert.Assert(t, strings.HasSuffix(lines[0].taskID, "..."), "truncated taskID should end with ...") -} - -// TestAdjustLineWidth_MultiByteTaskIDFits guards against drift between -// applyPadding (rune-based) and maxBeforeStatusWidth (formerly byte-based): -// a byte-based measurement falsely flags overflow for multi-byte taskIDs. -func TestAdjustLineWidth_MultiByteTaskIDFits(t *testing.T) { - w := &ttyWriter{} - taskID := "Image 测试测试" // 10 runes, 18 bytes - lines := []lineData{{ - taskID: taskID, - status: "Pulling", - }} - - // terminalWidth=30 fits in runes (3+10+1+7+1+4 = 26) but not in bytes - // (3+18+1+7+1+4 = 34), so a byte-based measurement would truncate. - w.adjustLineWidth(lines, 4, 30) - - assert.Equal(t, taskID, lines[0].taskID, - "taskID should not be modified when it fits terminal width in runes") -} - -// TestTruncateLongestTaskID_PreservesValidUTF8 verifies that when truncation -// of a multi-byte UTF-8 taskID is genuinely required, the resulting string -// remains valid UTF-8. Byte-indexed slicing can land mid-rune and emit -// replacement characters (�) into the rendered output. -func TestTruncateLongestTaskID_PreservesValidUTF8(t *testing.T) { - taskID := "Image 测试测试测试测试" // 14 runes, 30 bytes - lines := []lineData{{taskID: taskID}} - - truncateLongestTaskID(lines, 8, 10) - - assert.Assert(t, utf8.ValidString(lines[0].taskID), - "truncated taskID must remain valid UTF-8, got %q", lines[0].taskID) - assert.Assert(t, strings.HasSuffix(lines[0].taskID, "..."), - "truncated taskID should end with ..., got %q", lines[0].taskID) -} - -// TestTruncateProgressSize_PicksWidestLine verifies that dropping the size -// suffix targets the line currently driving maxBeforeStatusWidth (the only -// line whose shrink can reduce overflow), preserving size info on narrower -// lines that are not the bottleneck. -func TestTruncateProgressSize_PicksWidestLine(t *testing.T) { - narrowSuffix := " 5MB / 10MB" - wideSuffix := " 50MB / 100MB" - lines := []lineData{ - { - taskID: "Image short", - progress: " [⣿⣿]" + narrowSuffix, - progressSizeBytes: len(narrowSuffix), - }, - { - taskID: "Image very-long-named-task", - progress: " [⣿⣿⣿⣿⣿⣿⣿⣿]" + wideSuffix, - progressSizeBytes: len(wideSuffix), - }, - } - - truncateProgressSize(lines) - - assert.Equal(t, 0, lines[1].progressSizeBytes, - "widest line should lose its size suffix first") - assert.Equal(t, len(narrowSuffix), lines[0].progressSizeBytes, - "narrower line should retain its size suffix") -} - -func TestAdjustLineWidth_NoTruncationNeeded(t *testing.T) { - w := &ttyWriter{} - originalDetails := "short" - originalTaskID := "Image foo" - lines := []lineData{ - { - taskID: originalTaskID, - status: "Pulling", - details: originalDetails, - }, - } - - // Wide terminal, nothing should be truncated - w.adjustLineWidth(lines, 5, 100) - - assert.Equal(t, originalTaskID, lines[0].taskID, "taskID should not be modified") - assert.Equal(t, originalDetails, lines[0].details, "details should not be modified") -} - -func TestAdjustLineWidth_DetailsRemovedWhenTooShort(t *testing.T) { - w := &ttyWriter{} - lines := []lineData{ - { - taskID: "Image foo", - status: "Pulling", - details: "abc", // Very short, can't be meaningfully truncated - }, - } - - // Terminal so narrow that even minimal details + "..." wouldn't help - w.adjustLineWidth(lines, 5, 28) - - assert.Equal(t, "", lines[0].details, "details should be removed entirely when too short to truncate") -} - // stripAnsi removes ANSI escape codes from a string func stripAnsi(s string) string { var result strings.Builder @@ -391,376 +52,215 @@ func stripAnsi(s string) string { return result.String() } -func TestPrintWithDimensions_PulledAndPullingWithLongIDs(t *testing.T) { - w, buf := newTestWriter() - - // Add a completed task with long ID - completedTask := &task{ - ID: "Image docker.io/library/nginx-long-name", - parents: make(map[string]struct{}), - startTime: time.Now().Add(-2 * time.Second), - endTime: time.Now(), - text: "Pulled", - status: api.Done, - spinner: NewSpinner(), - } - completedTask.spinner.Stop() - w.tasks[completedTask.ID] = completedTask - w.ids = append(w.ids, completedTask.ID) - - // Add a pending task with long ID - pendingTask := &task{ - ID: "Image docker.io/library/postgres-database", - parents: make(map[string]struct{}), - startTime: time.Now(), - text: "Pulling", - status: api.Working, - spinner: NewSpinner(), - } - w.tasks[pendingTask.ID] = pendingTask - w.ids = append(w.ids, pendingTask.ID) - - terminalWidth := 50 - w.printWithDimensions(terminalWidth, 24) - - // Strip all ANSI codes from output and split by newline - stripped := stripAnsi(buf.String()) - lines := strings.Split(stripped, "\n") - - // Filter non-empty lines - var nonEmptyLines []string - for _, line := range lines { - if strings.TrimSpace(line) != "" { - nonEmptyLines = append(nonEmptyLines, line) - } - } - - // Expected output format (50 runes per task line) - expected := `[+] pull 1/2 - ✔ Image docker.io/library/nginx-l... Pulled 2.0s - ⠋ Image docker.io/library/postgre... Pulling 0.0s` - - expectedLines := strings.Split(expected, "\n") - - // Debug output - t.Logf("Actual output:\n") - for i, line := range nonEmptyLines { - t.Logf(" line %d (%2d runes): %q", i, utf8.RuneCountInString(line), line) - } - - // Verify number of lines - assert.Equal(t, len(expectedLines), len(nonEmptyLines), "number of lines should match") - - // Verify each line matches expected - for i, line := range nonEmptyLines { - if i < len(expectedLines) { - assert.Equal(t, expectedLines[i], line, - "line %d should match expected", i) - } - } - - // Verify task lines fit within terminal width (strict - no tolerance) - for i, line := range nonEmptyLines { - if i > 0 { // Skip header line - runeCount := utf8.RuneCountInString(line) - assert.Assert(t, runeCount <= terminalWidth, - "line %d has %d runes which exceeds terminal width %d: %q", - i, runeCount, terminalWidth, line) - } - } +// testClock returns a controllable clock starting at a fixed instant. +func testClock() (func() time.Time, *time.Time) { + now := time.Unix(1_700_000_000, 0) + return func() time.Time { return now }, &now } -func TestPrintWithDimensions_TimerIsRightAligned(t *testing.T) { - w, buf := newTestWriter() - - base := time.Unix(0, 0) - - // Long timer: "10.6s" (length 5) - longTask := &task{ - ID: "task-long", - parents: make(map[string]struct{}), - startTime: base, - endTime: base.Add(10*time.Second + 600*time.Millisecond), - text: "Pulled", - status: api.Done, - spinner: NewSpinner(), - } - longTask.spinner.Stop() - w.tasks[longTask.ID] = longTask - w.ids = append(w.ids, longTask.ID) - - // Short timer: "0.0s" (length 4) - shortTask := &task{ - ID: "task-short", - parents: make(map[string]struct{}), - startTime: base, - endTime: base, - text: "Pulled", - status: api.Done, - spinner: NewSpinner(), +func newTermWriter(width, height int) (*termWriter, *bytes.Buffer, *time.Time) { + var buf bytes.Buffer + clock, now := testClock() + w := &termWriter{ + out: &buf, + info: &buf, + tree: newTaskTree(), + scr: screen{out: &buf}, + size: func() (int, int) { return width, height }, + now: clock, + operation: "pull", } - shortTask.spinner.Stop() - w.tasks[shortTask.ID] = shortTask - w.ids = append(w.ids, shortTask.ID) - - terminalWidth := 80 - w.printWithDimensions(terminalWidth, 24) - - // Strip ANSI codes from output and split by newline - stripped := stripAnsi(buf.String()) - lines := strings.Split(stripped, "\n") + return w, &buf, now +} - var nonEmptyLines []string - for _, line := range lines { - if strings.TrimSpace(line) != "" { - nonEmptyLines = append(nonEmptyLines, line) - } +// feed applies events at the writer's current clock. +func feed(w *termWriter, events ...api.Resource) { + for _, e := range events { + w.tree.apply(e, w.now()) } +} - // Find the line containing the shorter timer. - var shortLine string - for _, line := range nonEmptyLines { - if strings.Contains(line, "0.0s") { - shortLine = line - break +// visualLines strips ANSI sequences from the painted frame and returns the +// visible rows. +func visualLines(buf *bytes.Buffer) []string { + var lines []string + for _, l := range strings.Split(stripAnsi(buf.String()), "\n") { + if strings.TrimSpace(l) != "" { + lines = append(lines, l) } } - assert.Assert(t, shortLine != "", "expected to find a rendered line containing \"0.0s\"") - assert.Assert(t, strings.HasSuffix(shortLine, "0.0s"), - "short timer should be left-padded (no trailing spaces after the timer); got: %q", - shortLine) + return lines } -func TestLenAnsi(t *testing.T) { - testCases := []struct { - input string - expected int - }{ - {"hello", 5}, - {"\x1b[32mhello\x1b[0m", 5}, - {"\x1b[1;32mgreen\x1b[0m text", 10}, - {"", 0}, - {"\x1b[0m", 0}, - } - - for _, tc := range testCases { - t.Run(tc.input, func(t *testing.T) { - result := lenAnsi(tc.input) - assert.Equal(t, tc.expected, result) +// adversarialEvents exercises every historical overflow trigger at once: +// unbounded status text (skippedEvent), CJK ids, long error details, layers +// with and without totals. +func adversarialEvents() []api.Resource { + events := []api.Resource{ + {ID: "app", Status: api.Warning, Text: "Skipped: current commandline does not match manifest, and image has no local build metadata"}, + {ID: "Image 测试测试测试测试-with-a-very-long-tag:v1.2.3-alpha.4", Status: api.Working, Text: "Pulling"}, + {ID: "db", Status: api.Error, Text: "Error", Details: "réseau « frontend » introuvable — vérifiez la configuration réseau du projet et les alias déclarés"}, + } + for i := range 20 { + events = append(events, api.Resource{ + ID: fmt.Sprintf("layer-%02d", i), + ParentID: "Image 测试测试测试测试-with-a-very-long-tag:v1.2.3-alpha.4", + Status: api.Working, + Text: "Downloading", + Current: int64(i) * 1_000_000, + Total: 50_000_000, + Percent: i * 5, }) } + return events } -// TestDoneBeforeStartDoesNotPanic guards against a nil-pointer panic if Done -// is ever called without a prior Start: Full must initialize done, since -// ttyWriter is exposed only through the public api.EventProcessor interface -// and a caller isn't guaranteed to call Start first. -func TestDoneBeforeStartDoesNotPanic(t *testing.T) { - w := Full(io.Discard, io.Discard, false) - w.Done("op", false) +func TestTerm_NoLineEverExceedsTerminalWidth(t *testing.T) { + for _, width := range []int{20, 30, 40, 60, 80, 120} { + t.Run(fmt.Sprintf("width=%d", width), func(t *testing.T) { + w, buf, _ := newTermWriter(width, 40) + feed(w, adversarialEvents()...) + w.repaint() + + for i, line := range visualLines(buf) { + got := runewidth.StringWidth(strings.TrimRight(line, " ")) + assert.Assert(t, got <= width, + "line %d is %d cells wide (> %d): %q", i, got, width, line) + } + }) + } } -func TestDoneDeadlockFix(t *testing.T) { - w, _ := newTestWriter() - addTask(w, "test-task", "Working", "details", api.Working) - ctx, cancel := context.WithCancel(t.Context()) - defer cancel() - - w.Start(ctx, "test") - done := make(chan bool) - go func() { - w.Done("test", true) - done <- true - }() +func TestTerm_TruncationPreservesUTF8(t *testing.T) { + w, buf, _ := newTermWriter(40, 24) + feed(w, adversarialEvents()...) + w.repaint() - select { - case <-done: - case <-time.After(5 * time.Second): - t.Fatal("Deadlock detected: Done() did not complete within 5 seconds") + for i, line := range visualLines(buf) { + assert.Assert(t, utf8.ValidString(line), + "line %d is invalid UTF-8 after truncation: %q", i, line) } } -// TestDoneAfterContextCancelDoesNotHang is the regression test for -// docker/compose#14114: a single SIGTERM/SIGINT cancels the command context -// (AdaptCmd). The render goroutine started by Start then exits through its -// ctx.Done() branch and never receives from the done channel. Done must -// still return. -func TestDoneAfterContextCancelDoesNotHang(t *testing.T) { - w, _ := newTestWriter() +// Done must never block, even when the operation context was cancelled +// (Ctrl-C) before Done runs — the historical unbuffered-channel handshake +// deadlocked in that ordering. +func TestTerm_DoneReturnsAfterContextCancel(t *testing.T) { + w, _, _ := newTermWriter(80, 24) ctx, cancel := context.WithCancel(t.Context()) - w.Start(ctx, "down") + w.Start(ctx, "pull") + w.On(api.Resource{ID: "Image foo", Text: "Pulling", Status: api.Working}) cancel() - // Let the render goroutine observe the cancellation and exit. - time.Sleep(100 * time.Millisecond) + time.Sleep(20 * time.Millisecond) // let the refresh goroutine exit finished := make(chan struct{}) go func() { - w.Done("down", false) + w.Done("pull", true) close(finished) }() select { case <-finished: case <-time.After(2 * time.Second): - t.Fatal("ttyWriter.Done blocked forever after context cancellation") + t.Fatal("Done() blocked after context cancellation") } } -// TestNestedStartDoneDoesNotPanic guards against a panic if Start/Done are -// ever nested (a Start called again before the matching Done): closing a -// channel more than once panics unless each cycle's own doneSignal (with its -// own sync.Once) is used, instead of a single shared channel. It also -// guards against a goroutine leak: the outer cycle's render goroutine must -// not be left waiting on a doneSignal nobody closes once Start replaces it. -func TestNestedStartDoneDoesNotPanic(t *testing.T) { - w, _ := newTestWriter() - ctx := t.Context() - - w.Start(ctx, "outer") - w.Start(ctx, "inner") - w.Done("inner", false) - w.Done("outer", false) - - // give the outer cycle's render goroutine a chance to observe the - // second Start closing its signal and exit before goleak checks. - time.Sleep(100 * time.Millisecond) - goleak.VerifyNone(t) -} +// The spinner animation must be a function of time, not of how often the +// layout runs. +func TestTerm_SpinnerIsTimeBased(t *testing.T) { + clock, now := testClock() + n := &node{status: api.Working, startedAt: clock()} -// TestSequentialStartDoneEachGetFreshChannel is the regression test for the -// bus going through more than one Start/Done cycle in its lifetime, as -// happens with `docker compose rm --stop` (a "stop" cycle, then its own -// "remove" cycle): each Start must allocate a fresh doneSignal, independent -// of any earlier cycle's already-closed one, or the second cycle's render -// goroutine would see it as immediately closed and exit without ever -// ticking. -func TestSequentialStartDoneEachGetFreshChannel(t *testing.T) { - w, _ := newTestWriter() - ctx := t.Context() - - w.Start(ctx, "first") - first := w.done - w.Done("first", false) - select { - case <-first.ch: - default: - t.Fatal("expected the first cycle's done channel to be closed after its Done") - } + a := spinGlyph(n, *now) + b := spinGlyph(n, *now) + assert.Equal(t, a.text, b.text, "same instant must yield the same frame") - w.Start(ctx, "second") - second := w.done - if second == first { - t.Fatal("expected Start to allocate a fresh done signal for the new cycle") - } - select { - case <-second.ch: - t.Fatal("expected the second cycle's done channel to still be open before its own Done") - default: - } + *now = now.Add(tickInterval) + c := spinGlyph(n, *now) + assert.Assert(t, a.text != c.text, "advancing the clock must advance the frame") +} - w.Done("second", false) - select { - case <-second.ch: - default: - t.Fatal("expected the second cycle's done channel to be closed after its own Done") - } +func TestTerm_DiffRepaintSkipsUnchangedRows(t *testing.T) { + var buf bytes.Buffer + s := screen{out: &buf} + s.paint([]string{"header", "row a", "row b"}, 80) + + buf.Reset() + s.paint([]string{"header", "row a CHANGED", "row b"}, 80) + out := buf.String() + erases := strings.Count(out, "\x1b[2K") + assert.Equal(t, 1, erases, "only the changed row should be erased and rewritten, got %d in %q", erases, out) + assert.Assert(t, !strings.Contains(out, "header"), "unchanged header must not be rewritten") + assert.Assert(t, strings.Contains(out, "row a CHANGED")) + + buf.Reset() + s.paint([]string{"header", "row a CHANGED", "row b"}, 80) + assert.Equal(t, "", buf.String(), "identical frame must write nothing") } -// TestAdjustLineWidth_WideProgressForcesSizeInfoDrop is the unit-level -// regression test for docker/compose#13595. When progress contains the -// " X.XMB / Y.YMB" size suffix and the bar makes beforeStatus large enough -// to overflow terminalWidth, taskID truncation alone cannot make the line -// fit: applyPadding's max(timerPad, 1) floor adds one char back, and the -// "..."-padding minimum (10 chars) on taskID puts a lower bound on -// beforeStatus. The size info portion of progress must therefore be -// droppable when overflow can't be eliminated otherwise. -func TestAdjustLineWidth_WideProgressForcesSizeInfoDrop(t *testing.T) { - w := &ttyWriter{} - // Mirror prepareLineData's layout: " [bar]" + " %7s / %-7s". - sizeSuffix := " 50MB / 100MB " - progress := " [" + strings.Repeat("⣿", 30) + "]" + sizeSuffix - lines := []lineData{{ - taskID: "Image mariadb:11", - progress: progress, - progressSizeBytes: len(sizeSuffix), - status: "Pulling", - statusColor: nocolor, - spinner: " ", - timer: "5.4s", - }} - - terminalWidth := 60 - timerLen := 4 - w.adjustLineWidth(lines, timerLen, terminalWidth) - w.applyPadding(lines, terminalWidth, timerLen) - - rendered := strings.TrimRight(lineText(lines[0]), "\n") - assert.Assert(t, lenAnsi(rendered) <= terminalWidth, - "line length %d should not exceed terminal width %d: %q", - lenAnsi(rendered), terminalWidth, rendered) +func TestTerm_ShrinkingFrameBlanksLeftoverRows(t *testing.T) { + var buf bytes.Buffer + s := screen{out: &buf} + s.paint([]string{"header", "row a", "row b"}, 80) + + buf.Reset() + s.paint([]string{"header", "row a"}, 80) + assert.Equal(t, 1, strings.Count(buf.String(), "\x1b[2K"), + "the leftover row must be blanked") } -// addParentWithDownloadingChildren wires a parent task with N children whose -// non-zero totals trigger the " X.XMB / Y.YMB" suffix in prepareLineData's -// progress field. Used by the multi-render regression test below. -func addParentWithDownloadingChildren(w *ttyWriter, parentID string, children int, totalBytes int64) { - parent := &task{ - ID: parentID, - parents: make(map[string]struct{}), - startTime: time.Now(), - text: "Pulling", - status: api.Working, - spinner: NewSpinner(), - } - w.tasks[parent.ID] = parent - w.ids = append(w.ids, parent.ID) - for i := range children { - c := &task{ - ID: fmt.Sprintf("%s/layer%d", parentID, i), - parents: map[string]struct{}{parent.ID: {}}, - startTime: time.Now(), - text: "Downloading", - status: api.Working, - total: totalBytes / int64(children), - current: totalBytes / int64(children) / 2, - percent: 50, - spinner: NewSpinner(), - } - w.tasks[c.ID] = c - w.ids = append(w.ids, c.ID) +func TestTerm_HeightCapAddsMoreMarker(t *testing.T) { + w, buf, _ := newTermWriter(80, 10) + var events []api.Resource + for i := range 30 { + events = append(events, api.Resource{ID: fmt.Sprintf("service-%02d", i), Text: "Creating", Status: api.Working}) } + feed(w, events...) + w.repaint() + + lines := visualLines(buf) + assert.Assert(t, len(lines) <= 10, "must not paint more rows than the terminal height, got %d", len(lines)) + assert.Assert(t, strings.Contains(lines[len(lines)-1], "more"), + "last line should advertise the hidden rows: %q", lines[len(lines)-1]) } -// TestPrintWithDimensions_MultipleRendersFit verifies the cross-render aspect -// of docker/compose#13595: even a single overflowing line desyncs the cursor -// on the following tick because aec.Up(numLines) counts logical lines while -// the terminal wraps visual lines. Use many concurrent parent tasks with -// wide progress bars in a narrow terminal so adjustLineWidth's truncation -// loop can't bring every line under terminalWidth without dropping size -// info from progress. -func TestPrintWithDimensions_MultipleRendersFit(t *testing.T) { - w, buf := newTestWriter() - // Two parents so the truncation loop must walk multiple lines; 30 children - // per parent makes each progress bar wide enough that taskID truncation - // alone can't bring the line under terminalWidth. - for i := range 2 { - addParentWithDownloadingChildren(w, - "Image very-long-name-image-"+string(rune('a'+i))+":v1.2.3", - 30, 100_000_000) - } +// Child tasks (image layers) are not rendered as rows: they only feed the +// parent's aggregated strip and size counters. +func TestTerm_ChildTasksAggregateIntoParentRow(t *testing.T) { + w, buf, _ := newTermWriter(100, 40) + feed(w, + api.Resource{ID: "Image nginx", Text: "Pulling", Status: api.Working}, + api.Resource{ID: "sha256:aaaa", ParentID: "Image nginx", Text: "Downloading", Status: api.Working, Current: 25_000_000, Total: 100_000_000, Percent: 25}, + ) + w.repaint() + + lines := visualLines(buf) + assert.Equal(t, 2, len(lines), "header + image only, got %v", lines) + image := lines[1] + assert.Assert(t, !strings.Contains(buf.String(), "sha256:aaaa"), "layers must not get their own row") + assert.Assert(t, strings.Contains(image, "25MB / 100MB"), "image row should aggregate layer sizes: %q", image) +} - terminalWidth := 60 - for tick := range 10 { - for _, t := range w.tasks { - if t.status == api.Working && t.total > 0 { - t.current = min(t.current+t.total/10, t.total) - } - } - buf.Reset() - w.printWithDimensions(terminalWidth, 24) - for i, line := range extractLines(buf) { - assert.Assert(t, lenAnsi(line) <= terminalWidth, - "tick %d line %d has length %d > terminalWidth %d: %q", - tick, i, lenAnsi(line), terminalWidth, line) - } +// Deterministic visual check with a fixed clock: overall shape, id +// truncation and right-aligned timers. +func TestTerm_VisualSnapshot(t *testing.T) { + w, buf, now := newTermWriter(50, 24) + feed(w, api.Resource{ID: "Image docker.io/library/nginx-long-name", Text: "Pulling", Status: api.Working}) + *now = now.Add(2 * time.Second) + feed(w, api.Resource{ID: "Image docker.io/library/nginx-long-name", Text: "Pulled", Status: api.Done}) + feed(w, api.Resource{ID: "Image docker.io/library/postgres-database", Text: "Pulling", Status: api.Working}) + w.repaint() + + lines := visualLines(buf) + // identical, character for character, to the legacy renderer's golden + // output in TestPrintWithDimensions_PulledAndPullingWithLongIDs + expected := []string{ + "[+] pull 1/2", + " ✔ Image docker.io/library/nginx-l... Pulled 2.0s", + " ⠋ Image docker.io/library/postgre... Pulling 0.0s", + } + assert.Equal(t, len(expected), len(lines)) + for i := range expected { + assert.Equal(t, expected[i], strings.TrimRight(lines[i], " "), "line %d", i) } } diff --git a/go.mod b/go.mod index bd1f292cd7f..a4d23a3cfdc 100644 --- a/go.mod +++ b/go.mod @@ -26,6 +26,7 @@ require ( github.com/google/uuid v1.6.0 github.com/hashicorp/go-version v1.9.0 github.com/jonboulle/clockwork v0.5.0 + github.com/mattn/go-runewidth v0.0.23 github.com/mattn/go-shellwords v1.0.14 github.com/mitchellh/go-ps v1.0.0 github.com/moby/buildkit v0.32.2 @@ -138,7 +139,6 @@ require ( github.com/lestrrat-go/option/v2 v2.0.0 // indirect github.com/mattn/go-colorable v0.1.14 // indirect github.com/mattn/go-isatty v0.0.20 // indirect - github.com/mattn/go-runewidth v0.0.23 // indirect github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b // indirect github.com/mitchellh/hashstructure/v2 v2.0.2 // indirect github.com/moby/docker-image-spec v1.3.1 // indirect From 22e5ccdc582146b18b4f22dd5d89f882cf4dd9a5 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Sat, 15 Aug 2026 17:51:38 +0200 Subject: [PATCH 2/4] fix(display): clip the header line to the terminal width like every other row The "[+] op N/M" header was the one line bypassing renderSegs, so on a very narrow terminal it could wrap and desync the cursor arithmetic the rest of the design guarantees against. Route it through the same clip and cover degenerate widths (8, 12 cells) in the invariant test. Co-Authored-By: Claude Fable 5 Signed-off-by: Nicolas De Loof --- cmd/display/tty_layout.go | 3 ++- cmd/display/tty_test.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cmd/display/tty_layout.go b/cmd/display/tty_layout.go index 08da3df36ff..9f66226111c 100644 --- a/cmd/display/tty_layout.go +++ b/cmd/display/tty_layout.go @@ -65,7 +65,8 @@ const ( // layoutFrame renders the whole progress block, one string per terminal row. func layoutFrame(t *taskTree, operation string, o layoutOpts) []string { done, total := t.counts() - lines := []string{fmt.Sprintf("[+] %s %d/%d", operation, done, total)} + header := fmt.Sprintf("[+] %s %d/%d", operation, done, total) + lines := []string{renderSegs([]seg{{text: header}}, o.width)} rows := buildRows(t, o) maxRows := max(o.height-2, 1) diff --git a/cmd/display/tty_test.go b/cmd/display/tty_test.go index 49b3c081b0e..0945ee8e65e 100644 --- a/cmd/display/tty_test.go +++ b/cmd/display/tty_test.go @@ -116,7 +116,9 @@ func adversarialEvents() []api.Resource { } func TestTerm_NoLineEverExceedsTerminalWidth(t *testing.T) { - for _, width := range []int{20, 30, 40, 60, 80, 120} { + // 8 and 12 exercise the degenerate widths where even the "[+] op N/M" + // header must be clipped rather than allowed to wrap + for _, width := range []int{8, 12, 20, 30, 40, 60, 80, 120} { t.Run(fmt.Sprintf("width=%d", width), func(t *testing.T) { w, buf, _ := newTermWriter(width, 40) feed(w, adversarialEvents()...) From c48995efc649d20fa22a6f6c5ef840e83c3bc23a Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Tue, 25 Aug 2026 11:44:59 +0200 Subject: [PATCH 3/4] fix(display): harden termWriter lifecycle against review findings on #14119 Transposes the applicable findings from the review of the ttyWriter fix (#14119) to the rewritten renderer: - Done waits for the refresh goroutine to exit (outside the lock), so nothing can repaint after Done returns and output printed right after an operation cannot be garbled by a stray frame - a nested Start retires the previous cycle instead of leaking its refresh goroutine until the parent context is cancelled - build suspension no longer leaks across cycles: Start resets it and starts a fresh block below whatever buildkit wrote - drop the dead strings.ToLower in startDependencies: the unexported start only reads projectName when no Project is passed The remaining findings are structural to this rewrite: Done is a single critical section, and per-cycle context cancellation replaces the doneSignal handshake. Signed-off-by: Nicolas De Loof --- cmd/display/tty.go | 49 ++++++++++++++++++++++++++++++------- cmd/display/tty_test.go | 54 +++++++++++++++++++++++++++++++++++++++++ pkg/compose/run.go | 3 +-- 3 files changed, 95 insertions(+), 11 deletions(-) diff --git a/cmd/display/tty.go b/cmd/display/tty.go index d858db88f96..a277da85658 100644 --- a/cmd/display/tty.go +++ b/cmd/display/tty.go @@ -69,11 +69,12 @@ type termWriter struct { detached bool dryRun bool - tree taskTree - scr screen - operation string - suspended bool - stopTicks context.CancelFunc + tree taskTree + scr screen + operation string + suspended bool + stopTicks context.CancelFunc + ticksExited chan struct{} // closed by the refresh goroutine when it returns // injected for tests size func() (width, height int) @@ -94,16 +95,33 @@ func termSize() (int, int) { func (w *termWriter) Start(ctx context.Context, operation string) { w.mu.Lock() defer w.mu.Unlock() + // A Start before the matching Done (nested brackets are a misuse, but + // Full is public API): retire the previous cycle so its refresh + // goroutine doesn't outlive it. + if w.stopTicks != nil { + w.stopTicks() + w.stopTicks = nil + } + // Build suspension is scoped to a cycle: a new operation must not stay + // silent because the previous one ended mid-build, and buildkit wrote + // below the last frame, so the new cycle starts a fresh block. + if w.suspended { + w.suspended = false + w.scr.reset() + } w.operation = operation // The refresh goroutine is bound to a derived context: parent // cancellation (Ctrl-C) and Done both stop it by cancelling, which can // never block — there is no channel handshake to miss. tickCtx, cancel := context.WithCancel(ctx) + exited := make(chan struct{}) w.stopTicks = cancel - go w.refresh(tickCtx) + w.ticksExited = exited + go w.refresh(tickCtx, exited) } -func (w *termWriter) refresh(ctx context.Context) { +func (w *termWriter) refresh(ctx context.Context, exited chan<- struct{}) { + defer close(exited) ticker := time.NewTicker(tickInterval) defer ticker.Stop() for { @@ -120,15 +138,25 @@ func (w *termWriter) refresh(ctx context.Context) { func (w *termWriter) Done(string, bool) { w.mu.Lock() - defer w.mu.Unlock() if w.stopTicks != nil { w.stopTicks() w.stopTicks = nil } + exited := w.ticksExited + w.ticksExited = nil w.repaint() // leave the final state on screen w.operation = "" // The tree and the screen survive: a follow-up operation on the same // writer (`up` chains create and start) extends the same block. + w.mu.Unlock() + + // Wait for the refresh goroutine outside the lock (it may be blocked on + // it, about to no-op since operation is cleared): once Done returns, + // nothing repaints anymore, so output printed right after an operation + // (a prompt, container logs) cannot be garbled by a stray frame. + if exited != nil { + <-exited + } } func (w *termWriter) On(events ...api.Resource) { @@ -140,7 +168,10 @@ func (w *termWriter) On(events ...api.Resource) { continue } if w.operation != "start" && (e.Text == api.StatusStarted || e.Text == api.StatusStarting) && !w.detached { - // skip those events to avoid mixing with container logs + // Deliberate: attached run/up stream container logs on this same + // terminal, and painting Starting/Started rows here would repaint + // over the first log lines of the container that just started. + // Outside an explicit `start` operation those events are dropped. continue } w.handle(e) diff --git a/cmd/display/tty_test.go b/cmd/display/tty_test.go index 0945ee8e65e..07e6066494f 100644 --- a/cmd/display/tty_test.go +++ b/cmd/display/tty_test.go @@ -26,6 +26,7 @@ import ( "unicode/utf8" "github.com/mattn/go-runewidth" + "go.uber.org/goleak" "gotest.tools/v3/assert" "github.com/docker/compose/v5/pkg/api" @@ -167,6 +168,59 @@ func TestTerm_DoneReturnsAfterContextCancel(t *testing.T) { } } +// Done is reachable through the public api.EventProcessor interface, so a +// caller isn't guaranteed to have called Start first: it must be a no-op, +// not a nil dereference. +func TestTerm_DoneBeforeStartDoesNotPanic(t *testing.T) { + w, _, _ := newTermWriter(80, 24) + w.Done("op", false) +} + +// Once Done returns, nothing repaints anymore: the refresh goroutine must +// not survive the bracket, or a stray frame could garble whatever the caller +// prints next (a prompt, container logs). goleak needs no settling sleep +// precisely because Done waits for the goroutine to exit. +func TestTerm_NoRenderGoroutineSurvivesDone(t *testing.T) { + w, _, _ := newTermWriter(80, 24) + w.Start(t.Context(), "pull") + w.On(api.Resource{ID: "Image foo", Text: "Pulling", Status: api.Working}) + w.Done("pull", true) + goleak.VerifyNone(t) +} + +// A Start before the matching Done (nested brackets are a misuse, but Full +// is public API) must retire the previous cycle instead of leaking its +// refresh goroutine until the parent context is cancelled. +func TestTerm_NestedStartRetiresPreviousCycle(t *testing.T) { + w, _, _ := newTermWriter(80, 24) + ctx := t.Context() + w.Start(ctx, "outer") + w.Start(ctx, "inner") + w.Done("inner", false) + w.Done("outer", false) + goleak.VerifyNone(t) +} + +// Build suspension is scoped to a cycle: an operation ending mid-build (a +// failed build aborts the bracket with suspended still set) must not leave +// the next operation's progress permanently silent. +func TestTerm_StartResetsBuildSuspension(t *testing.T) { + w, buf, _ := newTermWriter(80, 24) + w.Start(t.Context(), "build") + w.On(api.Resource{ID: "Image app", Text: api.StatusBuilding, Status: api.Working}) + w.Done("build", false) + + w.Start(t.Context(), "create") + w.On(api.Resource{ID: "container app-1", Text: "Creating", Status: api.Working}) + w.mu.Lock() + w.repaint() + w.mu.Unlock() + w.Done("create", true) + + frame := stripAnsi(buf.String()) + assert.Assert(t, strings.Contains(frame, "container app-1"), "expected the new cycle to paint, got: %q", frame) +} + // The spinner animation must be a function of time, not of how often the // layout runs. func TestTerm_SpinnerIsTimeBased(t *testing.T) { diff --git a/pkg/compose/run.go b/pkg/compose/run.go index 3b3308493d8..1f233fe32f5 100644 --- a/pkg/compose/run.go +++ b/pkg/compose/run.go @@ -23,7 +23,6 @@ import ( "os" "os/signal" "slices" - "strings" "github.com/compose-spec/compose-go/v2/types" "github.com/docker/cli/cli" @@ -295,7 +294,7 @@ func (s *composeService) startDependencies(ctx context.Context, project *types.P } if len(project.Services) > 0 { - return s.start(ctx, strings.ToLower(project.Name), api.StartOptions{ + return s.start(ctx, project.Name, api.StartOptions{ Project: project, }, nil) } From 6ba9fadbcdef9fdabb8ced829f16e0e4f1c66a43 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Tue, 25 Aug 2026 17:32:42 +0200 Subject: [PATCH 4/4] refactor(display): address review feedback on the term renderer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - layoutFrame clips its output to the terminal height: height==1 no longer overflows with header + more-marker - the two status-to-color switches merge into eventColor(status, def) - the clear-suspension sequence extracts into unsuspend() - the injected clock becomes a clockwork.Clock (already a direct dependency); tests advance a fake clock and the cancel test loses its settling sleep — Done waits for the refresh goroutine itself - roots, per-parent children and the completed count are maintained incrementally by apply(), so a 100ms tick with an unchanged tree no longer rescans every task - the frame<0 clamp in spinGlyph drops: time comes from the writer's single clock and never regresses; the invariant is documented on the model - Start's retire-previous-cycle comment spells out that cancellation only guarantees the old goroutine will exit, not that it already has Signed-off-by: Nicolas De Loof --- cmd/display/tty.go | 49 ++++++++++------------ cmd/display/tty_layout.go | 25 ++++++++---- cmd/display/tty_model.go | 70 ++++++++++++++++++-------------- cmd/display/tty_test.go | 85 ++++++++++++++++++++++++++++++--------- 4 files changed, 145 insertions(+), 84 deletions(-) diff --git a/cmd/display/tty.go b/cmd/display/tty.go index a277da85658..892f245dea1 100644 --- a/cmd/display/tty.go +++ b/cmd/display/tty.go @@ -21,9 +21,9 @@ import ( "fmt" "io" "sync" - "time" "github.com/buger/goterm" + "github.com/jonboulle/clockwork" "github.com/docker/compose/v5/pkg/api" ) @@ -46,7 +46,7 @@ func Full(out io.Writer, info io.Writer, detached bool, opts ...TermOption) api. tree: newTaskTree(), scr: screen{out: out}, size: termSize, - now: time.Now, + clock: clockwork.NewRealClock(), } for _, opt := range opts { opt(w) @@ -77,8 +77,8 @@ type termWriter struct { ticksExited chan struct{} // closed by the refresh goroutine when it returns // injected for tests - size func() (width, height int) - now func() time.Time + size func() (width, height int) + clock clockwork.Clock } func termSize() (int, int) { @@ -97,17 +97,17 @@ func (w *termWriter) Start(ctx context.Context, operation string) { defer w.mu.Unlock() // A Start before the matching Done (nested brackets are a misuse, but // Full is public API): retire the previous cycle so its refresh - // goroutine doesn't outlive it. + // goroutine doesn't outlive it. Cancellation only guarantees the old + // goroutine *will* exit — not that it already has when Start returns; + // a last repaint from it is harmless (same model, under the mutex). if w.stopTicks != nil { w.stopTicks() w.stopTicks = nil } // Build suspension is scoped to a cycle: a new operation must not stay - // silent because the previous one ended mid-build, and buildkit wrote - // below the last frame, so the new cycle starts a fresh block. + // silent because the previous one ended mid-build. if w.suspended { - w.suspended = false - w.scr.reset() + w.unsuspend() } w.operation = operation // The refresh goroutine is bound to a derived context: parent @@ -122,13 +122,13 @@ func (w *termWriter) Start(ctx context.Context, operation string) { func (w *termWriter) refresh(ctx context.Context, exited chan<- struct{}) { defer close(exited) - ticker := time.NewTicker(tickInterval) + ticker := w.clock.NewTicker(tickInterval) defer ticker.Stop() for { select { case <-ctx.Done(): return - case <-ticker.C: + case <-ticker.Chan(): w.mu.Lock() w.repaint() w.mu.Unlock() @@ -185,18 +185,24 @@ func (w *termWriter) handle(e api.Resource) { if e.Text == api.StatusBuilding { w.suspended = true } else if w.suspended { - w.suspended = false - w.scr.reset() + w.unsuspend() } - w.tree.apply(e, w.now()) + w.tree.apply(e, w.clock.Now()) if w.operation == "" { // outside any operation: degrade to one plain line per event - _, _ = fmt.Fprintf(w.out, "%s %s %s\n", e.ID, plainEventColor(e.Status)(e.Text), e.Details) + _, _ = fmt.Fprintf(w.out, "%s %s %s\n", e.ID, eventColor(e.Status, SuccessColor)(e.Text), e.Details) } } +// unsuspend clears build suspension and starts a fresh block below whatever +// buildkit wrote, instead of repainting over it. +func (w *termWriter) unsuspend() { + w.suspended = false + w.scr.reset() +} + func (w *termWriter) repaint() { if w.suspended || w.operation == "" || len(w.tree.nodes) == 0 { return @@ -206,18 +212,7 @@ func (w *termWriter) repaint() { width: width, height: height, dryRun: w.dryRun, - now: w.now(), + now: w.clock.Now(), }) w.scr.paint(lines, width) } - -func plainEventColor(s api.EventStatus) colorFunc { - switch s { - case api.Warning: - return WarningColor - case api.Error: - return ErrorColor - default: - return SuccessColor - } -} diff --git a/cmd/display/tty_layout.go b/cmd/display/tty_layout.go index 9f66226111c..00aa3f32cae 100644 --- a/cmd/display/tty_layout.go +++ b/cmd/display/tty_layout.go @@ -83,6 +83,12 @@ func layoutFrame(t *taskTree, operation string, o layoutOpts) []string { if more > 0 { lines = append(lines, renderSegs([]seg{{text: fmt.Sprintf(" ... %d more", more)}}, o.width)) } + // hard guarantee against degenerate heights (height==1 would otherwise + // still yield header + "more" = 2 lines): never return more lines than + // the terminal has rows + if o.height > 0 && len(lines) > o.height { + lines = lines[:o.height] + } return lines } @@ -101,7 +107,7 @@ type row struct { func buildRows(t *taskTree, o layoutOpts) []row { var rows []row - for _, root := range t.roots() { + for _, root := range t.roots { rows = append(rows, makeRow(t, root, o)) } return rows @@ -112,7 +118,7 @@ func makeRow(t *taskTree, n *node, o layoutOpts) row { spin: spinGlyph(n, o.now), id: n.id, status: n.text, - statusColor: colorFn(n.status), + statusColor: eventColor(n.status, nocolor), details: n.details, timer: fmt.Sprintf("%.1fs", nodeElapsed(n, o.now).Seconds()), } @@ -145,7 +151,7 @@ func aggregateProgress(t *taskTree, root *node) (strip, sizes string) { hideSizes bool glyphs []string ) - for _, child := range t.children(root.id) { + for _, child := range t.children[root.id] { if child.status == api.Working && child.total == 0 { hideSizes = true } @@ -313,7 +319,11 @@ var ( termSpinnerFrames = spinnerFrames() ) -func colorFn(s api.EventStatus) colorFunc { +// eventColor maps an event status to its display color; statuses without a +// dedicated color (typically Working) fall back to def, which differs +// between the progress rows (no color) and the plain per-event lines +// (success green). +func eventColor(s api.EventStatus, def colorFunc) colorFunc { switch s { case api.Done: return SuccessColor @@ -322,7 +332,7 @@ func colorFn(s api.EventStatus) colorFunc { case api.Error: return ErrorColor default: - return nocolor + return def } } @@ -345,10 +355,9 @@ func spinGlyph(n *node, now time.Time) seg { case api.Error: return seg{text: spinnerError, color: ErrorColor} default: + // now and startedAt come from the writer's single clock (see + // taskTree doc), so the difference is never negative frame := int(now.Sub(n.startedAt)/tickInterval) % len(termSpinnerFrames) - if frame < 0 { - frame = 0 - } return seg{text: termSpinnerFrames[frame], color: CountColor} } } diff --git a/cmd/display/tty_model.go b/cmd/display/tty_model.go index 0c1a981b457..b9716f6b5ad 100644 --- a/cmd/display/tty_model.go +++ b/cmd/display/tty_model.go @@ -52,13 +52,27 @@ func (n *node) completed() bool { // taskTree stores tasks in arrival order and resolves parent/child links. // It is a plain data structure: all mutation goes through apply, all time is // injected, so its behavior is exhaustively testable without a terminal. +// +// Roots, per-parent children and the completed count are maintained +// incrementally by apply: the layout reads them on every refresh tick, so a +// frame with an unchanged tree costs O(1) here instead of a full rescan. +// +// Every time value handed to apply (and to the layout) must come from the +// writer's single clock: the model relies on time never regressing between +// calls — spinner frames and elapsed timers are computed as bare +// differences against startedAt/endedAt. type taskTree struct { - order []string - nodes map[string]*node + nodes map[string]*node + roots []*node // nodes without any parent, in arrival order + children map[string][]*node // parent id → children, in link-arrival order + done int // nodes currently in a completed status } func newTaskTree() taskTree { - return taskTree{nodes: map[string]*node{}} + return taskTree{ + nodes: map[string]*node{}, + children: map[string][]*node{}, + } } // apply is the single state transition of the model. @@ -72,10 +86,19 @@ func (t *taskTree) apply(e api.Resource, now time.Time) { startedAt: now, } t.nodes[e.ID] = n - t.order = append(t.order, e.ID) + if e.ParentID == "" { + t.roots = append(t.roots, n) + } } if e.ParentID != "" { - n.parents.Add(e.ParentID) + if !n.parents.Has(e.ParentID) { + n.parents.Add(e.ParentID) + t.children[e.ParentID] = append(t.children[e.ParentID], n) + if len(n.parents) == 1 { + // first parent ever: the node is not a root after all + t.removeRoot(n) + } + } // Layers shared by several images receive the same events once per // image. Accept updates from the first declared parent only, so the // rendered state doesn't flicker between concurrent pull streams. @@ -92,41 +115,26 @@ func (t *taskTree) apply(e api.Resource, now time.Time) { n.total = max(n.total, e.Total) n.current = max(n.current, e.Current) n.percent = max(n.percent, e.Percent) - if n.completed() && !wasCompleted { + switch { + case n.completed() && !wasCompleted: n.endedAt = now + t.done++ + case !n.completed() && wasCompleted: + t.done-- } } -// roots returns the top-level tasks in arrival order. -func (t *taskTree) roots() []*node { - var roots []*node - for _, id := range t.order { - if n := t.nodes[id]; len(n.parents) == 0 { - roots = append(roots, n) - } - } - return roots -} - -// children returns the tasks attached to parent, in arrival order. A layer -// shared by several images is listed under each of them. -func (t *taskTree) children(parent string) []*node { - var children []*node - for _, id := range t.order { - if n := t.nodes[id]; n.parents.Has(parent) { - children = append(children, n) +func (t *taskTree) removeRoot(n *node) { + for i, root := range t.roots { + if root == n { + t.roots = append(t.roots[:i], t.roots[i+1:]...) + return } } - return children } // counts returns completed and total task counts, children included, to // preserve the historical "[+] pull 3/15" header semantics. func (t *taskTree) counts() (done, total int) { - for _, n := range t.nodes { - if n.completed() { - done++ - } - } - return done, len(t.nodes) + return t.done, len(t.nodes) } diff --git a/cmd/display/tty_test.go b/cmd/display/tty_test.go index 07e6066494f..6327da0d249 100644 --- a/cmd/display/tty_test.go +++ b/cmd/display/tty_test.go @@ -25,6 +25,7 @@ import ( "time" "unicode/utf8" + "github.com/jonboulle/clockwork" "github.com/mattn/go-runewidth" "go.uber.org/goleak" "gotest.tools/v3/assert" @@ -53,31 +54,31 @@ func stripAnsi(s string) string { return result.String() } -// testClock returns a controllable clock starting at a fixed instant. -func testClock() (func() time.Time, *time.Time) { - now := time.Unix(1_700_000_000, 0) - return func() time.Time { return now }, &now +// testClock returns a fake clock starting at a fixed instant; tests advance +// it explicitly instead of sleeping. +func testClock() *clockwork.FakeClock { + return clockwork.NewFakeClockAt(time.Unix(1_700_000_000, 0)) } -func newTermWriter(width, height int) (*termWriter, *bytes.Buffer, *time.Time) { +func newTermWriter(width, height int) (*termWriter, *bytes.Buffer, *clockwork.FakeClock) { var buf bytes.Buffer - clock, now := testClock() + clock := testClock() w := &termWriter{ out: &buf, info: &buf, tree: newTaskTree(), scr: screen{out: &buf}, size: func() (int, int) { return width, height }, - now: clock, + clock: clock, operation: "pull", } - return w, &buf, now + return w, &buf, clock } // feed applies events at the writer's current clock. func feed(w *termWriter, events ...api.Resource) { for _, e := range events { - w.tree.apply(e, w.now()) + w.tree.apply(e, w.clock.Now()) } } @@ -154,7 +155,7 @@ func TestTerm_DoneReturnsAfterContextCancel(t *testing.T) { w.Start(ctx, "pull") w.On(api.Resource{ID: "Image foo", Text: "Pulling", Status: api.Working}) cancel() - time.Sleep(20 * time.Millisecond) // let the refresh goroutine exit + // no settling sleep: Done itself waits for the refresh goroutine to exit finished := make(chan struct{}) go func() { @@ -221,18 +222,66 @@ func TestTerm_StartResetsBuildSuspension(t *testing.T) { assert.Assert(t, strings.Contains(frame, "container app-1"), "expected the new cycle to paint, got: %q", frame) } +// Degenerate terminal heights must never overflow: even height==1 (where +// header + "... more" would naively yield two lines) returns a single line. +func TestTerm_FrameNeverExceedsTerminalHeight(t *testing.T) { + tree := newTaskTree() + clock := testClock() + for _, id := range []string{"a", "b", "c"} { + tree.apply(api.Resource{ID: id, Text: "Pulling", Status: api.Working}, clock.Now()) + } + for height := 1; height <= 4; height++ { + lines := layoutFrame(&tree, "pull", layoutOpts{width: 40, height: height, now: clock.Now()}) + assert.Assert(t, len(lines) <= height, "height %d yielded %d lines", height, len(lines)) + } +} + +// roots, children and the completed count are maintained incrementally by +// apply; this locks the bookkeeping across the tricky transitions: a node +// created as root later gaining a parent, and a completed node going back +// to work. +func TestTerm_TreeIncrementalBookkeeping(t *testing.T) { + tree := newTaskTree() + clock := testClock() + + tree.apply(api.Resource{ID: "layer", Text: "Waiting", Status: api.Working}, clock.Now()) + assert.Equal(t, len(tree.roots), 1) + + // gaining a first parent demotes the node from the roots ("Image app" + // has no node of its own yet, so no root remains at this point) + tree.apply(api.Resource{ID: "layer", ParentID: "Image app", Text: "Downloading", Status: api.Working}, clock.Now()) + assert.Equal(t, len(tree.roots), 0) + tree.apply(api.Resource{ID: "Image app", Text: "Pulling", Status: api.Working}, clock.Now()) + assert.Equal(t, len(tree.roots), 1) + assert.Equal(t, tree.roots[0].id, "Image app") + assert.Equal(t, len(tree.children["Image app"]), 1) + + // completion transitions keep the running counter exact, both ways. + // "layer" was first seen without a parent, so its anchor is "" and only + // parentless events update its state (the anchor rule). + done, total := tree.counts() + assert.Equal(t, done, 0) + assert.Equal(t, total, 2) + tree.apply(api.Resource{ID: "layer", Text: "Pulled", Status: api.Done}, clock.Now()) + done, _ = tree.counts() + assert.Equal(t, done, 1) + tree.apply(api.Resource{ID: "layer", Text: "Downloading", Status: api.Working}, clock.Now()) + done, _ = tree.counts() + assert.Equal(t, done, 0) +} + // The spinner animation must be a function of time, not of how often the // layout runs. func TestTerm_SpinnerIsTimeBased(t *testing.T) { - clock, now := testClock() - n := &node{status: api.Working, startedAt: clock()} + clock := testClock() + n := &node{status: api.Working, startedAt: clock.Now()} - a := spinGlyph(n, *now) - b := spinGlyph(n, *now) + a := spinGlyph(n, clock.Now()) + b := spinGlyph(n, clock.Now()) assert.Equal(t, a.text, b.text, "same instant must yield the same frame") - *now = now.Add(tickInterval) - c := spinGlyph(n, *now) + clock.Advance(tickInterval) + c := spinGlyph(n, clock.Now()) assert.Assert(t, a.text != c.text, "advancing the clock must advance the frame") } @@ -300,9 +349,9 @@ func TestTerm_ChildTasksAggregateIntoParentRow(t *testing.T) { // Deterministic visual check with a fixed clock: overall shape, id // truncation and right-aligned timers. func TestTerm_VisualSnapshot(t *testing.T) { - w, buf, now := newTermWriter(50, 24) + w, buf, clock := newTermWriter(50, 24) feed(w, api.Resource{ID: "Image docker.io/library/nginx-long-name", Text: "Pulling", Status: api.Working}) - *now = now.Add(2 * time.Second) + clock.Advance(2 * time.Second) feed(w, api.Resource{ID: "Image docker.io/library/nginx-long-name", Text: "Pulled", Status: api.Done}) feed(w, api.Resource{ID: "Image docker.io/library/postgres-database", Text: "Pulling", Status: api.Working}) w.repaint()