Skip to content

Commit cf8cc61

Browse files
aheritieraheritier
authored
Merge pull request #4627 from dgageot/worktree-board-89b5da612b6a952c
test: speed up fixtures and isolate flaky test dependencies
2 parents 71b914a + 5c48efe commit cf8cc61

51 files changed

Lines changed: 472 additions & 951 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎cmd/root/agent_picker_test.go‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ func TestAgentPickerWindowing(t *testing.T) {
337337
assert.NotContains(t, out, "agent-0")
338338

339339
// Hit-testing maps points to absolute indices within the window.
340-
x, y := firstCardPoint(t, m, 4)
340+
x, y := renderedCardRefPoint(t, m, 4)
341341
i, ok := m.cardAt(x, y)
342342
assert.True(t, ok)
343343
assert.Equal(t, 4, i)
@@ -433,7 +433,7 @@ func TestAgentPickerMouseHoverSelects(t *testing.T) {
433433
m.width = 120
434434
m.height = 40
435435

436-
x, y := firstCardPoint(t, m, 1)
436+
x, y := renderedCardRefPoint(t, m, 1)
437437
_, _ = m.Update(tea.MouseMotionMsg{X: x, Y: y})
438438
assert.Equal(t, 1, m.cursor, "hover should move the cursor to the hovered card")
439439
}
@@ -448,7 +448,7 @@ func TestAgentPickerDoubleClickSelects(t *testing.T) {
448448
m.width = 120
449449
m.height = 40
450450

451-
x, y := firstCardPoint(t, m, 1)
451+
x, y := renderedCardRefPoint(t, m, 1)
452452
click := tea.MouseClickMsg{X: x, Y: y, Button: tea.MouseLeft}
453453

454454
// First click selects (moves cursor) but does not quit.
@@ -469,7 +469,7 @@ func TestAgentPickerDoubleClickResetsAfterTimeout(t *testing.T) {
469469
m.width = 120
470470
m.height = 40
471471

472-
x, y := firstCardPoint(t, m, 0)
472+
x, y := renderedCardRefPoint(t, m, 0)
473473
click := tea.MouseClickMsg{X: x, Y: y, Button: tea.MouseLeft}
474474

475475
_, cmd := m.Update(click)
@@ -577,7 +577,7 @@ func TestAgentPickerDetailsResetsClickTracking(t *testing.T) {
577577
m.width = 120
578578
m.height = 40
579579

580-
x, y := firstCardPoint(t, m, 0)
580+
x, y := renderedCardRefPoint(t, m, 0)
581581
click := tea.MouseClickMsg{X: x, Y: y, Button: tea.MouseLeft}
582582

583583
// First click primes double-click tracking.
@@ -811,16 +811,25 @@ func TestAgentPickerPanelSizeMatchesRenderNarrow(t *testing.T) {
811811
}
812812
}
813813

814-
// firstCardPoint scans the grid for a coordinate that maps to card index want.
815-
func firstCardPoint(t *testing.T, m *agentPickerModel, want int) (int, int) {
814+
// renderedCardRefPoint locates fixture text on the rendered screen, independently of hit testing.
815+
func renderedCardRefPoint(t *testing.T, m *agentPickerModel, want int) (int, int) {
816816
t.Helper()
817-
for y := range m.height {
818-
for x := range m.width {
819-
if i, ok := m.cardAt(x, y); ok && i == want {
820-
return x, y
821-
}
817+
require.GreaterOrEqual(t, want, 0)
818+
require.Less(t, want, len(m.choices))
819+
ref := displayRef(m.choices[want].ref)
820+
require.NotEmpty(t, ref)
821+
screen := ansi.Strip(lipgloss.Place(m.width, m.height, lipgloss.Center, lipgloss.Center, m.render()))
822+
for y, line := range strings.Split(screen, "\n") {
823+
offset := strings.Index(line, ref)
824+
if offset < 0 {
825+
continue
822826
}
827+
x := lipgloss.Width(line[:offset])
828+
index, ok := m.cardAt(x, y)
829+
require.True(t, ok, "rendered card %d must be clickable", want)
830+
require.Equal(t, want, index)
831+
return x, y
823832
}
824-
t.Fatalf("no coordinate maps to card %d", want)
833+
t.Fatalf("card %d (%q) is not visible", want, ref)
825834
return 0, 0
826835
}

‎cmd/root/debug_tool_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/docker/docker-agent/pkg/agent"
2727
"github.com/docker/docker-agent/pkg/environment"
2828
"github.com/docker/docker-agent/pkg/hooks"
29+
"github.com/docker/docker-agent/pkg/modelsdev"
2930
"github.com/docker/docker-agent/pkg/tools"
3031
"github.com/docker/docker-agent/pkg/tools/builtin/filesystem"
3132
)
@@ -73,6 +74,9 @@ func runDebugToolConfigOutputs(t *testing.T, flags *debugFlags, cfg string, args
7374
require.NoError(t, os.WriteFile(filepath.Join(dir, "hello.txt"), []byte("hello from a tool"), 0o600))
7475
flags.runConfig.WorkingDir = dir
7576
flags.runConfig.EnvProviderOverride = environment.NewMapEnvProvider(map[string]string{"OPENAI_API_KEY": "test-key"})
77+
if flags.runConfig.ModelsDevStoreOverride == nil {
78+
flags.runConfig.ModelsDevStoreOverride = modelsdev.NewDatabaseStore(modelsdev.EmbeddedSnapshot())
79+
}
7680

7781
var out, warnings bytes.Buffer
7882
cmd := &cobra.Command{}
@@ -383,6 +387,8 @@ func TestDebugToolsetsCommand_IncludesDeferredTools(t *testing.T) {
383387
require.NoError(t, os.WriteFile(path, []byte(debugToolConfig), 0o600))
384388
flags := &debugFlags{toolsetsJSON: true}
385389
flags.runConfig.EnvProviderOverride = environment.NewMapEnvProvider(map[string]string{"OPENAI_API_KEY": "test-key"})
390+
flags.runConfig.ModelsDevStoreOverride = modelsdev.NewDatabaseStore(modelsdev.EmbeddedSnapshot())
391+
386392
var out bytes.Buffer
387393
cmd := &cobra.Command{}
388394
cmd.SetContext(t.Context())
@@ -1072,3 +1078,24 @@ func TestTransformDebugToolInput_RedactionPreservesNestedNumbers(t *testing.T) {
10721078
assert.Equal(t, int64(9007199254740993), args.Record.Value)
10731079
assert.Equal(t, portcullis.Marker, args.Record.Secret)
10741080
}
1081+
1082+
func TestDebugToolUsesEmbeddedCatalog(t *testing.T) {
1083+
t.Parallel()
1084+
flags := &debugFlags{}
1085+
_, err := runDebugTool(t, flags, "think")
1086+
require.NoError(t, err)
1087+
require.NotNil(t, flags.runConfig.ModelsDevStoreOverride)
1088+
db, err := flags.runConfig.ModelsDevStoreOverride.GetDatabase(t.Context())
1089+
require.NoError(t, err)
1090+
assert.Same(t, modelsdev.EmbeddedSnapshot(), db)
1091+
}
1092+
1093+
func TestDebugToolPreservesCatalogOverride(t *testing.T) {
1094+
t.Parallel()
1095+
store := modelsdev.NewDatabaseStore(&modelsdev.Database{})
1096+
flags := &debugFlags{}
1097+
flags.runConfig.ModelsDevStoreOverride = store
1098+
_, err := runDebugTool(t, flags, "think")
1099+
require.NoError(t, err)
1100+
assert.Same(t, store, flags.runConfig.ModelsDevStoreOverride)
1101+
}

‎cmd/root/dmr_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func TestDMRDockerConnection(t *testing.T) {
8787
require.NoError(t, err)
8888
t.Setenv("DMR_DOCKER_HELPER", "1")
8989
script := "#!/bin/sh\nexec \"" + testBinary + "\" -test.run=^TestDMRDockerHelper$ -- \"$@\"\n"
90-
require.NoError(t, os.WriteFile(filepath.Join(shimDir, "docker"), []byte(script), 0o755))
90+
writeMockCLI(t, filepath.Join(shimDir, "docker"), script)
9191
t.Setenv("PATH", shimDir+string(os.PathListSeparator)+os.Getenv("PATH"))
9292

9393
relativeConfig, err := filepath.Rel(wd, configDir)
@@ -224,7 +224,7 @@ func TestDMRDockerConnectionErrorNamesEngine(t *testing.T) {
224224
t.Skip("uses a shell Docker shim")
225225
}
226226
dir := t.TempDir()
227-
require.NoError(t, os.WriteFile(filepath.Join(dir, "docker"), []byte("#!/bin/sh\necho 'missing engine' >&2\nexit 1\n"), 0o755))
227+
writeMockCLI(t, filepath.Join(dir, "docker"), "#!/bin/sh\necho 'missing engine' >&2\nexit 1\n")
228228
t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
229229
opts := cliflags.NewClientOptions()
230230
flags := pflag.NewFlagSet("docker", pflag.ContinueOnError)
@@ -245,7 +245,7 @@ func TestDMRSelectedDockerStatusFailureDoesNotFallBack(t *testing.T) {
245245
t.Skip("uses a shell Docker shim")
246246
}
247247
dir := t.TempDir()
248-
require.NoError(t, os.WriteFile(filepath.Join(dir, "docker"), []byte("#!/bin/sh\necho 'selected engine unavailable' >&2\nexit 1\n"), 0o755))
248+
writeMockCLI(t, filepath.Join(dir, "docker"), "#!/bin/sh\necho 'selected engine unavailable' >&2\nexit 1\n")
249249
t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
250250
t.Setenv("MODEL_RUNNER_HOST", "")
251251
opts := cliflags.NewClientOptions()
@@ -308,7 +308,7 @@ func TestDMRDockerTunnelCancellation(t *testing.T) {
308308
t.Setenv("MODEL_RUNNER_HOST", "")
309309
dir := t.TempDir()
310310
// Read forever without answering HTTP, as a stuck engine/TLS handshake would.
311-
require.NoError(t, os.WriteFile(filepath.Join(dir, "docker"), []byte("#!/bin/sh\nwhile read line; do :; done\n"), 0o755))
311+
writeMockCLI(t, filepath.Join(dir, "docker"), "#!/bin/sh\nwhile read line; do :; done\n")
312312
t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
313313
ctx, err := withDMRDockerConnection(t.Context(), nil, nil)
314314
require.NoError(t, err)

‎cmd/root/evaluator_preflight_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func TestRunInSandboxForwardsInheritedEvaluatorCredentials(t *testing.T) {
104104
t.Setenv("EVALUATOR_TEST_EXEC_ARGS", argsFile)
105105
t.Setenv("EVALUATOR_TEST_EXEC_ENV", envFile)
106106
t.Setenv("PATH", fakeDir)
107-
require.NoError(t, os.WriteFile(filepath.Join(fakeDir, "docker"), []byte(`#!/bin/sh
107+
writeMockCLI(t, filepath.Join(fakeDir, "docker"), `#!/bin/sh
108108
set -eu
109109
case "$1 $2" in
110110
"sandbox version"|"sandbox policy"|"sandbox create") ;;
@@ -115,7 +115,7 @@ case "$1 $2" in
115115
;;
116116
*) printf 'unexpected Docker invocation: %s\n' "$*" >&2; exit 99 ;;
117117
esac
118-
`), 0o700))
118+
`)
119119

120120
rc := &config.RuntimeConfig{Config: config.Config{WorkingDir: workspace}}
121121
require.Nil(t, rc.GlobalHooks, "sandbox dispatch happens before runOrExec loads hooks")

‎cmd/root/mock_script_test.go‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package root
2+
3+
import (
4+
"os"
5+
"os/exec"
6+
"path/filepath"
7+
"runtime"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
// Capture the fixture path before tests change the process working directory.
15+
var mockCLILauncher, mockCLILauncherErr = filepath.Abs("testdata/cli")
16+
17+
func writeMockCLI(t *testing.T, path, script string) {
18+
t.Helper()
19+
require.NoError(t, mockCLILauncherErr)
20+
require.NoError(t, os.WriteFile(path+".script", []byte(script), 0o600))
21+
require.NoError(t, os.Symlink(mockCLILauncher, path))
22+
}
23+
24+
func TestMockCLI(t *testing.T) {
25+
if runtime.GOOS == "windows" {
26+
t.Skip("POSIX shell fixture")
27+
}
28+
dir := filepath.Join(t.TempDir(), "space ' $dir")
29+
require.NoError(t, os.Mkdir(dir, 0o700))
30+
first := filepath.Join(dir, "docker")
31+
second := filepath.Join(dir, "sbx")
32+
writeMockCLI(t, first, `printf '<%s>\n' "$@"; printf diagnostic >&2; exit 7`)
33+
writeMockCLI(t, second, `exec /bin/sh -c 'printf alternate; exit 9'`)
34+
a, err := os.Stat(first)
35+
require.NoError(t, err)
36+
b, err := os.Stat(second)
37+
require.NoError(t, err)
38+
assert.True(t, os.SameFile(a, b))
39+
t.Chdir(t.TempDir())
40+
cmd := exec.CommandContext(t.Context(), first, "", "two words", "$(echo literal)")
41+
cmd.Env = []string{"PATH=" + dir}
42+
out, err := cmd.CombinedOutput()
43+
var exitErr *exec.ExitError
44+
require.ErrorAs(t, err, &exitErr)
45+
assert.Equal(t, 7, exitErr.ExitCode())
46+
assert.Equal(t, "<>\n<two words>\n<$(echo literal)>\ndiagnostic", string(out))
47+
cmd = exec.CommandContext(t.Context(), second)
48+
cmd.Env = []string{"PATH=" + dir}
49+
out, err = cmd.Output()
50+
require.ErrorAs(t, err, &exitErr)
51+
assert.Equal(t, 9, exitErr.ExitCode())
52+
assert.Equal(t, "alternate", string(out))
53+
}

‎cmd/root/sandbox_cancel_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func TestCloudCancellationCleansOwnedSandbox(t *testing.T) {
3333
require.NoError(t, err)
3434
fakeDir := t.TempDir()
3535
script := fmt.Sprintf("#!/bin/sh\nexec %q -test.run=^TestCloudCancellationHelper$ -- \"$@\"\n", executable)
36-
require.NoError(t, os.WriteFile(filepath.Join(fakeDir, "sbx"), []byte(script), 0o700))
36+
writeMockCLI(t, filepath.Join(fakeDir, "sbx"), script)
3737
t.Setenv("PATH", fakeDir)
3838
t.Setenv("SANDBOX_CANCEL_HELPER", "1")
3939
for _, phase := range []string{"create-with-id", "create-without-id", "exec"} {

‎cmd/root/sandbox_cloud_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ case "$2" in
7676
*) exit 99 ;;
7777
esac
7878
`, log)
79-
require.NoError(t, os.WriteFile(filepath.Join(fakeDir, "sbx"), []byte(script), 0o700))
79+
writeMockCLI(t, filepath.Join(fakeDir, "sbx"), script)
8080
for _, tc := range []struct {
8181
name string
8282
execExit, stopExit string
@@ -209,7 +209,7 @@ func TestCloudRejectsInvalidUserConfigBeforeProvisioning(t *testing.T) {
209209
`), 0o600))
210210
fakeDir := t.TempDir()
211211
calls := filepath.Join(fakeDir, "calls")
212-
require.NoError(t, os.WriteFile(filepath.Join(fakeDir, "sbx"), fmt.Appendf(nil, "#!/bin/sh\nprintf '%%s\\n' \"$*\" >> %q\n", calls), 0o700))
212+
writeMockCLI(t, filepath.Join(fakeDir, "sbx"), fmt.Sprintf("#!/bin/sh\nprintf '%%s\\n' \"$*\" >> %q\n", calls))
213213
t.Setenv("PATH", fakeDir)
214214
t.Setenv("SANDBOX_VM_ID", "")
215215
cmd := newRunCmd()
@@ -326,7 +326,7 @@ case "$1 $2" in
326326
*) exit 99 ;;
327327
esac
328328
`, calls, newConfig, configPath)
329-
require.NoError(t, os.WriteFile(filepath.Join(fakeDir, "docker"), []byte(script), 0o700))
329+
writeMockCLI(t, filepath.Join(fakeDir, "docker"), script)
330330
t.Setenv("PATH", fakeDir)
331331
cmd := newRunCmd()
332332
cmd.SetOut(io.Discard)

‎cmd/root/sandbox_data_dir_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ case "$1 $2" in
138138
*) exit 99 ;;
139139
esac
140140
`, calls)
141-
require.NoError(t, os.WriteFile(filepath.Join(fakeDir, "docker"), []byte(script), 0o700))
141+
writeMockCLI(t, filepath.Join(fakeDir, "docker"), script)
142142
t.Setenv("PATH", fakeDir)
143143
logger := slog.Default()
144144
t.Cleanup(func() {

‎cmd/root/testdata/cli‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
#!/bin/sh
2+
. "$0.script"

‎docs/community/contributing/index.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,9 @@ unchanged during analysis; file-cop coverage still includes inactive files.
148148

149149
`CancellationLoop`, `ClosedChannelLoop`, `ChannelStateCheck`,
150150
`CancellationSafeResultSend`, `NonblockingChannelReplacement`, and `SlicesConcat`
151-
use the upstream implementations and synthetic tests. Their frozen-config
152-
exclusions, registration tests, and application regressions remain here.
151+
use the upstream implementations and tests. Shared-cop behavior and version
152+
compatibility are tested in rubocop-go; only project-owned rules, exclusions,
153+
and registration checks are tested here.
153154

154155
`Lint/ChannelDirection` suggests directional raw-channel parameters and results.
155156
It follows requirements through locals, fields, direct calls, returns, and closure

0 commit comments

Comments
 (0)