From a40a41a1a9b23613d36961437177550106c0ed97 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 27 May 2022 22:17:42 +0100 Subject: [PATCH 1/5] chore: executor_test: break out some test helpers, attempt to speed up test execution --- .../executor/lifecycle_executor_test.go | 133 ++++++++---------- 1 file changed, 59 insertions(+), 74 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index 94f8152208486..01dcde1cb2b68 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/codersdk" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -49,12 +50,8 @@ func TestExecutorAutostartOK(t *testing.T) { close(tickCh) }() - // Then: the workspace should be started - <-time.After(5 * time.Second) - ws := mustWorkspace(t, client, workspace.ID) - require.NotEqual(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected a workspace build to occur") - require.Equal(t, codersdk.ProvisionerJobSucceeded, ws.LatestBuild.Job.Status, "expected provisioner job to have succeeded") - require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected latest transition to be start") + // Then: the workspace should eventually be started + assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) } func TestExecutorAutostartTemplateUpdated(t *testing.T) { @@ -99,11 +96,8 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { }() // Then: the workspace should be started using the previous template version, and not the updated version. - <-time.After(5 * time.Second) + assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) ws := mustWorkspace(t, client, workspace.ID) - require.NotEqual(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected a workspace build to occur") - require.Equal(t, codersdk.ProvisionerJobSucceeded, ws.LatestBuild.Job.Status, "expected provisioner job to have succeeded") - require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected latest transition to be start") require.Equal(t, workspace.LatestBuild.TemplateVersionID, ws.LatestBuild.TemplateVersionID, "expected workspace build to be using the old template version") } @@ -139,10 +133,7 @@ func TestExecutorAutostartAlreadyRunning(t *testing.T) { }() // Then: the workspace should not be started. - <-time.After(5 * time.Second) - ws := mustWorkspace(t, client, workspace.ID) - require.Equal(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected no further workspace builds to occur") - require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected workspace to be running") + assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) } func TestExecutorAutostartNotEnabled(t *testing.T) { @@ -173,10 +164,7 @@ func TestExecutorAutostartNotEnabled(t *testing.T) { }() // Then: the workspace should not be started. - <-time.After(5 * time.Second) - ws := mustWorkspace(t, client, workspace.ID) - require.Equal(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected no further workspace builds to occur") - require.NotEqual(t, database.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected workspace not to be running") + assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) } func TestExecutorAutostopOK(t *testing.T) { @@ -202,11 +190,7 @@ func TestExecutorAutostopOK(t *testing.T) { }() // Then: the workspace should be stopped - <-time.After(5 * time.Second) - ws := mustWorkspace(t, client, workspace.ID) - require.NotEqual(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected a workspace build to occur") - require.Equal(t, codersdk.ProvisionerJobSucceeded, ws.LatestBuild.Job.Status, "expected provisioner job to have succeeded") - require.Equal(t, codersdk.WorkspaceTransitionStop, ws.LatestBuild.Transition, "expected workspace not to be running") + assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) } func TestExecutorAutostopExtend(t *testing.T) { @@ -228,8 +212,9 @@ func TestExecutorAutostopExtend(t *testing.T) { require.NotZero(t, originalDeadline) // Given: we extend the workspace deadline + newDeadline := originalDeadline.Add(30 * time.Minute) err := client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{ - Deadline: originalDeadline.Add(30 * time.Minute), + Deadline: newDeadline, }) require.NoError(t, err, "extend workspace deadline") @@ -238,24 +223,17 @@ func TestExecutorAutostopExtend(t *testing.T) { tickCh <- originalDeadline.Add(time.Minute) }() - // Then: nothing should happen - <-time.After(5 * time.Second) - ws := mustWorkspace(t, client, workspace.ID) - require.Equal(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected no further workspace builds to occur") - require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected workspace to be running") + // Then: nothing should happen and the workspace should stay running + assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) // When: the autobuild executor ticks after the *new* deadline: go func() { - tickCh <- ws.LatestBuild.Deadline.Add(time.Minute) + tickCh <- newDeadline.Add(time.Minute) close(tickCh) }() // Then: the workspace should be stopped - <-time.After(5 * time.Second) - ws = mustWorkspace(t, client, workspace.ID) - require.NotEqual(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected a workspace build to occur") - require.Equal(t, codersdk.ProvisionerJobSucceeded, ws.LatestBuild.Job.Status, "expected provisioner job to have succeeded") - require.Equal(t, codersdk.WorkspaceTransitionStop, ws.LatestBuild.Transition, "expected workspace not to be running") + assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) } func TestExecutorAutostopAlreadyStopped(t *testing.T) { @@ -282,11 +260,8 @@ func TestExecutorAutostopAlreadyStopped(t *testing.T) { close(tickCh) }() - // Then: the workspace should not be stopped. - <-time.After(5 * time.Second) - ws := mustWorkspace(t, client, workspace.ID) - require.Equal(t, codersdk.WorkspaceTransitionStop, ws.LatestBuild.Transition, "expected workspace not to be running") - require.Equal(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected no further workspace builds to occur") + // Then: the workspace should remain stopped and no build should happen. + assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) } func TestExecutorAutostopNotEnabled(t *testing.T) { @@ -317,10 +292,7 @@ func TestExecutorAutostopNotEnabled(t *testing.T) { }() // Then: the workspace should not be stopped. - <-time.After(5 * time.Second) - ws := mustWorkspace(t, client, workspace.ID) - require.Equal(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected no further workspace builds to occur") - require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected workspace to be running") + assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) } func TestExecutorWorkspaceDeleted(t *testing.T) { @@ -355,10 +327,7 @@ func TestExecutorWorkspaceDeleted(t *testing.T) { }() // Then: nothing should happen - <-time.After(5 * time.Second) - ws := mustWorkspace(t, client, workspace.ID) - require.Equal(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected no further workspace builds to occur") - require.Equal(t, codersdk.WorkspaceTransitionDelete, ws.LatestBuild.Transition, "expected workspace to be deleted") + assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionDelete) } func TestExecutorWorkspaceAutostartTooEarly(t *testing.T) { @@ -394,10 +363,7 @@ func TestExecutorWorkspaceAutostartTooEarly(t *testing.T) { }() // Then: nothing should happen - <-time.After(5 * time.Second) - ws := mustWorkspace(t, client, workspace.ID) - require.Equal(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected no further workspace builds to occur") - require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected workspace to be running") + assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) } func TestExecutorWorkspaceAutostopBeforeDeadline(t *testing.T) { @@ -420,10 +386,7 @@ func TestExecutorWorkspaceAutostopBeforeDeadline(t *testing.T) { }() // Then: nothing should happen - <-time.After(5 * time.Second) - ws := mustWorkspace(t, client, workspace.ID) - require.Equal(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected no further workspace builds to occur") - require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected workspace to be running") + assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) } func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) { @@ -451,11 +414,7 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) { }() // Then: the workspace should still stop - sorry! - <-time.After(5 * time.Second) - ws := mustWorkspace(t, client, workspace.ID) - require.NotEqual(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected a workspace build to occur") - require.Equal(t, codersdk.ProvisionerJobSucceeded, ws.LatestBuild.Job.Status, "expected provisioner job to have succeeded") - require.Equal(t, codersdk.WorkspaceTransitionStop, ws.LatestBuild.Transition, "expected workspace not to be running") + assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) } func TestExecutorAutostartMultipleOK(t *testing.T) { @@ -492,23 +451,21 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { close(tickCh2) }() - // Then: the workspace should be started - <-time.After(5 * time.Second) + // Then: the workspace should eventually be started + assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) + ws := mustWorkspace(t, client, workspace.ID) - require.NotEqual(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected a workspace build to occur") - require.Equal(t, codersdk.ProvisionerJobSucceeded, ws.LatestBuild.Job.Status, "expected provisioner job to have succeeded") - require.Equal(t, codersdk.WorkspaceTransitionStart, ws.LatestBuild.Transition, "expected latest transition to be start") builds, err := client.WorkspaceBuilds(ctx, codersdk.WorkspaceBuildsRequest{WorkspaceID: ws.ID}) require.NoError(t, err, "fetch list of workspace builds from primary") // One build to start, one stop transition, and one autostart. No more. - require.Equal(t, codersdk.WorkspaceTransitionStart, builds[0].Transition) - require.Equal(t, codersdk.WorkspaceTransitionStop, builds[1].Transition) - require.Equal(t, codersdk.WorkspaceTransitionStart, builds[2].Transition) - require.Len(t, builds, 3, "unexpected number of builds for workspace from primary") - - // Builds are returned most recent first. - require.True(t, builds[0].CreatedAt.After(builds[1].CreatedAt)) - require.True(t, builds[1].CreatedAt.After(builds[2].CreatedAt)) + if assert.Len(t, builds, 3, "unexpected number of builds for workspace from primary") { + assert.Equal(t, codersdk.WorkspaceTransitionStart, builds[0].Transition) + assert.Equal(t, codersdk.WorkspaceTransitionStop, builds[1].Transition) + assert.Equal(t, codersdk.WorkspaceTransitionStart, builds[2].Transition) + // Builds are returned most recent first. + require.True(t, builds[0].CreatedAt.After(builds[1].CreatedAt)) + require.True(t, builds[1].CreatedAt.After(builds[2].CreatedAt)) + } } func mustProvisionWorkspace(t *testing.T, client *codersdk.Client, mut ...func(*codersdk.CreateWorkspaceRequest)) codersdk.Workspace { @@ -555,6 +512,34 @@ func mustWorkspace(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID) return ws } +func assertLatestTransitionWithEventualBuild(t *testing.T, client *codersdk.Client, workspace codersdk.Workspace, expected codersdk.WorkspaceTransition) { + assert.Eventually(t, func() bool { + ws := mustWorkspace(t, client, workspace.ID) + if ws.LatestBuild.ID == workspace.LatestBuild.ID { + return false + } + if ws.LatestBuild.Job.Status != codersdk.ProvisionerJobSucceeded { + return false + } + // At this point a build has happened. Is it what we want? + return assert.Equal(t, expected, ws.LatestBuild.Transition) + }, 5*time.Second, 25*time.Millisecond) +} + +func assertLatestTransitionWithNoEventualBuild(t *testing.T, client *codersdk.Client, workspace codersdk.Workspace, expected codersdk.WorkspaceTransition) { + assert.Never(t, func() bool { + ws := mustWorkspace(t, client, workspace.ID) + if ws.LatestBuild.ID == workspace.LatestBuild.ID { + return false + } + if ws.LatestBuild.Job.Status != codersdk.ProvisionerJobSucceeded { + return false + } + // At this point a build should not have happened. Is the state still as expected? + return assert.Equal(t, expected, ws.LatestBuild.Transition) + }, 5*time.Second, 25*time.Millisecond) +} + func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } From b0260d7dffe705ff197cc387cb9e70a82cc24c88 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 27 May 2022 22:23:43 +0100 Subject: [PATCH 2/5] reduce timeout --- coderd/autobuild/executor/lifecycle_executor_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index 01dcde1cb2b68..f75cb2e559ac4 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -503,6 +503,7 @@ func mustTransitionWorkspace(t *testing.T, client *codersdk.Client, workspaceID } func mustWorkspace(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID) codersdk.Workspace { + t.Helper() ctx := context.Background() ws, err := client.Workspace(ctx, workspaceID) if err != nil && strings.Contains(err.Error(), "status code 410") { @@ -513,6 +514,7 @@ func mustWorkspace(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID) } func assertLatestTransitionWithEventualBuild(t *testing.T, client *codersdk.Client, workspace codersdk.Workspace, expected codersdk.WorkspaceTransition) { + t.Helper() assert.Eventually(t, func() bool { ws := mustWorkspace(t, client, workspace.ID) if ws.LatestBuild.ID == workspace.LatestBuild.ID { @@ -523,10 +525,11 @@ func assertLatestTransitionWithEventualBuild(t *testing.T, client *codersdk.Clie } // At this point a build has happened. Is it what we want? return assert.Equal(t, expected, ws.LatestBuild.Transition) - }, 5*time.Second, 25*time.Millisecond) + }, 3*time.Second, 25*time.Millisecond) } func assertLatestTransitionWithNoEventualBuild(t *testing.T, client *codersdk.Client, workspace codersdk.Workspace, expected codersdk.WorkspaceTransition) { + t.Helper() assert.Never(t, func() bool { ws := mustWorkspace(t, client, workspace.ID) if ws.LatestBuild.ID == workspace.LatestBuild.ID { @@ -537,7 +540,7 @@ func assertLatestTransitionWithNoEventualBuild(t *testing.T, client *codersdk.Cl } // At this point a build should not have happened. Is the state still as expected? return assert.Equal(t, expected, ws.LatestBuild.Transition) - }, 5*time.Second, 25*time.Millisecond) + }, 3*time.Second, 25*time.Millisecond) } func TestMain(m *testing.M) { From 05e4c5e922dbb797d73a784d180b0e4490f27397 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 30 May 2022 10:44:43 +0100 Subject: [PATCH 3/5] no asserts in eventually/never --- coderd/autobuild/executor/lifecycle_executor_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index f75cb2e559ac4..3abf702aa1d5a 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -515,6 +515,7 @@ func mustWorkspace(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID) func assertLatestTransitionWithEventualBuild(t *testing.T, client *codersdk.Client, workspace codersdk.Workspace, expected codersdk.WorkspaceTransition) { t.Helper() + // assert.Eventually appears to have concurrency issues assert.Eventually(t, func() bool { ws := mustWorkspace(t, client, workspace.ID) if ws.LatestBuild.ID == workspace.LatestBuild.ID { @@ -524,7 +525,8 @@ func assertLatestTransitionWithEventualBuild(t *testing.T, client *codersdk.Clie return false } // At this point a build has happened. Is it what we want? - return assert.Equal(t, expected, ws.LatestBuild.Transition) + // return assert.Equal(t, expected, ws.LatestBuild.Transition) + return expected == ws.LatestBuild.Transition }, 3*time.Second, 25*time.Millisecond) } @@ -539,7 +541,8 @@ func assertLatestTransitionWithNoEventualBuild(t *testing.T, client *codersdk.Cl return false } // At this point a build should not have happened. Is the state still as expected? - return assert.Equal(t, expected, ws.LatestBuild.Transition) + // return assert.Equal(t, expected, ws.LatestBuild.Transition) + return expected == ws.LatestBuild.Transition }, 3*time.Second, 25*time.Millisecond) } From 0d96b341692ee8cd2ba447f2a7da4a97c29c512b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 30 May 2022 12:08:41 +0100 Subject: [PATCH 4/5] test: autobuild/executor: add channel for execution stats, use for assertions --- .../autobuild/executor/lifecycle_executor.go | 46 ++- .../executor/lifecycle_executor_test.go | 286 ++++++++++-------- coderd/coderdtest/coderdtest.go | 24 +- 3 files changed, 209 insertions(+), 147 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index 831085049024a..ce7fb650caab1 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -17,10 +17,18 @@ import ( // Executor automatically starts or stops workspaces. type Executor struct { - ctx context.Context - db database.Store - log slog.Logger - tick <-chan time.Time + ctx context.Context + db database.Store + log slog.Logger + tick <-chan time.Time + statsCh chan<- RunStats +} + +// RunStats contains information about one run of Executor. +type RunStats struct { + Transitions map[uuid.UUID]database.WorkspaceTransition + Elapsed time.Duration + Error error } // New returns a new autobuild executor. @@ -34,22 +42,42 @@ func New(ctx context.Context, db database.Store, log slog.Logger, tick <-chan ti return le } +// WithStatsChannel will cause Executor to push a RunStats to ch after +// every tick. +func (e *Executor) WithStatsChannel(ch chan<- RunStats) *Executor { + e.statsCh = ch + return e +} + // Run will cause executor to start or stop workspaces on every // tick from its channel. It will stop when its context is Done, or when // its channel is closed. func (e *Executor) Run() { go func() { for t := range e.tick { - if err := e.runOnce(t); err != nil { - e.log.Error(e.ctx, "error running once", slog.Error(err)) + stats := e.runOnce(t) + if stats.Error != nil { + e.log.Error(e.ctx, "error running once", slog.Error(stats.Error)) } + if e.statsCh != nil { + e.statsCh <- stats + } + e.log.Debug(e.ctx, "run stats", slog.F("elapsed", stats.Elapsed), slog.F("transitions", stats.Transitions)) } }() } -func (e *Executor) runOnce(t time.Time) error { +func (e *Executor) runOnce(t time.Time) RunStats { + var err error + stats := RunStats{ + Transitions: make(map[uuid.UUID]database.WorkspaceTransition), + } + defer func() { + stats.Elapsed = time.Since(t) + stats.Error = err + }() currentTick := t.Truncate(time.Minute) - return e.db.InTx(func(db database.Store) error { + err = e.db.InTx(func(db database.Store) error { // TTL is set at the workspace level, and deadline at the workspace build level. // When a workspace build is created, its deadline initially starts at zero. // When provisionerd successfully completes a provision job, the deadline is @@ -146,6 +174,7 @@ func (e *Executor) runOnce(t time.Time) error { slog.F("transition", validTransition), ) + stats.Transitions[ws.ID] = validTransition if err := build(e.ctx, db, ws, validTransition, priorHistory, priorJob); err != nil { e.log.Error(e.ctx, "unable to transition workspace", slog.F("workspace_id", ws.ID), @@ -156,6 +185,7 @@ func (e *Executor) runOnce(t time.Time) error { } return nil }) + return stats } // TODO(cian): this function duplicates most of api.postWorkspaceBuilds. Refactor. diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index 3abf702aa1d5a..1d1edcd190b4d 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -10,6 +10,7 @@ import ( "go.uber.org/goleak" + "github.com/coder/coder/coderd/autobuild/executor" "github.com/coder/coder/coderd/autobuild/schedule" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/database" @@ -24,12 +25,14 @@ func TestExecutorAutostartOK(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - err error - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + ctx = context.Background() + err error + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -51,19 +54,25 @@ func TestExecutorAutostartOK(t *testing.T) { }() // Then: the workspace should eventually be started - assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) + stats := <-statsCh + assert.NoError(t, stats.Error) + assert.Len(t, stats.Transitions, 1) + assert.Contains(t, stats.Transitions, workspace.ID) + assert.Equal(t, database.WorkspaceTransitionStart, stats.Transitions[workspace.ID]) } func TestExecutorAutostartTemplateUpdated(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - err error - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + ctx = context.Background() + err error + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -96,21 +105,27 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { }() // Then: the workspace should be started using the previous template version, and not the updated version. - assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) + stats := <-statsCh + assert.NoError(t, stats.Error) + assert.Len(t, stats.Transitions, 1) + assert.Contains(t, stats.Transitions, workspace.ID) + assert.Equal(t, database.WorkspaceTransitionStart, stats.Transitions[workspace.ID]) ws := mustWorkspace(t, client, workspace.ID) - require.Equal(t, workspace.LatestBuild.TemplateVersionID, ws.LatestBuild.TemplateVersionID, "expected workspace build to be using the old template version") + assert.Equal(t, workspace.LatestBuild.TemplateVersionID, ws.LatestBuild.TemplateVersionID, "expected workspace build to be using the old template version") } func TestExecutorAutostartAlreadyRunning(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - err error - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + ctx = context.Background() + err error + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -133,17 +148,21 @@ func TestExecutorAutostartAlreadyRunning(t *testing.T) { }() // Then: the workspace should not be started. - assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) + stats := <-statsCh + require.NoError(t, stats.Error) + require.Len(t, stats.Transitions, 0) } func TestExecutorAutostartNotEnabled(t *testing.T) { t.Parallel() var ( - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { @@ -164,17 +183,21 @@ func TestExecutorAutostartNotEnabled(t *testing.T) { }() // Then: the workspace should not be started. - assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) + stats := <-statsCh + require.NoError(t, stats.Error) + require.Len(t, stats.Transitions, 0) } func TestExecutorAutostopOK(t *testing.T) { t.Parallel() var ( - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -190,18 +213,24 @@ func TestExecutorAutostopOK(t *testing.T) { }() // Then: the workspace should be stopped - assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) + stats := <-statsCh + assert.NoError(t, stats.Error) + assert.Len(t, stats.Transitions, 1) + assert.Contains(t, stats.Transitions, workspace.ID) + assert.Equal(t, database.WorkspaceTransitionStop, stats.Transitions[workspace.ID]) } func TestExecutorAutostopExtend(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + ctx = context.Background() + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -224,7 +253,9 @@ func TestExecutorAutostopExtend(t *testing.T) { }() // Then: nothing should happen and the workspace should stay running - assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) + stats := <-statsCh + assert.NoError(t, stats.Error) + assert.Len(t, stats.Transitions, 0) // When: the autobuild executor ticks after the *new* deadline: go func() { @@ -233,17 +264,23 @@ func TestExecutorAutostopExtend(t *testing.T) { }() // Then: the workspace should be stopped - assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) + stats = <-statsCh + assert.NoError(t, stats.Error) + assert.Len(t, stats.Transitions, 1) + assert.Contains(t, stats.Transitions, workspace.ID) + assert.Equal(t, database.WorkspaceTransitionStop, stats.Transitions[workspace.ID]) } func TestExecutorAutostopAlreadyStopped(t *testing.T) { t.Parallel() var ( - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) // Given: we have a user with a workspace (disabling autostart) workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { @@ -261,17 +298,21 @@ func TestExecutorAutostopAlreadyStopped(t *testing.T) { }() // Then: the workspace should remain stopped and no build should happen. - assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) + stats := <-statsCh + assert.NoError(t, stats.Error) + assert.Len(t, stats.Transitions, 0) } func TestExecutorAutostopNotEnabled(t *testing.T) { t.Parallel() var ( - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) // Given: we have a user with a workspace that has no TTL set workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { @@ -292,19 +333,23 @@ func TestExecutorAutostopNotEnabled(t *testing.T) { }() // Then: the workspace should not be stopped. - assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) + stats := <-statsCh + assert.NoError(t, stats.Error) + assert.Len(t, stats.Transitions, 0) } func TestExecutorWorkspaceDeleted(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - err error - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + ctx = context.Background() + err error + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -327,19 +372,23 @@ func TestExecutorWorkspaceDeleted(t *testing.T) { }() // Then: nothing should happen - assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionDelete) + stats := <-statsCh + assert.NoError(t, stats.Error) + assert.Len(t, stats.Transitions, 0) } func TestExecutorWorkspaceAutostartTooEarly(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - err error - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + ctx = context.Background() + err error + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) futureTime = time.Now().Add(time.Hour) futureTimeCron = fmt.Sprintf("%d %d * * *", futureTime.Minute(), futureTime.Hour()) @@ -363,17 +412,21 @@ func TestExecutorWorkspaceAutostartTooEarly(t *testing.T) { }() // Then: nothing should happen - assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) + stats := <-statsCh + assert.NoError(t, stats.Error) + assert.Len(t, stats.Transitions, 0) } func TestExecutorWorkspaceAutostopBeforeDeadline(t *testing.T) { t.Parallel() var ( - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -386,18 +439,22 @@ func TestExecutorWorkspaceAutostopBeforeDeadline(t *testing.T) { }() // Then: nothing should happen - assertLatestTransitionWithNoEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) + stats := <-statsCh + assert.NoError(t, stats.Error) + assert.Len(t, stats.Transitions, 0) } func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - tickCh = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + ctx = context.Background() + tickCh = make(chan time.Time) + statsCh = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -414,7 +471,11 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) { }() // Then: the workspace should still stop - sorry! - assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStop) + stats := <-statsCh + assert.NoError(t, stats.Error) + assert.Len(t, stats.Transitions, 1) + assert.Contains(t, stats.Transitions, workspace.ID) + assert.Equal(t, database.WorkspaceTransitionStop, stats.Transitions[workspace.ID]) } func TestExecutorAutostartMultipleOK(t *testing.T) { @@ -425,17 +486,19 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - err error - tickCh = make(chan time.Time) - tickCh2 = make(chan time.Time) - client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, + tickCh = make(chan time.Time) + tickCh2 = make(chan time.Time) + statsCh1 = make(chan executor.RunStats) + statsCh2 = make(chan executor.RunStats) + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh1, }) _ = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh2, - IncludeProvisionerD: true, + AutobuildTicker: tickCh2, + IncludeProvisionerD: true, + AutobuildStatsChannel: statsCh2, }) // Given: we have a user with a workspace that has autostart enabled (default) workspace = mustProvisionWorkspace(t, client) @@ -452,20 +515,16 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { }() // Then: the workspace should eventually be started - assertLatestTransitionWithEventualBuild(t, client, workspace, codersdk.WorkspaceTransitionStart) - - ws := mustWorkspace(t, client, workspace.ID) - builds, err := client.WorkspaceBuilds(ctx, codersdk.WorkspaceBuildsRequest{WorkspaceID: ws.ID}) - require.NoError(t, err, "fetch list of workspace builds from primary") - // One build to start, one stop transition, and one autostart. No more. - if assert.Len(t, builds, 3, "unexpected number of builds for workspace from primary") { - assert.Equal(t, codersdk.WorkspaceTransitionStart, builds[0].Transition) - assert.Equal(t, codersdk.WorkspaceTransitionStop, builds[1].Transition) - assert.Equal(t, codersdk.WorkspaceTransitionStart, builds[2].Transition) - // Builds are returned most recent first. - require.True(t, builds[0].CreatedAt.After(builds[1].CreatedAt)) - require.True(t, builds[1].CreatedAt.After(builds[2].CreatedAt)) - } + stats1 := <-statsCh1 + assert.NoError(t, stats1.Error) + assert.Len(t, stats1.Transitions, 1) + assert.Contains(t, stats1.Transitions, workspace.ID) + assert.Equal(t, database.WorkspaceTransitionStart, stats1.Transitions[workspace.ID]) + + // Then: the other executor should not have done anything + stats2 := <-statsCh2 + assert.NoError(t, stats2.Error) + assert.Len(t, stats2.Transitions, 0) } func mustProvisionWorkspace(t *testing.T, client *codersdk.Client, mut ...func(*codersdk.CreateWorkspaceRequest)) codersdk.Workspace { @@ -513,39 +572,6 @@ func mustWorkspace(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID) return ws } -func assertLatestTransitionWithEventualBuild(t *testing.T, client *codersdk.Client, workspace codersdk.Workspace, expected codersdk.WorkspaceTransition) { - t.Helper() - // assert.Eventually appears to have concurrency issues - assert.Eventually(t, func() bool { - ws := mustWorkspace(t, client, workspace.ID) - if ws.LatestBuild.ID == workspace.LatestBuild.ID { - return false - } - if ws.LatestBuild.Job.Status != codersdk.ProvisionerJobSucceeded { - return false - } - // At this point a build has happened. Is it what we want? - // return assert.Equal(t, expected, ws.LatestBuild.Transition) - return expected == ws.LatestBuild.Transition - }, 3*time.Second, 25*time.Millisecond) -} - -func assertLatestTransitionWithNoEventualBuild(t *testing.T, client *codersdk.Client, workspace codersdk.Workspace, expected codersdk.WorkspaceTransition) { - t.Helper() - assert.Never(t, func() bool { - ws := mustWorkspace(t, client, workspace.ID) - if ws.LatestBuild.ID == workspace.LatestBuild.ID { - return false - } - if ws.LatestBuild.Job.Status != codersdk.ProvisionerJobSucceeded { - return false - } - // At this point a build should not have happened. Is the state still as expected? - // return assert.Equal(t, expected, ws.LatestBuild.Transition) - return expected == ws.LatestBuild.Transition - }, 3*time.Second, 25*time.Millisecond) -} - func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 7d397e78c2758..ce994d9ad48d6 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -56,14 +56,15 @@ import ( ) type Options struct { - AWSCertificates awsidentity.Certificates - Authorizer rbac.Authorizer - AzureCertificates x509.VerifyOptions - GithubOAuth2Config *coderd.GithubOAuth2Config - GoogleTokenValidator *idtoken.Validator - SSHKeygenAlgorithm gitsshkey.Algorithm - APIRateLimit int - AutobuildTicker <-chan time.Time + AWSCertificates awsidentity.Certificates + Authorizer rbac.Authorizer + AzureCertificates x509.VerifyOptions + GithubOAuth2Config *coderd.GithubOAuth2Config + GoogleTokenValidator *idtoken.Validator + SSHKeygenAlgorithm gitsshkey.Algorithm + APIRateLimit int + AutobuildTicker <-chan time.Time + AutobuildStatsChannel chan<- executor.RunStats // IncludeProvisionerD when true means to start an in-memory provisionerD IncludeProvisionerD bool @@ -92,6 +93,11 @@ func NewWithAPI(t *testing.T, options *Options) (*codersdk.Client, *coderd.API) options.AutobuildTicker = ticker t.Cleanup(func() { close(ticker) }) } + if options.AutobuildStatsChannel != nil { + t.Cleanup(func() { + close(options.AutobuildStatsChannel) + }) + } // This can be hotswapped for a live database instance. db := databasefake.New() @@ -122,7 +128,7 @@ func NewWithAPI(t *testing.T, options *Options) (*codersdk.Client, *coderd.API) db, slogtest.Make(t, nil).Named("autobuild.executor").Leveled(slog.LevelDebug), options.AutobuildTicker, - ) + ).WithStatsChannel(options.AutobuildStatsChannel) lifecycleExecutor.Run() srv := httptest.NewUnstartedServer(nil) From 4314760b2f649427d4976c4817d3d722db8cb7f9 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Mon, 30 May 2022 19:08:45 +0000 Subject: [PATCH 5/5] Address PR comments --- .../autobuild/executor/lifecycle_executor.go | 12 +- .../executor/lifecycle_executor_test.go | 112 +++++++++--------- coderd/coderdtest/coderdtest.go | 24 ++-- 3 files changed, 74 insertions(+), 74 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index ce7fb650caab1..55b8b3228f775 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -21,11 +21,11 @@ type Executor struct { db database.Store log slog.Logger tick <-chan time.Time - statsCh chan<- RunStats + statsCh chan<- Stats } -// RunStats contains information about one run of Executor. -type RunStats struct { +// Stats contains information about one run of Executor. +type Stats struct { Transitions map[uuid.UUID]database.WorkspaceTransition Elapsed time.Duration Error error @@ -44,7 +44,7 @@ func New(ctx context.Context, db database.Store, log slog.Logger, tick <-chan ti // WithStatsChannel will cause Executor to push a RunStats to ch after // every tick. -func (e *Executor) WithStatsChannel(ch chan<- RunStats) *Executor { +func (e *Executor) WithStatsChannel(ch chan<- Stats) *Executor { e.statsCh = ch return e } @@ -67,9 +67,9 @@ func (e *Executor) Run() { }() } -func (e *Executor) runOnce(t time.Time) RunStats { +func (e *Executor) runOnce(t time.Time) Stats { var err error - stats := RunStats{ + stats := Stats{ Transitions: make(map[uuid.UUID]database.WorkspaceTransition), } defer func() { diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index 1d1edcd190b4d..8ca7e321d9030 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -28,11 +28,11 @@ func TestExecutorAutostartOK(t *testing.T) { ctx = context.Background() err error tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -68,11 +68,11 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { ctx = context.Background() err error tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -121,11 +121,11 @@ func TestExecutorAutostartAlreadyRunning(t *testing.T) { ctx = context.Background() err error tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -158,11 +158,11 @@ func TestExecutorAutostartNotEnabled(t *testing.T) { var ( tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { @@ -193,11 +193,11 @@ func TestExecutorAutostopOK(t *testing.T) { var ( tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -226,11 +226,11 @@ func TestExecutorAutostopExtend(t *testing.T) { var ( ctx = context.Background() tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -276,11 +276,11 @@ func TestExecutorAutostopAlreadyStopped(t *testing.T) { var ( tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) // Given: we have a user with a workspace (disabling autostart) workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { @@ -308,11 +308,11 @@ func TestExecutorAutostopNotEnabled(t *testing.T) { var ( tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) // Given: we have a user with a workspace that has no TTL set workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { @@ -345,11 +345,11 @@ func TestExecutorWorkspaceDeleted(t *testing.T) { ctx = context.Background() err error tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -384,11 +384,11 @@ func TestExecutorWorkspaceAutostartTooEarly(t *testing.T) { ctx = context.Background() err error tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) futureTime = time.Now().Add(time.Hour) futureTimeCron = fmt.Sprintf("%d %d * * *", futureTime.Minute(), futureTime.Hour()) @@ -422,11 +422,11 @@ func TestExecutorWorkspaceAutostopBeforeDeadline(t *testing.T) { var ( tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -450,11 +450,11 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) { var ( ctx = context.Background() tickCh = make(chan time.Time) - statsCh = make(chan executor.RunStats) + statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh, }) // Given: we have a user with a workspace workspace = mustProvisionWorkspace(t, client) @@ -488,17 +488,17 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { var ( tickCh = make(chan time.Time) tickCh2 = make(chan time.Time) - statsCh1 = make(chan executor.RunStats) - statsCh2 = make(chan executor.RunStats) + statsCh1 = make(chan executor.Stats) + statsCh2 = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh1, + AutobuildTicker: tickCh, + IncludeProvisionerD: true, + AutobuildStats: statsCh1, }) _ = coderdtest.New(t, &coderdtest.Options{ - AutobuildTicker: tickCh2, - IncludeProvisionerD: true, - AutobuildStatsChannel: statsCh2, + AutobuildTicker: tickCh2, + IncludeProvisionerD: true, + AutobuildStats: statsCh2, }) // Given: we have a user with a workspace that has autostart enabled (default) workspace = mustProvisionWorkspace(t, client) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index ce994d9ad48d6..48fe3b81deb72 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -56,15 +56,15 @@ import ( ) type Options struct { - AWSCertificates awsidentity.Certificates - Authorizer rbac.Authorizer - AzureCertificates x509.VerifyOptions - GithubOAuth2Config *coderd.GithubOAuth2Config - GoogleTokenValidator *idtoken.Validator - SSHKeygenAlgorithm gitsshkey.Algorithm - APIRateLimit int - AutobuildTicker <-chan time.Time - AutobuildStatsChannel chan<- executor.RunStats + AWSCertificates awsidentity.Certificates + Authorizer rbac.Authorizer + AzureCertificates x509.VerifyOptions + GithubOAuth2Config *coderd.GithubOAuth2Config + GoogleTokenValidator *idtoken.Validator + SSHKeygenAlgorithm gitsshkey.Algorithm + APIRateLimit int + AutobuildTicker <-chan time.Time + AutobuildStats chan<- executor.Stats // IncludeProvisionerD when true means to start an in-memory provisionerD IncludeProvisionerD bool @@ -93,9 +93,9 @@ func NewWithAPI(t *testing.T, options *Options) (*codersdk.Client, *coderd.API) options.AutobuildTicker = ticker t.Cleanup(func() { close(ticker) }) } - if options.AutobuildStatsChannel != nil { + if options.AutobuildStats != nil { t.Cleanup(func() { - close(options.AutobuildStatsChannel) + close(options.AutobuildStats) }) } @@ -128,7 +128,7 @@ func NewWithAPI(t *testing.T, options *Options) (*codersdk.Client, *coderd.API) db, slogtest.Make(t, nil).Named("autobuild.executor").Leveled(slog.LevelDebug), options.AutobuildTicker, - ).WithStatsChannel(options.AutobuildStatsChannel) + ).WithStatsChannel(options.AutobuildStats) lifecycleExecutor.Run() srv := httptest.NewUnstartedServer(nil)