From 83773bdda8bb421692ed17cb702781ae2e38cb38 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 11 Dec 2025 19:20:28 +0000 Subject: [PATCH 1/2] test(cli): modernize TestTemplatePush and fix contexts --- cli/templatepush_test.go | 227 ++++++++++++++++++--------------------- 1 file changed, 107 insertions(+), 120 deletions(-) diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go index 28c5adc20f213..1e58713a24232 100644 --- a/cli/templatepush_test.go +++ b/cli/templatepush_test.go @@ -52,10 +52,9 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) matches := []struct { match string @@ -64,11 +63,11 @@ func TestTemplatePush(t *testing.T) { {match: "Upload", write: "yes"}, } for _, m := range matches { - pty.ExpectMatch(m.match) + pty.ExpectMatchContext(ctx, m.match) pty.WriteLine(m.write) } - require.NoError(t, <-execDone) + w.RequireSuccess() // Assert that the template version changed. templateVersions, err := client.TemplateVersionsByTemplate(context.Background(), codersdk.TemplateVersionsByTemplateRequest{ @@ -100,9 +99,7 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) - defer cancel() - + ctx := testutil.Context(t, testutil.WaitMedium) inv = inv.WithContext(ctx) w := clitest.StartWithWaiter(t, inv) @@ -111,6 +108,7 @@ func TestTemplatePush(t *testing.T) { w.RequireSuccess() // Assert that the template version changed. + ctx = testutil.Context(t, testutil.WaitMedium) templateVersions, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ TemplateID: template.ID, }) @@ -134,9 +132,6 @@ func TestTemplatePush(t *testing.T) { ProvisionApply: echo.ApplyComplete, }) - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - for i, tt := range []struct { wantMessage string wantMatch string @@ -153,6 +148,7 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) + ctx := testutil.Context(t, testutil.WaitMedium) inv = inv.WithContext(ctx) w := clitest.StartWithWaiter(t, inv) @@ -161,6 +157,7 @@ func TestTemplatePush(t *testing.T) { w.RequireSuccess() // Assert that the template version changed. + ctx = testutil.Context(t, testutil.WaitMedium) templateVersions, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ TemplateID: template.ID, }) @@ -196,10 +193,9 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) matches := []struct { match string @@ -209,14 +205,14 @@ func TestTemplatePush(t *testing.T) { {match: "Upload", write: "no"}, } for _, m := range matches { - pty.ExpectMatch(m.match) + pty.ExpectMatchContext(ctx, m.match) if m.write != "" { pty.WriteLine(m.write) } } // cmd should error once we say no. - require.Error(t, <-execDone) + w.RequireError() }) t.Run("NoLockfileIgnored", func(t *testing.T) { @@ -245,21 +241,19 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) { - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) - defer cancel() + ctx := testutil.Context(t, testutil.WaitMedium) pty.ExpectNoMatchBefore(ctx, "No .terraform.lock.hcl file found", "Upload") pty.WriteLine("no") } // cmd should error once we say no. - require.Error(t, <-execDone) + w.RequireError() }) t.Run("PushInactiveTemplateVersion", func(t *testing.T) { @@ -285,6 +279,8 @@ func TestTemplatePush(t *testing.T) { ) clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) w := clitest.StartWithWaiter(t, inv) matches := []struct { @@ -294,14 +290,15 @@ func TestTemplatePush(t *testing.T) { {match: "Upload", write: "yes"}, } for _, m := range matches { - pty.ExpectMatch(m.match) + pty.ExpectMatchContext(ctx, m.match) pty.WriteLine(m.write) } w.RequireSuccess() // Assert that the template version didn't change. - templateVersions, err := client.TemplateVersionsByTemplate(context.Background(), codersdk.TemplateVersionsByTemplateRequest{ + ctx = testutil.Context(t, testutil.WaitMedium) + templateVersions, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ TemplateID: template.ID, }) require.NoError(t, err) @@ -344,7 +341,9 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - waiter := clitest.StartWithWaiter(t, inv) + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) matches := []struct { match string @@ -353,14 +352,15 @@ func TestTemplatePush(t *testing.T) { {match: "Upload", write: "yes"}, } for _, m := range matches { - pty.ExpectMatch(m.match) + pty.ExpectMatchContext(ctx, m.match) pty.WriteLine(m.write) } - waiter.RequireSuccess() + w.RequireSuccess() // Assert that the template version changed. - templateVersions, err := client.TemplateVersionsByTemplate(context.Background(), codersdk.TemplateVersionsByTemplateRequest{ + ctx = testutil.Context(t, testutil.WaitMedium) + templateVersions, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ TemplateID: template.ID, }) require.NoError(t, err) @@ -541,16 +541,15 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - ctx := testutil.Context(t, testutil.WaitShort) + setupCtx := testutil.Context(t, testutil.WaitMedium) now := dbtime.Now() - require.NoError(t, tt.setupDaemon(ctx, store, owner, wantTags, now)) + require.NoError(t, tt.setupDaemon(setupCtx, store, owner, wantTags, now)) + ctx := testutil.Context(t, testutil.WaitMedium) cancelCtx, cancel := context.WithCancel(ctx) - t.Cleanup(cancel) - done := make(chan error) - go func() { - done <- inv.WithContext(cancelCtx).Run() - }() + defer cancel() + inv = inv.WithContext(cancelCtx) + w := clitest.StartWithWaiter(t, inv) require.Eventually(t, func() bool { jobs, err := store.GetProvisionerJobsCreatedAfter(ctx, time.Time{}) @@ -564,11 +563,11 @@ func TestTemplatePush(t *testing.T) { }, testutil.WaitShort, testutil.IntervalFast) if tt.expectOutput != "" { - pty.ExpectMatch(tt.expectOutput) + pty.ExpectMatchContext(ctx, tt.expectOutput) } cancel() - <-done + _ = w.Wait() }) } }) @@ -613,10 +612,9 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) matches := []struct { match string @@ -625,11 +623,11 @@ func TestTemplatePush(t *testing.T) { {match: "Upload", write: "yes"}, } for _, m := range matches { - pty.ExpectMatch(m.match) + pty.ExpectMatchContext(ctx, m.match) pty.WriteLine(m.write) } - require.NoError(t, <-execDone) + w.RequireSuccess() // Verify template version tags template, err := client.Template(context.Background(), template.ID) @@ -643,8 +641,6 @@ func TestTemplatePush(t *testing.T) { t.Run("DeleteTags", func(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitLong) - // Start the first provisioner with no tags. client, provisionerDocker, api := coderdtest.NewWithAPI(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, @@ -682,10 +678,9 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - execDone := make(chan error) - go func() { - execDone <- inv.WithContext(ctx).Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) matches := []struct { match string @@ -694,11 +689,11 @@ func TestTemplatePush(t *testing.T) { {match: "Upload", write: "yes"}, } for _, m := range matches { - pty.ExpectMatch(m.match) + pty.ExpectMatchContext(ctx, m.match) pty.WriteLine(m.write) } - require.NoError(t, <-execDone) + w.RequireSuccess() // Verify template version tags template, err := client.Template(ctx, template.ID) @@ -740,10 +735,9 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) matches := []struct { match string @@ -752,11 +746,11 @@ func TestTemplatePush(t *testing.T) { {match: "Upload", write: "yes"}, } for _, m := range matches { - pty.ExpectMatch(m.match) + pty.ExpectMatchContext(ctx, m.match) pty.WriteLine(m.write) } - require.NoError(t, <-execDone) + w.RequireSuccess() // Verify template version tags template, err := client.Template(context.Background(), template.ID) @@ -818,10 +812,9 @@ func TestTemplatePush(t *testing.T) { inv.Stdin = pty.Input() inv.Stdout = pty.Output() - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) matches := []struct { match string @@ -830,11 +823,11 @@ func TestTemplatePush(t *testing.T) { {match: "Upload", write: "yes"}, } for _, m := range matches { - pty.ExpectMatch(m.match) + pty.ExpectMatchContext(ctx, m.match) pty.WriteLine(m.write) } - require.NoError(t, <-execDone) + w.RequireSuccess() // Assert that the template version changed. templateVersions, err := client.TemplateVersionsByTemplate(context.Background(), codersdk.TemplateVersionsByTemplateRequest{ @@ -884,10 +877,9 @@ func TestTemplatePush(t *testing.T) { inv.Stdin = pty.Input() inv.Stdout = pty.Output() - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) matches := []struct { match string @@ -896,11 +888,11 @@ func TestTemplatePush(t *testing.T) { {match: "Upload", write: "yes"}, } for _, m := range matches { - pty.ExpectMatch(m.match) + pty.ExpectMatchContext(ctx, m.match) pty.WriteLine(m.write) } - require.NoError(t, <-execDone) + w.RequireSuccess() // Assert that the template version changed. templateVersions, err := client.TemplateVersionsByTemplate(context.Background(), codersdk.TemplateVersionsByTemplateRequest{ @@ -952,10 +944,9 @@ func TestTemplatePush(t *testing.T) { inv.Stdin = pty.Input() inv.Stdout = pty.Output() - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) matches := []struct { match string @@ -964,11 +955,11 @@ func TestTemplatePush(t *testing.T) { {match: "Upload", write: "yes"}, } for _, m := range matches { - pty.ExpectMatch(m.match) + pty.ExpectMatchContext(ctx, m.match) pty.WriteLine(m.write) } - require.NoError(t, <-execDone) + w.RequireSuccess() // Assert that the template version changed. templateVersions, err := client.TemplateVersionsByTemplate(context.Background(), codersdk.TemplateVersionsByTemplateRequest{ @@ -1005,7 +996,9 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - waiter := clitest.StartWithWaiter(t, inv) + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) matches := []struct { match string @@ -1015,13 +1008,13 @@ func TestTemplatePush(t *testing.T) { {match: "template has been created"}, } for _, m := range matches { - pty.ExpectMatch(m.match) + pty.ExpectMatchContext(ctx, m.match) if m.write != "" { pty.WriteLine(m.write) } } - waiter.RequireSuccess() + w.RequireSuccess() template, err := client.TemplateByName(context.Background(), owner.OrganizationID, templateName) require.NoError(t, err) @@ -1054,9 +1047,7 @@ func TestTemplatePush(t *testing.T) { inv.Stdin = strings.NewReader("invalid tar content that would cause failure") - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) - defer cancel() - + ctx := testutil.Context(t, testutil.WaitMedium) err := inv.WithContext(ctx).Run() require.NoError(t, err, "Should succeed without reading from stdin") @@ -1107,31 +1098,30 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) // Select "Yes" for the "Upload " prompt - pty.ExpectMatch("Upload") + pty.ExpectMatchContext(ctx, "Upload") pty.WriteLine("yes") - pty.ExpectMatch("var.string_var") - pty.ExpectMatch("Enter value:") + pty.ExpectMatchContext(ctx, "var.string_var") + pty.ExpectMatchContext(ctx, "Enter value:") pty.WriteLine("test-string") - pty.ExpectMatch("var.number_var") - pty.ExpectMatch("Enter value:") + pty.ExpectMatchContext(ctx, "var.number_var") + pty.ExpectMatchContext(ctx, "Enter value:") pty.WriteLine("42") // Boolean variable automatically selects the first option ("true") - pty.ExpectMatch("var.bool_var") + pty.ExpectMatchContext(ctx, "var.bool_var") - pty.ExpectMatch("var.sensitive_var") - pty.ExpectMatch("Enter value:") + pty.ExpectMatchContext(ctx, "var.sensitive_var") + pty.ExpectMatchContext(ctx, "Enter value:") pty.WriteLine("secret-value") - require.NoError(t, <-execDone) + w.RequireSuccess() }) t.Run("ValidateNumberInput", func(t *testing.T) { @@ -1154,23 +1144,22 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) // Select "Yes" for the "Upload " prompt - pty.ExpectMatch("Upload") + pty.ExpectMatchContext(ctx, "Upload") pty.WriteLine("yes") - pty.ExpectMatch("var.number_var") + pty.ExpectMatchContext(ctx, "var.number_var") pty.WriteLine("not-a-number") - pty.ExpectMatch("must be a valid number") + pty.ExpectMatchContext(ctx, "must be a valid number") pty.WriteLine("123.45") - require.NoError(t, <-execDone) + w.RequireSuccess() }) t.Run("DontPromptForDefaultValues", func(t *testing.T) { @@ -1198,19 +1187,18 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) // Select "Yes" for the "Upload " prompt - pty.ExpectMatch("Upload") + pty.ExpectMatchContext(ctx, "Upload") pty.WriteLine("yes") - pty.ExpectMatch("var.without_default") + pty.ExpectMatchContext(ctx, "var.without_default") pty.WriteLine("test-value") - require.NoError(t, <-execDone) + w.RequireSuccess() }) t.Run("VariableSourcesPriority", func(t *testing.T) { @@ -1268,21 +1256,20 @@ cli_overrides_file_var: from-file`) clitest.SetupConfig(t, templateAdmin, root) pty := ptytest.New(t).Attach(inv) - execDone := make(chan error) - go func() { - execDone <- inv.Run() - }() + ctx := testutil.Context(t, testutil.WaitMedium) + inv = inv.WithContext(ctx) + w := clitest.StartWithWaiter(t, inv) // Select "Yes" for the "Upload " prompt - pty.ExpectMatch("Upload") + pty.ExpectMatchContext(ctx, "Upload") pty.WriteLine("yes") // Only check for prompt_var, other variables should not prompt - pty.ExpectMatch("var.prompt_var") - pty.ExpectMatch("Enter value:") + pty.ExpectMatchContext(ctx, "var.prompt_var") + pty.ExpectMatchContext(ctx, "Enter value:") pty.WriteLine("from-prompt") - require.NoError(t, <-execDone) + w.RequireSuccess() template, err := client.TemplateByName(context.Background(), owner.OrganizationID, "test-template") require.NoError(t, err) From 8ef85c8aea1769fc07dc7e0970e8ed828a9a1c79 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 11 Dec 2025 19:34:01 +0000 Subject: [PATCH 2/2] order by name --- cli/templatepush_test.go | 22 ++++++++----------- coderd/database/queries.sql.go | 2 +- .../queries/templateversionvariables.sql | 2 +- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go index 1e58713a24232..58c1b803dbb3d 100644 --- a/cli/templatepush_test.go +++ b/cli/templatepush_test.go @@ -546,10 +546,8 @@ func TestTemplatePush(t *testing.T) { require.NoError(t, tt.setupDaemon(setupCtx, store, owner, wantTags, now)) ctx := testutil.Context(t, testutil.WaitMedium) - cancelCtx, cancel := context.WithCancel(ctx) - defer cancel() - inv = inv.WithContext(cancelCtx) - w := clitest.StartWithWaiter(t, inv) + inv = inv.WithContext(ctx) + clitest.Start(t, inv) // Only used for output, disregard exit status. require.Eventually(t, func() bool { jobs, err := store.GetProvisionerJobsCreatedAfter(ctx, time.Time{}) @@ -565,9 +563,6 @@ func TestTemplatePush(t *testing.T) { if tt.expectOutput != "" { pty.ExpectMatchContext(ctx, tt.expectOutput) } - - cancel() - _ = w.Wait() }) } }) @@ -1106,21 +1101,22 @@ func TestTemplatePush(t *testing.T) { pty.ExpectMatchContext(ctx, "Upload") pty.WriteLine("yes") - pty.ExpectMatchContext(ctx, "var.string_var") - pty.ExpectMatchContext(ctx, "Enter value:") - pty.WriteLine("test-string") + // Variables are prompted in alphabetical order. + // Boolean variable automatically selects the first option ("true") + pty.ExpectMatchContext(ctx, "var.bool_var") pty.ExpectMatchContext(ctx, "var.number_var") pty.ExpectMatchContext(ctx, "Enter value:") pty.WriteLine("42") - // Boolean variable automatically selects the first option ("true") - pty.ExpectMatchContext(ctx, "var.bool_var") - pty.ExpectMatchContext(ctx, "var.sensitive_var") pty.ExpectMatchContext(ctx, "Enter value:") pty.WriteLine("secret-value") + pty.ExpectMatchContext(ctx, "var.string_var") + pty.ExpectMatchContext(ctx, "Enter value:") + pty.WriteLine("test-string") + w.RequireSuccess() }) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 4ea20f1bed12d..4c81d43441cd0 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -15301,7 +15301,7 @@ func (q *sqlQuerier) InsertTemplateVersionTerraformValuesByJobID(ctx context.Con } const getTemplateVersionVariables = `-- name: GetTemplateVersionVariables :many -SELECT template_version_id, name, description, type, value, default_value, required, sensitive FROM template_version_variables WHERE template_version_id = $1 +SELECT template_version_id, name, description, type, value, default_value, required, sensitive FROM template_version_variables WHERE template_version_id = $1 ORDER BY name ` func (q *sqlQuerier) GetTemplateVersionVariables(ctx context.Context, templateVersionID uuid.UUID) ([]TemplateVersionVariable, error) { diff --git a/coderd/database/queries/templateversionvariables.sql b/coderd/database/queries/templateversionvariables.sql index ff6c16a6df1d7..3e37ed01d4735 100644 --- a/coderd/database/queries/templateversionvariables.sql +++ b/coderd/database/queries/templateversionvariables.sql @@ -23,4 +23,4 @@ VALUES ) RETURNING *; -- name: GetTemplateVersionVariables :many -SELECT * FROM template_version_variables WHERE template_version_id = $1; +SELECT * FROM template_version_variables WHERE template_version_id = $1 ORDER BY name;