From 65ca9ec9c6bff38456007af4728f63521735778c Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 21 Jul 2022 12:41:58 +0000 Subject: [PATCH 1/2] fix: coderd: TestPatchCancelTemplateVersion: check job error Two unit tests (TestPatchCancelTemplateVersion/Success and Test- PatchCancelWorkspaceBuild) implied errneously that the job was canceled successfully. This is not the case, as these unit tests do not include a Provision_Complete response in the input to the echo provisioner. Now explicitly checking the job error. --- coderd/coderdtest/coderdtest.go | 2 +- coderd/templateversions_test.go | 7 +++++-- coderd/workspacebuilds_test.go | 8 ++++++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 9c27fe8e0358f..98fd6d1a33b7a 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -230,7 +230,7 @@ func NewProvisionerDaemon(t *testing.T, coderAPI *coderd.API) io.Closer { Logger: slogtest.Make(t, nil).Named("provisionerd").Leveled(slog.LevelDebug), PollInterval: 10 * time.Millisecond, UpdateInterval: 25 * time.Millisecond, - ForceCancelInterval: 25 * time.Millisecond, + ForceCancelInterval: time.Second, Provisioners: provisionerd.Provisioners{ string(database.ProvisionerTypeEcho): proto.NewDRPCProvisionerClient(provisionersdk.Conn(echoClient)), }, diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 498a1e349a492..ce9cb396f98e8 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -150,8 +150,11 @@ func TestPatchCancelTemplateVersion(t *testing.T) { require.Eventually(t, func() bool { var err error version, err = client.TemplateVersion(context.Background(), version.ID) - require.NoError(t, err) - return version.Job.Status == codersdk.ProvisionerJobCanceled + 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 }, 5*time.Second, 25*time.Millisecond) }) } diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index 9ee40a6460525..c9f24d0442c14 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/coderdtest" @@ -228,8 +229,11 @@ func TestPatchCancelWorkspaceBuild(t *testing.T) { require.Eventually(t, func() bool { var err error build, err = client.WorkspaceBuild(context.Background(), build.ID) - require.NoError(t, err) - return build.Job.Status == codersdk.ProvisionerJobCanceled + return assert.NoError(t, err) && + // The job will never actually cancel successfully because it will never send a + // provision complete response. + assert.Empty(t, build.Job.Error) && + build.Job.Status == codersdk.ProvisionerJobCanceling }, 5*time.Second, 25*time.Millisecond) } From 7b491d62257ea45d1dd219f15d6b33042dd23f56 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 21 Jul 2022 19:17:45 +0000 Subject: [PATCH 2/2] fixup! fix: coderd: TestPatchCancelTemplateVersion: check job error --- coderd/templateversions_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index ce9cb396f98e8..0c461cf4d303a 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -126,7 +126,9 @@ func TestPatchCancelTemplateVersion(t *testing.T) { require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusPreconditionFailed, apiErr.StatusCode()) }) - t.Run("Success", func(t *testing.T) { + // TODO(Cian): until we are able to test cancellation properly, validating + // Running -> Canceling is the best we can do for now. + t.Run("Canceling", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user := coderdtest.CreateFirstUser(t, client)