diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index a9ce12b892602..b956e17d5efaa 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -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) + } }) }