Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion pkg/evaluation/agent_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,14 @@ const NoAgentImage = "none"
// Falls back to the rolling :edge image when the host binary isn't a release
// build (version.Version isn't a valid semantic version, e.g. "dev" or "pr").
func DefaultAgentImage() string {
v, err := semver.NewVersion(version.Version)
return defaultAgentImageFor(version.Version)
}

// defaultAgentImageFor is the pure core of DefaultAgentImage, taking the host
// CLI version explicitly so tests can cover it without mutating the global
// version.Version (which races with parallel tests reading it).
func defaultAgentImageFor(hostVersion string) string {
v, err := semver.NewVersion(hostVersion)
if err != nil {
return edgeAgentImage
}
Expand Down
24 changes: 7 additions & 17 deletions pkg/evaluation/agent_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,11 @@ import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/docker/docker-agent/pkg/version"
)

// withVersion temporarily overrides version.Version for the duration of the
// test, restoring it afterwards. version.Version is a package-level var (set
// via -ldflags at release build time), so tests mutate it directly rather
// than plumbing it through as a parameter.
func withVersion(t *testing.T, v string) {
t.Helper()
original := version.Version
version.Version = v
t.Cleanup(func() { version.Version = original })
}

func TestDefaultAgentImage(t *testing.T) {
t.Parallel()

tests := []struct {
name string
version string
Expand All @@ -35,28 +24,29 @@ func TestDefaultAgentImage(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
withVersion(t, tt.version)
assert.Equal(t, tt.want, DefaultAgentImage())
t.Parallel()
assert.Equal(t, tt.want, defaultAgentImageFor(tt.version))
})
}
}

