From 89030405d4e0616e75e18e41a418b08ab272762d Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Fri, 25 Aug 2023 09:11:44 +0000 Subject: [PATCH 1/5] Fix test flake introduced by #9264 Signed-off-by: Spike Curtis --- coderd/templateversions_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index d06e68fabb368..2656745e27c53 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -308,11 +308,12 @@ func TestPatchCancelTemplateVersion(t *testing.T) { require.Eventually(t, func() bool { var err error version, err = client.TemplateVersion(ctx, version.ID) + // job gets marked Failed when there is an Error; in practice we never get to Status = Canceled + // because provisioners report an Error when canceled. We check the Error string to ensure we don't mask + // other errors in this test. return assert.NoError(t, err) && - // The job will never actually cancel successfully because it will never send a - // provision complete response. - assert.Empty(t, version.Job.Error) && - version.Job.Status == codersdk.ProvisionerJobCanceling + version.Job.Error == "canceled" && + version.Job.Status == codersdk.ProvisionerJobFailed }, testutil.WaitShort, testutil.IntervalFast) }) } From ee0636a26c65ab844f58006c26441cd87d07e6be Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Fri, 25 Aug 2023 09:24:45 +0000 Subject: [PATCH 2/5] change check to match suffix Signed-off-by: Spike Curtis --- coderd/templateversions_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 2656745e27c53..2ca934e2d4085 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -311,8 +311,9 @@ func TestPatchCancelTemplateVersion(t *testing.T) { // job gets marked Failed when there is an Error; in practice we never get to Status = Canceled // because provisioners report an Error when canceled. We check the Error string to ensure we don't mask // other errors in this test. + t.Logf("got version %s | %s", version.Job.Error, version.Job.Status) return assert.NoError(t, err) && - version.Job.Error == "canceled" && + strings.HasSuffix(version.Job.Error, "canceled") && version.Job.Status == codersdk.ProvisionerJobFailed }, testutil.WaitShort, testutil.IntervalFast) }) From b4d796866c0003922850fead0d819cd21756f740 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Fri, 25 Aug 2023 09:39:49 +0000 Subject: [PATCH 3/5] Ignore error logs on uncanceled build Signed-off-by: Spike Curtis --- coderd/workspacebuilds_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index 0ee810e3e3eda..2c32f9ac39b7a 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -14,6 +14,8 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/xerrors" + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" @@ -412,7 +414,10 @@ func TestPatchCancelWorkspaceBuild(t *testing.T) { t.Run("User is not allowed to cancel", func(t *testing.T) { t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + // need to include our own logger because the provisioner (rightly) drops error logs when we shut down the + // test with a build in progress. + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, Logger: &logger}) owner := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, &echo.Responses{ Parse: echo.ParseComplete, From 52db2cee198e8d1d96ec9c9b0e494dea9293b9fb Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Fri, 25 Aug 2023 10:09:52 +0000 Subject: [PATCH 4/5] ignore error logs in Test_Runner/CleanupPendingBuild Signed-off-by: Spike Curtis --- scaletest/createworkspaces/run_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scaletest/createworkspaces/run_test.go b/scaletest/createworkspaces/run_test.go index b69297f622676..ddb9e76a4922e 100644 --- a/scaletest/createworkspaces/run_test.go +++ b/scaletest/createworkspaces/run_test.go @@ -163,8 +163,12 @@ func Test_Runner(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() + // need to include our own logger because the provisioner (rightly) drops error logs when we shut down the + // test with a build in progress. + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client := coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, + Logger: &logger, }) user := coderdtest.CreateFirstUser(t, client) @@ -257,8 +261,8 @@ func Test_Runner(t *testing.T) { continue } - // And it should be either canceled or canceling - if build.Job.Status == codersdk.ProvisionerJobCanceled || build.Job.Status == codersdk.ProvisionerJobCanceling { + // And it should be either failed (Echo returns an error when job is canceled) or canceling + if build.Job.Status == codersdk.ProvisionerJobFailed || build.Job.Status == codersdk.ProvisionerJobCanceling { return true } } From 2d3bd031b369f4c3e28a04f5bc63d97a4bafb915 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Fri, 25 Aug 2023 10:35:58 +0000 Subject: [PATCH 5/5] accept canceled in Test_Runner/CleanupPendingBuild Signed-off-by: Spike Curtis --- scaletest/createworkspaces/run_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/scaletest/createworkspaces/run_test.go b/scaletest/createworkspaces/run_test.go index ddb9e76a4922e..d5e96e22fcc83 100644 --- a/scaletest/createworkspaces/run_test.go +++ b/scaletest/createworkspaces/run_test.go @@ -255,14 +255,17 @@ func Test_Runner(t *testing.T) { if err != nil { return false } - for _, build := range builds { + for i, build := range builds { + t.Logf("checking build #%d: %s | %s", i, build.Transition, build.Job.Status) // One of the builds should be for creating the workspace, if build.Transition != codersdk.WorkspaceTransitionStart { continue } - // And it should be either failed (Echo returns an error when job is canceled) or canceling - if build.Job.Status == codersdk.ProvisionerJobFailed || build.Job.Status == codersdk.ProvisionerJobCanceling { + // And it should be either failed (Echo returns an error when job is canceled), canceling, or canceled. + if build.Job.Status == codersdk.ProvisionerJobFailed || + build.Job.Status == codersdk.ProvisionerJobCanceling || + build.Job.Status == codersdk.ProvisionerJobCanceled { return true } }