fix(agent/agentcontainers): fix TestDevcontainerDiscovery/AutoStart flake (#19179)

Fixes https://github.com/coder/internal/issues/864
This commit is contained in:
Danielle Maywood
2025-08-05 13:58:55 +01:00
committed by GitHub
parent 8b66a5ad59
commit 760dc8b467
+107 -147
View File
@@ -71,6 +71,7 @@ func (f *fakeContainerCLI) ExecAs(ctx context.Context, name, user string, args .
// fakeDevcontainerCLI implements the agentcontainers.DevcontainerCLI
// interface for testing.
type fakeDevcontainerCLI struct {
up func(workspaceFolder, configPath string) (string, error)
upID string
upErr error
upErrC chan func() error // If set, send to return err, close to return upErr.
@@ -79,9 +80,14 @@ type fakeDevcontainerCLI struct {
readConfig agentcontainers.DevcontainerConfig
readConfigErr error
readConfigErrC chan func(envs []string) error
configMap map[string]agentcontainers.DevcontainerConfig // By config path
}
func (f *fakeDevcontainerCLI) Up(ctx context.Context, _, _ string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) {
func (f *fakeDevcontainerCLI) Up(ctx context.Context, workspaceFolder, configPath string, _ ...agentcontainers.DevcontainerCLIUpOptions) (string, error) {
if f.up != nil {
return f.up(workspaceFolder, configPath)
}
if f.upErrC != nil {
select {
case <-ctx.Done():
@@ -109,7 +115,12 @@ func (f *fakeDevcontainerCLI) Exec(ctx context.Context, _, _ string, cmd string,
return f.execErr
}
func (f *fakeDevcontainerCLI) ReadConfig(ctx context.Context, _, _ string, envs []string, _ ...agentcontainers.DevcontainerCLIReadConfigOptions) (agentcontainers.DevcontainerConfig, error) {
func (f *fakeDevcontainerCLI) ReadConfig(ctx context.Context, _, configPath string, envs []string, _ ...agentcontainers.DevcontainerCLIReadConfigOptions) (agentcontainers.DevcontainerConfig, error) {
if f.configMap != nil {
if v, found := f.configMap[configPath]; found {
return v, f.readConfigErr
}
}
if f.readConfigErrC != nil {
select {
case <-ctx.Done():
@@ -3576,193 +3587,112 @@ func TestDevcontainerDiscovery(t *testing.T) {
name string
agentDir string
fs map[string]string
configMap map[string]agentcontainers.DevcontainerConfig
expectDevcontainerCount int
setupMocks func(mDCCLI *acmock.MockDevcontainerCLI)
expectUpCalledCount int
}{
{
name: "SingleEnabled",
agentDir: "/home/coder",
expectDevcontainerCount: 1,
expectUpCalledCount: 1,
fs: map[string]string{
"/home/coder/.git/HEAD": "",
"/home/coder/.devcontainer/devcontainer.json": "",
},
setupMocks: func(mDCCLI *acmock.MockDevcontainerCLI) {
gomock.InOrder(
// Given: This dev container has auto start enabled.
mDCCLI.EXPECT().ReadConfig(gomock.Any(),
"/home/coder",
"/home/coder/.devcontainer/devcontainer.json",
[]string{},
).Return(agentcontainers.DevcontainerConfig{
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: true,
},
configMap: map[string]agentcontainers.DevcontainerConfig{
"/home/coder/.devcontainer/devcontainer.json": {
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: true,
},
},
}, nil),
// Then: We expect it to be started.
mDCCLI.EXPECT().Up(gomock.Any(),
"/home/coder",
"/home/coder/.devcontainer/devcontainer.json",
gomock.Any(),
).Return("", nil),
)
},
},
},
},
{
name: "SingleDisabled",
agentDir: "/home/coder",
expectDevcontainerCount: 1,
expectUpCalledCount: 0,
fs: map[string]string{
"/home/coder/.git/HEAD": "",
"/home/coder/.devcontainer/devcontainer.json": "",
},
setupMocks: func(mDCCLI *acmock.MockDevcontainerCLI) {
gomock.InOrder(
// Given: This dev container has auto start disabled.
mDCCLI.EXPECT().ReadConfig(gomock.Any(),
"/home/coder",
"/home/coder/.devcontainer/devcontainer.json",
[]string{},
).Return(agentcontainers.DevcontainerConfig{
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: false,
},
configMap: map[string]agentcontainers.DevcontainerConfig{
"/home/coder/.devcontainer/devcontainer.json": {
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: false,
},
},
}, nil),
// Then: We expect it to _not_ be started.
mDCCLI.EXPECT().Up(gomock.Any(),
"/home/coder",
"/home/coder/.devcontainer/devcontainer.json",
gomock.Any(),
).Return("", nil).Times(0),
)
},
},
},
},
{
name: "OneEnabledOneDisabled",
agentDir: "/home/coder",
expectDevcontainerCount: 2,
expectUpCalledCount: 1,
fs: map[string]string{
"/home/coder/.git/HEAD": "",
"/home/coder/.devcontainer/devcontainer.json": "",
"/home/coder/project/.devcontainer.json": "",
},
setupMocks: func(mDCCLI *acmock.MockDevcontainerCLI) {
gomock.InOrder(
// Given: This dev container has auto start enabled.
mDCCLI.EXPECT().ReadConfig(gomock.Any(),
"/home/coder",
"/home/coder/.devcontainer/devcontainer.json",
[]string{},
).Return(agentcontainers.DevcontainerConfig{
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: true,
},
configMap: map[string]agentcontainers.DevcontainerConfig{
"/home/coder/.devcontainer/devcontainer.json": {
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: true,
},
},
}, nil),
// Then: We expect it to be started.
mDCCLI.EXPECT().Up(gomock.Any(),
"/home/coder",
"/home/coder/.devcontainer/devcontainer.json",
gomock.Any(),
).Return("", nil),
)
gomock.InOrder(
// Given: This dev container has auto start disabled.
mDCCLI.EXPECT().ReadConfig(gomock.Any(),
"/home/coder/project",
"/home/coder/project/.devcontainer.json",
[]string{},
).Return(agentcontainers.DevcontainerConfig{
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: false,
},
},
},
"/home/coder/project/.devcontainer.json": {
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: false,
},
},
}, nil),
// Then: We expect it to _not_ be started.
mDCCLI.EXPECT().Up(gomock.Any(),
"/home/coder/project",
"/home/coder/project/.devcontainer.json",
gomock.Any(),
).Return("", nil).Times(0),
)
},
},
},
},
{
name: "MultipleEnabled",
agentDir: "/home/coder",
expectDevcontainerCount: 2,
expectUpCalledCount: 2,
fs: map[string]string{
"/home/coder/.git/HEAD": "",
"/home/coder/.devcontainer/devcontainer.json": "",
"/home/coder/project/.devcontainer.json": "",
},
setupMocks: func(mDCCLI *acmock.MockDevcontainerCLI) {
gomock.InOrder(
// Given: This dev container has auto start enabled.
mDCCLI.EXPECT().ReadConfig(gomock.Any(),
"/home/coder",
"/home/coder/.devcontainer/devcontainer.json",
[]string{},
).Return(agentcontainers.DevcontainerConfig{
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: true,
},
configMap: map[string]agentcontainers.DevcontainerConfig{
"/home/coder/.devcontainer/devcontainer.json": {
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: true,
},
},
}, nil),
// Then: We expect it to be started.
mDCCLI.EXPECT().Up(gomock.Any(),
"/home/coder",
"/home/coder/.devcontainer/devcontainer.json",
gomock.Any(),
).Return("", nil),
)
gomock.InOrder(
// Given: This dev container has auto start enabled.
mDCCLI.EXPECT().ReadConfig(gomock.Any(),
"/home/coder/project",
"/home/coder/project/.devcontainer.json",
[]string{},
).Return(agentcontainers.DevcontainerConfig{
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: true,
},
},
},
"/home/coder/project/.devcontainer.json": {
Configuration: agentcontainers.DevcontainerConfiguration{
Customizations: agentcontainers.DevcontainerCustomizations{
Coder: agentcontainers.CoderCustomization{
AutoStart: true,
},
},
}, nil),
// Then: We expect it to be started.
mDCCLI.EXPECT().Up(gomock.Any(),
"/home/coder/project",
"/home/coder/project/.devcontainer.json",
gomock.Any(),
).Return("", nil),
)
},
},
},
},
}
@@ -3775,43 +3705,73 @@ func TestDevcontainerDiscovery(t *testing.T) {
ctx = testutil.Context(t, testutil.WaitShort)
logger = testutil.Logger(t)
mClock = quartz.NewMock(t)
mDCCLI = acmock.NewMockDevcontainerCLI(gomock.NewController(t))
upCalledMu sync.Mutex
upCalledFor = map[string]bool{}
fCCLI = &fakeContainerCLI{}
fDCCLI = &fakeDevcontainerCLI{
configMap: tt.configMap,
up: func(_, configPath string) (string, error) {
upCalledMu.Lock()
upCalledFor[configPath] = true
upCalledMu.Unlock()
return "", nil
},
}
r = chi.NewRouter()
)
// Given: We setup our mocks. These mocks handle our expectations for these
// tests. If there are missing/unexpected mock calls, the test will fail.
tt.setupMocks(mDCCLI)
api := agentcontainers.NewAPI(logger,
agentcontainers.WithClock(mClock),
agentcontainers.WithWatcher(watcher.NewNoop()),
agentcontainers.WithFileSystem(initFS(t, tt.fs)),
agentcontainers.WithManifestInfo("owner", "workspace", "parent-agent", "/home/coder"),
agentcontainers.WithContainerCLI(&fakeContainerCLI{}),
agentcontainers.WithDevcontainerCLI(mDCCLI),
agentcontainers.WithContainerCLI(fCCLI),
agentcontainers.WithDevcontainerCLI(fDCCLI),
agentcontainers.WithProjectDiscovery(true),
agentcontainers.WithDiscoveryAutostart(true),
)
api.Start()
defer api.Close()
r.Mount("/", api.Routes())
// When: All expected dev containers have been found.
// Given: We allow the discover routing to progress
var got codersdk.WorkspaceAgentListContainersResponse
require.Eventuallyf(t, func() bool {
req := httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx)
rec := httptest.NewRecorder()
r.ServeHTTP(rec, req)
got := codersdk.WorkspaceAgentListContainersResponse{}
got = codersdk.WorkspaceAgentListContainersResponse{}
err := json.NewDecoder(rec.Body).Decode(&got)
require.NoError(t, err)
return len(got.Devcontainers) >= tt.expectDevcontainerCount
upCalledMu.Lock()
upCalledCount := len(upCalledFor)
upCalledMu.Unlock()
return len(got.Devcontainers) >= tt.expectDevcontainerCount && upCalledCount >= tt.expectUpCalledCount
}, testutil.WaitShort, testutil.IntervalFast, "dev containers never found")
// Then: We expect the mock infra to not fail.
// Close the API. We expect this not to fail because we should have finished
// at this point.
err := api.Close()
require.NoError(t, err)
// Then: We expect to find the expected devcontainers
assert.Len(t, got.Devcontainers, tt.expectDevcontainerCount)
// And: We expect `up` to have been called the expected amount of times.
assert.Len(t, upCalledFor, tt.expectUpCalledCount)
// And: `up` was called on the correct containers
for configPath, config := range tt.configMap {
autoStart := config.Configuration.Customizations.Coder.AutoStart
wasUpCalled := upCalledFor[configPath]
require.Equal(t, autoStart, wasUpCalled)
}
})
}