func TestResolvedAgentImage(t *testing.T) {
withVersion(t, "v1.133.0")
t.Parallel()

tests := []struct {
name string
agentImage string
want string
}{
{name: "default", agentImage: "", want: "docker/docker-agent:1.133.0"},
{name: "default", agentImage: "", want: DefaultAgentImage()},
{name: "skip injection", agentImage: NoAgentImage, want: ""},
{name: "explicit override", agentImage: "docker/docker-agent:1.100.0", want: "docker/docker-agent:1.100.0"},
{name: "explicit override, different registry", agentImage: "myregistry.example.com/docker-agent:custom", want: "myregistry.example.com/docker-agent:custom"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
cfg := Config{AgentImage: tt.agentImage}
assert.Equal(t, tt.want, ResolvedAgentImage(cfg))
})
Expand Down
20 changes: 9 additions & 11 deletions pkg/evaluation/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package evaluation

import (
"bufio"
"bytes"
"cmp"
"context"
"encoding/json"
Expand Down Expand Up @@ -495,20 +496,17 @@ func (r *Runner) runDockerAgentInContainer(ctx context.Context, imageID string,
if err != nil {
return nil, fmt.Errorf("creating stdout pipe: %w", err)
}
stderr, err := cmd.StderrPipe()
if err != nil {
return nil, fmt.Errorf("creating stderr pipe: %w", err)
}
// Let exec.Cmd own the stderr copy: Wait blocks until its own copying
// goroutine finishes (bounded by WaitDelay), so stderrBuf is safe to read
// after Wait returns. A hand-rolled StderrPipe+goroutine has no such
// guarantee and races with the reads below.
var stderrBuf bytes.Buffer
cmd.Stderr = &stderrBuf

if err := cmd.Start(); err != nil {
return nil, fmt.Errorf("starting %s run: %w", containerRuntime, err)
}

var stderrData []byte
go func() {
stderrData, _ = io.ReadAll(stderr)
}()

var events []map[string]any
scanner := bufio.NewScanner(stdout)
scanner.Buffer(make([]byte, 0, 1024*1024), 10*1024*1024)
Expand All @@ -533,11 +531,11 @@ func (r *Runner) runDockerAgentInContainer(ctx context.Context, imageID string,

waitErr := cmd.Wait()
if waitErr != nil {
slog.DebugContext(ctx, "Container exited with error", "stderr", string(stderrData), "error", waitErr)
slog.DebugContext(ctx, "Container exited with error", "stderr", stderrBuf.String(), "error", waitErr)
}

if len(events) == 0 {
stderrStr := strings.TrimSpace(string(stderrData))
stderrStr := strings.TrimSpace(stderrBuf.String())
if waitErr != nil {
return nil, fmt.Errorf("container failed: %w (stderr: %s)", waitErr, stderrStr)
}
Expand Down
4 changes: 1 addition & 3 deletions pkg/evaluation/save_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,6 @@ func TestSaveRunSessionsJSONContainerRuntime(t *testing.T) {
func TestSaveRunSessionsJSONAgentImage(t *testing.T) {
t.Parallel()

withVersion(t, "v1.133.0")

save := func(t *testing.T, cfg Config) []byte {
t.Helper()
run := &EvalRun{
Expand All @@ -366,7 +364,7 @@ func TestSaveRunSessionsJSONAgentImage(t *testing.T) {

var output RunOutput
require.NoError(t, json.Unmarshal(data, &output))
assert.Equal(t, "docker/docker-agent:1.133.0", output.Config.AgentImage)
assert.Equal(t, DefaultAgentImage(), output.Config.AgentImage)
})

t.Run("records an explicit override", func(t *testing.T) {
Expand Down
11 changes: 2 additions & 9 deletions pkg/tui/tui_first_chunk_terminal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,10 @@ import (
)

func TestActualProgramFirstChunkReplacesPrimedSpinnerWithoutClick(t *testing.T) {
root, _, _ := wallClockRoot(t, 120, 40)
root, _, _ := frozenClockRoot(t, 120, 40)
_, _ = root.Update(messages.RoutedMsg{SessionID: "profile", Inner: agentruntime.StreamStarted("profile", "root")})
model := &streamingMotionModel{root: root, ready: make(chan struct{})}
program := tea.NewProgram(model, tea.WithInput(nil), tea.WithOutput(&wallClockCountingWriter{}), tea.WithWindowSize(120, 40))
done := make(chan error, 1)
go func() { _, err := program.Run(); done <- err }()
<-model.ready
program := startStreamingMotionProgram(t, model, tea.WithOutput(&wallClockCountingWriter{}))
before := programFrame(t, program)
require.NotEmpty(t, strings.TrimSpace(ansi.Strip(before)))

Expand All @@ -32,8 +29,4 @@ func TestActualProgramFirstChunkReplacesPrimedSpinnerWithoutClick(t *testing.T)
programAck(t, program)
recovered := programFrame(t, program)
require.Equal(t, ansi.Strip(current), ansi.Strip(recovered), "inert click must not repair the current viewport")

program.Quit()
require.NoError(t, <-done)
root.ar.Stop()
}
26 changes: 4 additions & 22 deletions pkg/tui/tui_hover_stream_reentry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,10 @@ func programAck(t *testing.T, program *tea.Program) {
}

func TestActualProgramPendingSpinnerHoverIsFrameIsolated(t *testing.T) {
root, _, _ := wallClockRoot(t, 120, 40)
root, _, _ := frozenClockRoot(t, 120, 40)
_, _ = root.Update(messages.RoutedMsg{SessionID: "profile", Inner: agentruntime.StreamStarted("profile", "root")})
model := &streamingMotionModel{root: root, ready: make(chan struct{})}
program := tea.NewProgram(model, tea.WithInput(nil), tea.WithOutput(&wallClockCountingWriter{}), tea.WithWindowSize(120, 40))
done := make(chan error, 1)
go func() { _, err := program.Run(); done <- err }()
<-model.ready
program := startStreamingMotionProgram(t, model, tea.WithOutput(&wallClockCountingWriter{}))

baseline := programFrame(t, program)
require.NotEmpty(t, strings.TrimSpace(ansi.Strip(baseline)), "pending spinner frame")
Expand All @@ -67,9 +64,6 @@ func TestActualProgramPendingSpinnerHoverIsFrameIsolated(t *testing.T) {
programAck(t, program)
require.Equal(t, baseline, programFrame(t, program), "leave restores the exact same-elapsed frame")
}
program.Quit()
require.NoError(t, <-done)
root.ar.Stop()
}

func TestActualProgramVirtualSuffixKeyWheelPageMatrixNeverBlanks(t *testing.T) {
Expand All @@ -81,10 +75,7 @@ func TestActualProgramVirtualSuffixKeyWheelPageMatrixNeverBlanks(t *testing.T) {
_ = root.View()
}
model := &streamingMotionModel{root: root, ready: make(chan struct{})}
program := tea.NewProgram(model, tea.WithInput(nil), tea.WithOutput(&wallClockCountingWriter{}), tea.WithWindowSize(120, 40))
done := make(chan error, 1)
go func() { _, err := program.Run(); done <- err }()
<-model.ready
program := startStreamingMotionProgram(t, model, tea.WithOutput(&wallClockCountingWriter{}))

for _, msg := range []tea.Msg{
messages.WheelCoalescedMsg{Delta: -1_000_000, X: 40, Y: 20},
Expand All @@ -105,9 +96,6 @@ func TestActualProgramVirtualSuffixKeyWheelPageMatrixNeverBlanks(t *testing.T) {
}
frame := programFrame(t, program)
require.Contains(t, ansi.Strip(frame), "stream marker", "exact bottom dropped virtual active suffix")
program.Quit()
require.NoError(t, <-done)
root.ar.Stop()
}

func TestActualProgramHoverThenBottomReentryStaysBounded(t *testing.T) {
Expand All @@ -122,10 +110,7 @@ func TestActualProgramHoverThenBottomReentryStaysBounded(t *testing.T) {
before := root.View().Content
model := &streamingMotionModel{root: root, ready: make(chan struct{})}
writer := &wallClockCountingWriter{}
program := tea.NewProgram(model, tea.WithInput(nil), tea.WithOutput(writer), tea.WithWindowSize(120, 40))
done := make(chan error, 1)
go func() { _, err := program.Run(); done <- err }()
<-model.ready
program := startStreamingMotionProgram(t, model, tea.WithOutput(writer))
program.Send(tea.MouseMotionMsg{X: 40, Y: 25})
programAck(t, program)
hovered := programFrame(t, program)
Expand All @@ -148,7 +133,4 @@ func TestActualProgramHoverThenBottomReentryStaysBounded(t *testing.T) {
}
programAck(t, program)
require.LessOrEqual(t, writer.writes.Load()-beforeWrites, uint64(30), "follow-tail renderer work remains bounded by events")
program.Quit()
require.NoError(t, <-done)
root.ar.Stop()
}
11 changes: 1 addition & 10 deletions pkg/tui/tui_noop_input_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,7 @@ func TestActualProgramWritesAfterEffectiveIdleInput(t *testing.T) {
root := populateScrollableRoot(t)
model := &cacheProgramModel{root: root}
writer := &cacheProgramWriter{}
program := tea.NewProgram(model, tea.WithInput(nil), tea.WithOutput(writer), tea.WithWindowSize(120, 40))
done := make(chan error, 1)
go func() {
_, err := program.Run()
done <- err
}()
program := startTestProgram(t, root, model, tea.WithOutput(writer))
require.Eventually(t, func() bool { return writer.snapshot() != "" }, time.Second, time.Millisecond)
ack := func() {
done := make(chan struct{})
Expand All @@ -154,10 +149,6 @@ func TestActualProgramWritesAfterEffectiveIdleInput(t *testing.T) {
require.Eventually(t, func() bool {
return strings.Contains(ansi.Strip(writer.snapshot()), "PROGRAM-STREAM-MARKER")
}, time.Second, time.Millisecond, "stream chunk produced no terminal write while idle")

program.Quit()
require.NoError(t, <-done)
root.ar.Stop()
}

func TestNoOpPointerAndWheelReuseRootCache(t *testing.T) {
Expand Down
38 changes: 38 additions & 0 deletions pkg/tui/tui_perf_harness_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,13 @@ import (
"testing"
"time"

tea "charm.land/bubbletea/v2"

"github.com/docker/docker-agent/pkg/app"
chatmsg "github.com/docker/docker-agent/pkg/chat"
"github.com/docker/docker-agent/pkg/session"
"github.com/docker/docker-agent/pkg/tools"
"github.com/docker/docker-agent/pkg/tui/animation"
"github.com/docker/docker-agent/pkg/tui/components/spinner"
"github.com/docker/docker-agent/pkg/tui/page/chat"
"github.com/docker/docker-agent/pkg/tui/service"
Expand Down Expand Up @@ -53,7 +56,39 @@ func mixedHistorySession(count int) (*session.Session, int, int) {
return &session.Session{ID: "profile", Title: "profile", Messages: items}, count * 1000, totalBytes
}

// wallClockRoot builds the harness root on the production wall-clock
// animation runtime. Use it for perf and geometry tests that measure
// wall-clock time or compare widths and counts rather than exact frames.
func wallClockRoot(tb testing.TB, width, height int) (*appModel, time.Duration, runtime.MemStats) {
tb.Helper()
return harnessRoot(tb, width, height, nil)
}

// frozenScheduler is an animation.Scheduler whose clock never advances and
// whose Tick never schedules a message. A runtime built on it therefore never
// accepts a TickMsg, so ar.Now() (and every FrameIndexAt-driven glyph) stays
// constant for the whole test.
type frozenScheduler struct{}

func (frozenScheduler) Now() time.Time { return time.Unix(1, 0) }
func (frozenScheduler) Tick(time.Duration, func(time.Time) tea.Msg) tea.Cmd { return nil }

// frozenClockRoot builds the harness root on a frozen animation runtime so
// that tests asserting exact frame equality across a message round trip cannot
// be broken by a spinner tick landing between the two frames (which happens
// readily under -race, where the loop is slow enough to straddle TickRate).
func frozenClockRoot(tb testing.TB, width, height int) (*appModel, time.Duration, runtime.MemStats) {
tb.Helper()
return harnessRoot(tb, width, height, animation.NewRuntimeWithScheduler(frozenScheduler{}))
}

// harnessRoot is the shared body of wallClockRoot and frozenClockRoot. When ar
// is non-nil it replaces the runtime created by New before the spinner and
// chat page are built, so both share it. The tab bar constructed inside New
// keeps the wall runtime, but it only reads ar.Now() and never schedules a
// tick itself (the harness discards Init()'s commands and EnsureRunning goes
// through m.ar), so its clock stays at zero.
func harnessRoot(tb testing.TB, width, height int, ar *animation.Runtime) (*appModel, time.Duration, runtime.MemStats) {
tb.Helper()
if setter, ok := tb.(interface{ Setenv(key, value string) }); ok {
home := tb.TempDir()
Expand All @@ -64,6 +99,9 @@ func wallClockRoot(tb testing.TB, width, height int) (*appModel, time.Duration,
sess := &session.Session{ID: "profile", Title: "profile"}
a := app.New(tb.Context(), stubRuntime{}, sess)
m := New(tb.Context(), nil, a, "", func() {}, WithHideSidebar()).(*appModel)
if ar != nil {
m.ar = ar
}
if cleaner, ok := tb.(interface{ Cleanup(f func()) }); ok {
cleaner.Cleanup(m.cleanupManagedResources)
}
Expand Down
43 changes: 43 additions & 0 deletions pkg/tui/tui_program_harness_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package tui

import (
"testing"
"time"

tea "charm.land/bubbletea/v2"
"github.com/stretchr/testify/assert"
)

// startTestProgram runs a bubbletea program around root in the background and
// guarantees teardown even when an assertion aborts the test: Quit, wait for
// Run to return, then stop the animation runtime. Without this a failed
// require leaks a live render loop whose View() keeps reading the styles
// package globals while later tests call styles.ApplyTheme, which the race
// detector reports against unrelated tests.
func startTestProgram(t *testing.T, root *appModel, model tea.Model, opts ...tea.ProgramOption) *tea.Program {
t.Helper()
program := tea.NewProgram(model, append([]tea.ProgramOption{tea.WithInput(nil), tea.WithWindowSize(120, 40)}, opts...)...)
done := make(chan error, 1)
go func() { _, err := program.Run(); done <- err }()
t.Cleanup(func() {
program.Quit()
select {
case err := <-done:
assert.NoError(t, err, "program run")
case <-time.After(10 * time.Second):
program.Kill()
t.Error("program did not exit within 10s of Quit")
}
root.ar.Stop()
})
return program
}

// startStreamingMotionProgram is startTestProgram for a streamingMotionModel;
// it additionally blocks until the model has rendered its first frame.
func startStreamingMotionProgram(t *testing.T, model *streamingMotionModel, opts ...tea.ProgramOption) *tea.Program {
t.Helper()
program := startTestProgram(t, model.root, model, opts...)
<-model.ready
return program
}
10 changes: 2 additions & 8 deletions pkg/tui/tui_scroll_cancel_matrix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
)

func TestActualProgramScrollCancelResizeMatrixNeverNeedsRecoveryClick(t *testing.T) {
root, _, _ := wallClockRoot(t, 120, 40)
root, _, _ := frozenClockRoot(t, 120, 40)
_, _ = root.Update(messages.RoutedMsg{SessionID: "profile", Inner: agentruntime.StreamStarted("profile", "root")})
_, _ = root.Update(messages.RoutedMsg{SessionID: "profile", Inner: agentruntime.AgentChoiceReasoning("root", "profile", "thinking prefix λ界\n\n")})
chunk := "matrix marker **bold** `code` λ界 [link](https://example.com)\n\n"
Expand All @@ -22,10 +22,7 @@ func TestActualProgramScrollCancelResizeMatrixNeverNeedsRecoveryClick(t *testing
_ = root.View()
}
model := &streamingMotionModel{root: root, ready: make(chan struct{})}
program := tea.NewProgram(model, tea.WithInput(nil), tea.WithOutput(&wallClockCountingWriter{}), tea.WithWindowSize(120, 40))
done := make(chan error, 1)
go func() { _, err := program.Run(); done <- err }()
<-model.ready
program := startStreamingMotionProgram(t, model, tea.WithOutput(&wallClockCountingWriter{}))
sequence := []tea.Msg{
messages.WheelCoalescedMsg{Delta: -1_000_000, X: 40, Y: 20},
tea.MouseMotionMsg{X: 40, Y: 20},
Expand All @@ -47,7 +44,4 @@ func TestActualProgramScrollCancelResizeMatrixNeverNeedsRecoveryClick(t *testing
require.Equal(t, ansi.Strip(current), ansi.Strip(programFrame(t, program)), "inert click repaired viewport after %T", msg)
}
require.Contains(t, ansi.Strip(programFrame(t, program)), "matrix marker", "bottom viewport retains current stream content")
program.Quit()
require.NoError(t, <-done)
root.ar.Stop()
}
Loading
Loading