test(picker): make UX regressions deterministic

commit: ba310a27a5685d676f0a54f65adcc2cf3e47c678
author: brice · 3mo ago · 2026-07-14 11:59 UTC
parent: 123b9fb
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +104 -38

M internal/usecase/daemon/palette_test.go +41 -31


@@ -4,7 +4,6 @@ import (
4	4		"errors"
5	5		"sync"
6	6		"testing"
7		-	"time"
8	7
9	8		"github.com/stretchr/testify/mock"
10	9		"github.com/stretchr/testify/require"

@@ -533,20 +532,20 @@ func TestPaletteRenderAndInputCanRunConcurrently(t *testing.T) {
533	532
534	533		func TestPaletteTabCompletesSelectedCommandWithoutForwardingToPTY(t *testing.T) {
535	534		tests := []struct {
536		-	name string
537		-	config map[string]string
538		-	recent string
539		-	query string
540		-	want string
541		-	wantRepaint bool
542		-	setupRepaints int
	535	+	name string
	536	+	config map[string]string
	537	+	recent string
	538	+	query string
	539	+	want string
	540	+	wantCompletionInvalidation bool
	541	+	setupInvalidations int
543	542		}{
544		-	{name: "changed completion repaints once", query: "NX", want: "NXT", wantRepaint: true},
545		-	{name: "exact argument command is a no-op", query: "JRS 1", want: "JRS 1", wantRepaint: false},
546		-	{name: "empty query starts at registry first", want: "CNT", wantRepaint: true},
547		-	{name: "empty query starts at daemon MRU", recent: "NXT", want: "NXT", wantRepaint: true},
548		-	{name: "effective configured override completes", config: map[string]string{"new-tab": "NEW"}, want: "NEW", wantRepaint: true, setupRepaints: 1},
549		-	{name: "stale pre-override MRU is ignored", config: map[string]string{"new-tab": "NEW"}, recent: "CNT", want: "NEW", wantRepaint: true, setupRepaints: 1},
	543	+	{name: "changed completion invalidates once", query: "NX", want: "NXT", wantCompletionInvalidation: true},
	544	+	{name: "exact argument command is a no-op", query: "JRS 1", want: "JRS 1"},
	545	+	{name: "empty query starts at registry first", want: "CNT", wantCompletionInvalidation: true},
	546	+	{name: "empty query starts at daemon MRU", recent: "NXT", want: "NXT", wantCompletionInvalidation: true},
	547	+	{name: "effective configured override completes", config: map[string]string{"new-tab": "NEW"}, want: "NEW", wantCompletionInvalidation: true, setupInvalidations: 1},
	548	+	{name: "stale pre-override MRU is ignored", config: map[string]string{"new-tab": "NEW"}, recent: "CNT", want: "NEW", wantCompletionInvalidation: true, setupInvalidations: 1},
550	549		}
551	550
552	551		for _, tt := range tests {

@@ -555,21 +554,25 @@ func TestPaletteTabCompletesSelectedCommandWithoutForwardingToPTY(t *testing.T)
555	554		p, release := newBlockingPTYWithWrites(t, writes)
556	555		defer release()
557	556		d, sess, ac, sends := newManualSessionWithPTYs(t, p)
	557	+	invs := installPaletteInvalidationObserver(sess)
558	558		if tt.recent != "" {
559	559		d.recordPaletteUse(tt.recent)
560	560		}
561	561		if tt.config != nil {
562	562		d.ApplyConfig(domain.Config{Codes: tt.config})
563		-	for range tt.setupRepaints {
564		-	awaitFrame(t, sends, ports.MsgOutput)
	563	+	for range tt.setupInvalidations {
	564	+	awaitInvalidation(t, invs)
565	565		}
	566	+	requireNoInvalidation(t, invs)
566	567		}
567	568
568	569		d.handleInput(sess, ac, []byte("\x1b "))
569		-	awaitFrame(t, sends, ports.MsgOutput) // drain palette open repaint
	570	+	awaitInvalidation(t, invs)
	571	+	requireNoInvalidation(t, invs)
570	572		if tt.query != "" {
571	573		d.handleInput(sess, ac, []byte(tt.query))
572		-	awaitFrame(t, sends, ports.MsgOutput) // drain typed-query repaint
	574	+	awaitInvalidation(t, invs)
	575	+	requireNoInvalidation(t, invs)
573	576		}
574	577
575	578		d.handleInput(sess, ac, []byte("\t"))

@@ -577,10 +580,11 @@ func TestPaletteTabCompletesSelectedCommandWithoutForwardingToPTY(t *testing.T)
577	580		got := ac.overlays.palette.Query()
578	581		ac.overlays.paletteMu.Unlock()
579	582		require.Equal(t, tt.want, got)
580		-	if tt.wantRepaint {
581		-	awaitFrame(t, sends, ports.MsgOutput)
	583	+	if tt.wantCompletionInvalidation {
	584	+	awaitInvalidation(t, invs)
582	585		}
583		-	requireNoPaletteFrame(t, sends)
	586	+	requireNoInvalidation(t, invs)
	587	+	requireNoOutputFrame(t, sends)
584	588		requireNoPTYWrite(t, writes)
585	589		})
586	590		}

@@ -591,27 +595,33 @@ func TestPaletteBatchedTabCompletionPreservesRepaintInvalidation(t *testing.T) {
591	595		p, release := newBlockingPTYWithWrites(t, writes)
592	596		defer release()
593	597		d, sess, ac, sends := newManualSessionWithPTYs(t, p)
	598	+	invs := installPaletteInvalidationObserver(sess)
594	599
595	600		d.handleInput(sess, ac, []byte("\x1b "))
596		-	awaitFrame(t, sends, ports.MsgOutput) // drain palette-open repaint
	601	+	awaitInvalidation(t, invs)
	602	+	requireNoInvalidation(t, invs)
597	603
598	604		d.handleInput(sess, ac, []byte("NX\t\t"))
599	605		ac.overlays.paletteMu.Lock()
600	606		got := ac.overlays.palette.Query()
601	607		ac.overlays.paletteMu.Unlock()
602	608		require.Equal(t, "NXT", got)
603		-	awaitFrame(t, sends, ports.MsgOutput)
604		-	requireNoPaletteFrame(t, sends)
	609	+	awaitInvalidation(t, invs)
	610	+	requireNoInvalidation(t, invs)
	611	+	requireNoOutputFrame(t, sends)
605	612		requireNoPTYWrite(t, writes)
606	613		}
607	614
608		-	func requireNoPaletteFrame(t *testing.T, sends <-chan ports.Frame) {
609		-	t.Helper()
610		-	select {
611		-	case frame := <-sends:
612		-	t.Fatalf("unexpected frame after no-op Tab: %#v", frame)
613		-	case <-time.After(50 * time.Millisecond):
614		-	}
	615	+	func installPaletteInvalidationObserver(sess *session) chan renderInvalidation {
	616	+	invs := make(chan renderInvalidation, 8)
	617	+	sess.installRenderCoordinator(newRenderCoordinator(renderCoordinatorOptions{
	618	+	clock: nil,
	619	+	wake: nil,
	620	+	onInvalidate: func(inv renderInvalidation) {
	621	+	invs <- inv
	622	+	},
	623	+	}))
	624	+	return invs
615	625		}
616	626
617	627		func TestPaletteExecMethods(t *testing.T) {

 	"errors"
 	"sync"
 	"testing"
-	"time"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 func TestPaletteTabCompletesSelectedCommandWithoutForwardingToPTY(t *testing.T) {
 	tests := []struct {
-		name          string
+		name                       string
-		config        map[string]string
+		config                     map[string]string
-		recent        string
+		recent                     string
-		query         string
+		query                      string
-		want          string
+		want                       string
-		wantRepaint   bool
+		wantCompletionInvalidation bool
-		setupRepaints int
+		setupInvalidations         int
 	}{
-		{name: "changed completion repaints once", query: "NX", want: "NXT", wantRepaint: true},
+		{name: "changed completion invalidates once", query: "NX", want: "NXT", wantCompletionInvalidation: true},
-		{name: "exact argument command is a no-op", query: "JRS 1", want: "JRS 1", wantRepaint: false},
+		{name: "exact argument command is a no-op", query: "JRS 1", want: "JRS 1"},
-		{name: "empty query starts at registry first", want: "CNT", wantRepaint: true},
+		{name: "empty query starts at registry first", want: "CNT", wantCompletionInvalidation: true},
-		{name: "empty query starts at daemon MRU", recent: "NXT", want: "NXT", wantRepaint: true},
+		{name: "empty query starts at daemon MRU", recent: "NXT", want: "NXT", wantCompletionInvalidation: true},
-		{name: "effective configured override completes", config: map[string]string{"new-tab": "NEW"}, want: "NEW", wantRepaint: true, setupRepaints: 1},
+		{name: "effective configured override completes", config: map[string]string{"new-tab": "NEW"}, want: "NEW", wantCompletionInvalidation: true, setupInvalidations: 1},
-		{name: "stale pre-override MRU is ignored", config: map[string]string{"new-tab": "NEW"}, recent: "CNT", want: "NEW", wantRepaint: true, setupRepaints: 1},
+		{name: "stale pre-override MRU is ignored", config: map[string]string{"new-tab": "NEW"}, recent: "CNT", want: "NEW", wantCompletionInvalidation: true, setupInvalidations: 1},
 	}
 	for _, tt := range tests {
 			p, release := newBlockingPTYWithWrites(t, writes)
 			defer release()
 			d, sess, ac, sends := newManualSessionWithPTYs(t, p)
+			invs := installPaletteInvalidationObserver(sess)
 			if tt.recent != "" {
 				d.recordPaletteUse(tt.recent)
 			}
 			if tt.config != nil {
 				d.ApplyConfig(domain.Config{Codes: tt.config})
-				for range tt.setupRepaints {
+				for range tt.setupInvalidations {
-					awaitFrame(t, sends, ports.MsgOutput)
+					awaitInvalidation(t, invs)
 				}
+				requireNoInvalidation(t, invs)
 			}
 			d.handleInput(sess, ac, []byte("\x1b "))
-			awaitFrame(t, sends, ports.MsgOutput) // drain palette open repaint
+			awaitInvalidation(t, invs)
+			requireNoInvalidation(t, invs)
 			if tt.query != "" {
 				d.handleInput(sess, ac, []byte(tt.query))
-				awaitFrame(t, sends, ports.MsgOutput) // drain typed-query repaint
+				awaitInvalidation(t, invs)
+				requireNoInvalidation(t, invs)
 			}
 			d.handleInput(sess, ac, []byte("\t"))
 			got := ac.overlays.palette.Query()
 			ac.overlays.paletteMu.Unlock()
 			require.Equal(t, tt.want, got)
-			if tt.wantRepaint {
+			if tt.wantCompletionInvalidation {
-				awaitFrame(t, sends, ports.MsgOutput)
+				awaitInvalidation(t, invs)
 			}
-			requireNoPaletteFrame(t, sends)
+			requireNoInvalidation(t, invs)
+			requireNoOutputFrame(t, sends)
 			requireNoPTYWrite(t, writes)
 		})
 	}
 	p, release := newBlockingPTYWithWrites(t, writes)
 	defer release()
 	d, sess, ac, sends := newManualSessionWithPTYs(t, p)
+	invs := installPaletteInvalidationObserver(sess)
 	d.handleInput(sess, ac, []byte("\x1b "))
-	awaitFrame(t, sends, ports.MsgOutput) // drain palette-open repaint
+	awaitInvalidation(t, invs)
+	requireNoInvalidation(t, invs)
 	d.handleInput(sess, ac, []byte("NX\t\t"))
 	ac.overlays.paletteMu.Lock()
 	got := ac.overlays.palette.Query()
 	ac.overlays.paletteMu.Unlock()
 	require.Equal(t, "NXT", got)
-	awaitFrame(t, sends, ports.MsgOutput)
+	awaitInvalidation(t, invs)
-	requireNoPaletteFrame(t, sends)
+	requireNoInvalidation(t, invs)
+	requireNoOutputFrame(t, sends)
 	requireNoPTYWrite(t, writes)
 }
-func requireNoPaletteFrame(t *testing.T, sends <-chan ports.Frame) {
+func installPaletteInvalidationObserver(sess *session) chan renderInvalidation {
-	t.Helper()
+	invs := make(chan renderInvalidation, 8)
-	select {
+	sess.installRenderCoordinator(newRenderCoordinator(renderCoordinatorOptions{
-	case frame := <-sends:
+		clock: nil,
-		t.Fatalf("unexpected frame after no-op Tab: %#v", frame)
+		wake:  nil,
-	case <-time.After(50 * time.Millisecond):
+		onInvalidate: func(inv renderInvalidation) {
-	}
+			invs <- inv
+		},
+	}))
+	return invs
 }
 func TestPaletteExecMethods(t *testing.T) {

M internal/usecase/daemon/picker_test.go +63 -7


@@ -2,6 +2,7 @@ package daemon
2	2
3	3		import (
4	4		"context"
	5	+	"runtime"
5	6		"strings"
6	7		"testing"
7	8		"time"

@@ -637,14 +638,15 @@ func TestCaptureOverlayLayersResizeRecomposesPickerWithoutStalePreview(t *testin
637	638		}
638	639		}
639	640
640		-	func TestPickerPreviewDoesNotCloneScrollback(t *testing.T) {
641		-	tb := newTab(nil, domain.Size{Cols: 10, Rows: 3})
642		-	p := tb.focusedPane()
643		-	p.screen.Write([]byte(strings.Repeat("history-only-marker\n", 10_000)))
644		-	p.screen.Write([]byte("\rNOW"))
645		-
646		-	preview := snapshotPickerPreview(tb)
	641	+	func TestPickerPreviewAllocationsAreIndependentOfTenThousandScrollbackRows(t *testing.T) {
	642	+	empty := newPickerPreviewTabWithHistoryRows(0)
	643	+	withScrollback := newPickerPreviewTabWithHistoryRows(10_000)
	644	+	require.Zero(t, empty.focusedPane().screen.History().Len())
	645	+	require.Equal(t, 10_000, withScrollback.focusedPane().screen.History().Len())
647	646
	647	+	preview := snapshotPickerPreview(withScrollback)
	648	+	emptyPreview := snapshotPickerPreview(empty)
	649	+	require.Equal(t, emptyPreview, preview, "identical visible tabs must produce identical previews")
648	650		require.Equal(t, 10, preview.Width)
649	651		require.Equal(t, 3, preview.Height)
650	652		require.Len(t, preview.Rows, preview.Height)

@@ -663,6 +665,60 @@ func TestPickerPreviewDoesNotCloneScrollback(t *testing.T) {
663	665		}(), "\n")
664	666		require.Contains(t, got, "NOW")
665	667		require.NotContains(t, got, "history-only-marker")
	668	+
	669	+	emptyResult := benchmarkPickerPreview(empty, snapshotPickerPreview)
	670	+	scrollbackResult := benchmarkPickerPreview(withScrollback, snapshotPickerPreview)
	671	+	require.Equal(t, emptyResult.AllocsPerOp(), scrollbackResult.AllocsPerOp(), "preview allocations must not depend on retained scrollback")
	672	+	require.Equal(t, emptyResult.AllocedBytesPerOp(), scrollbackResult.AllocedBytesPerOp(), "preview allocated bytes must not depend on retained scrollback")
	673	+
	674	+	// The local seam deliberately clones every history row. Its larger result
	675	+	// demonstrates that the allocation assertion would fail if preview capture
	676	+	// regressed to copying scrollback, without changing production code to make
	677	+	// this test red.
	678	+	cloningEmptyResult := benchmarkPickerPreview(empty, snapshotPickerPreviewWithClonedHistoryForTest)
	679	+	cloningScrollbackResult := benchmarkPickerPreview(withScrollback, snapshotPickerPreviewWithClonedHistoryForTest)
	680	+	require.Greater(t, cloningScrollbackResult.AllocsPerOp(), cloningEmptyResult.AllocsPerOp())
	681	+	require.Greater(t, cloningScrollbackResult.AllocedBytesPerOp(), cloningEmptyResult.AllocedBytesPerOp())
	682	+	}
	683	+
	684	+	func newPickerPreviewTabWithHistoryRows(historyRows int) *tab {
	685	+	tb := newTab(nil, domain.Size{Cols: 10, Rows: 3})
	686	+	p := tb.focusedPane()
	687	+	for range historyRows {
	688	+	p.screen.History().Append(testRow("history-only-marker"))
	689	+	}
	690	+	p.screen.Write([]byte("NOW"))
	691	+	return tb
	692	+	}
	693	+
	694	+	func benchmarkPickerPreview(tb tab, snapshot func(tab) picker.Preview) testing.BenchmarkResult {
	695	+	return testing.Benchmark(func(b *testing.B) {
	696	+	b.ReportAllocs()
	697	+	for range b.N {
	698	+	runtime.KeepAlive(snapshot(tb))
	699	+	}
	700	+	})
	701	+	}
	702	+
	703	+	func snapshotPickerPreviewWithClonedHistoryForTest(tb *tab) picker.Preview {
	704	+	tb.mu.Lock()
	705	+	p := tb.focusedPane()
	706	+	if p == nil {
	707	+	tb.mu.Unlock()
	708	+	return picker.Preview{}
	709	+	}
	710	+	p.mu.Lock()
	711	+	history := p.screen.History().View()
	712	+	p.mu.Unlock()
	713	+	tb.mu.Unlock()
	714	+
	715	+	var cloned [][]renderer.Cell
	716	+	history.Range(func(row []renderer.Cell) bool {
	717	+	cloned = append(cloned, append([]renderer.Cell(nil), row...))
	718	+	return true
	719	+	})
	720	+	runtime.KeepAlive(cloned)
	721	+	return snapshotPickerPreview(tb)
666	722		}
667	723
668	724		func TestPickerPreviewSinglePaneSnapshotsFocusedPane(t *testing.T) {


@@ -2,6 +2,7 @@ package daemon
2			2
3		import (	3		import (
4		"context"	4		"context"
			5	+	"runtime"
5		"strings"	6		"strings"
6		"testing"	7		"testing"
7		"time"	8		"time"

@@ -637,14 +638,15 @@ func TestCaptureOverlayLayersResizeRecomposesPickerWithoutStalePreview(t *testin
637		}	638		}
638		}	639		}
639			640
640	-	func TestPickerPreviewDoesNotCloneScrollback(t *testing.T) {	641	+	func TestPickerPreviewAllocationsAreIndependentOfTenThousandScrollbackRows(t *testing.T) {
641	-	tb := newTab(nil, domain.Size{Cols: 10, Rows: 3})	642	+	empty := newPickerPreviewTabWithHistoryRows(0)
642	-	p := tb.focusedPane()	643	+	withScrollback := newPickerPreviewTabWithHistoryRows(10_000)
643	-	p.screen.Write([]byte(strings.Repeat("history-only-marker\n", 10_000)))	644	+	require.Zero(t, empty.focusedPane().screen.History().Len())
644	-	p.screen.Write([]byte("\rNOW"))	645	+	require.Equal(t, 10_000, withScrollback.focusedPane().screen.History().Len())
645	-
646	-	preview := snapshotPickerPreview(tb)
647			646
			647	+	preview := snapshotPickerPreview(withScrollback)
			648	+	emptyPreview := snapshotPickerPreview(empty)
			649	+	require.Equal(t, emptyPreview, preview, "identical visible tabs must produce identical previews")
648		require.Equal(t, 10, preview.Width)	650		require.Equal(t, 10, preview.Width)
649		require.Equal(t, 3, preview.Height)	651		require.Equal(t, 3, preview.Height)
650		require.Len(t, preview.Rows, preview.Height)	652		require.Len(t, preview.Rows, preview.Height)

@@ -663,6 +665,60 @@ func TestPickerPreviewDoesNotCloneScrollback(t *testing.T) {
663		}(), "\n")	665		}(), "\n")
664		require.Contains(t, got, "NOW")	666		require.Contains(t, got, "NOW")
665		require.NotContains(t, got, "history-only-marker")	667		require.NotContains(t, got, "history-only-marker")
			668	+
			669	+	emptyResult := benchmarkPickerPreview(empty, snapshotPickerPreview)
			670	+	scrollbackResult := benchmarkPickerPreview(withScrollback, snapshotPickerPreview)
			671	+	require.Equal(t, emptyResult.AllocsPerOp(), scrollbackResult.AllocsPerOp(), "preview allocations must not depend on retained scrollback")
			672	+	require.Equal(t, emptyResult.AllocedBytesPerOp(), scrollbackResult.AllocedBytesPerOp(), "preview allocated bytes must not depend on retained scrollback")
			673	+
			674	+	// The local seam deliberately clones every history row. Its larger result
			675	+	// demonstrates that the allocation assertion would fail if preview capture
			676	+	// regressed to copying scrollback, without changing production code to make
			677	+	// this test red.
			678	+	cloningEmptyResult := benchmarkPickerPreview(empty, snapshotPickerPreviewWithClonedHistoryForTest)
			679	+	cloningScrollbackResult := benchmarkPickerPreview(withScrollback, snapshotPickerPreviewWithClonedHistoryForTest)
			680	+	require.Greater(t, cloningScrollbackResult.AllocsPerOp(), cloningEmptyResult.AllocsPerOp())
			681	+	require.Greater(t, cloningScrollbackResult.AllocedBytesPerOp(), cloningEmptyResult.AllocedBytesPerOp())
			682	+	}
			683	+
			684	+	func newPickerPreviewTabWithHistoryRows(historyRows int) *tab {
			685	+	tb := newTab(nil, domain.Size{Cols: 10, Rows: 3})
			686	+	p := tb.focusedPane()
			687	+	for range historyRows {
			688	+	p.screen.History().Append(testRow("history-only-marker"))
			689	+	}
			690	+	p.screen.Write([]byte("NOW"))
			691	+	return tb
			692	+	}
			693	+
			694	+	func benchmarkPickerPreview(tb tab, snapshot func(tab) picker.Preview) testing.BenchmarkResult {
			695	+	return testing.Benchmark(func(b *testing.B) {
			696	+	b.ReportAllocs()
			697	+	for range b.N {
			698	+	runtime.KeepAlive(snapshot(tb))
			699	+	}
			700	+	})
			701	+	}
			702	+
			703	+	func snapshotPickerPreviewWithClonedHistoryForTest(tb *tab) picker.Preview {
			704	+	tb.mu.Lock()
			705	+	p := tb.focusedPane()
			706	+	if p == nil {
			707	+	tb.mu.Unlock()
			708	+	return picker.Preview{}
			709	+	}
			710	+	p.mu.Lock()
			711	+	history := p.screen.History().View()
			712	+	p.mu.Unlock()
			713	+	tb.mu.Unlock()
			714	+
			715	+	var cloned [][]renderer.Cell
			716	+	history.Range(func(row []renderer.Cell) bool {
			717	+	cloned = append(cloned, append([]renderer.Cell(nil), row...))
			718	+	return true
			719	+	})
			720	+	runtime.KeepAlive(cloned)
			721	+	return snapshotPickerPreview(tb)
666		}	722		}
667			723
668		func TestPickerPreviewSinglePaneSnapshotsFocusedPane(t *testing.T) {	724		func TestPickerPreviewSinglePaneSnapshotsFocusedPane(t *testing.T) {