From 4cb8daf8b4afd0ad1cd757004f81f1bbbd0389a0 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 11 Dec 2025 15:36:43 +0000 Subject: [PATCH 1/6] perf: optimize postgres on macOS/Windows for CI, remove ramdisk Previously we used ramdisk for embedded postgres on macOS and Windows to improve test performance. This wastes memory since data ends up cached twice: once in the ramdisk and again in postgres's own buffers. Instead, we give postgres more memory and tune it more aggressively: - Increase shared_buffers to 2GB, effective_cache_size to 4GB - Minimal WAL level, infrequent checkpoints - Disable autovacuum, JIT, logging, stats tracking - OS-specific I/O settings This gives postgres more memory to work with while simplifying CI setup. --- .github/workflows/ci.yaml | 26 +----------------- scripts/embedded-pg/main.go | 55 +++++++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 28 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 68494f3d21cc1..dcb783f01a66a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -363,12 +363,6 @@ jobs: sudo mdutil -X / sudo launchctl bootout system /System/Library/LaunchDaemons/com.apple.metadata.mds.plist - # Set up RAM disks to speed up the rest of the job. This action is in - # a separate repository to allow its use before actions/checkout. - - name: Setup RAM Disks - if: runner.os == 'Windows' - uses: coder/setup-ramdisk-action@e1100847ab2d7bcd9d14bcda8f2d1b0f07b36f1b # v0.1.0 - - name: Checkout uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 with: @@ -426,27 +420,11 @@ jobs: source scripts/normalize_path.sh normalize_path_with_symlinks "$RUNNER_TEMP/sym" "$(dirname "$(which terraform)")" - - name: Setup RAM disk for Embedded Postgres (Windows) - if: runner.os == 'Windows' - shell: bash - # The default C: drive is extremely slow: - # https://github.com/actions/runner-images/issues/8755 - run: mkdir -p "R:/temp/embedded-pg" - - - name: Setup RAM disk for Embedded Postgres (macOS) + - name: Setup macOS (install google-chrome, silence zsh warning) if: runner.os == 'macOS' shell: bash run: | - # Postgres runs faster on a ramdisk on macOS. - mkdir -p /tmp/tmpfs - sudo mount_tmpfs -o noowners -s 8g /tmp/tmpfs - - # Install google-chrome for scaletests. - # As another concern, should we really have this kind of external dependency - # requirement on standard CI? brew install google-chrome - - # macOS will output "The default interactive shell is now zsh" intermittently in CI. touch ~/.bash_profile && echo "export BASH_SILENCE_DEPRECATION_WARNING=1" >> ~/.bash_profile - name: Test with PostgreSQL Database (Linux) @@ -476,7 +454,6 @@ jobs: test-count: ${{ github.ref == 'refs/heads/main' && '1' || '' }} # Only the CLI and Agent are officially supported on macOS; the rest are too flaky. test-packages: "./cli/... ./enterprise/cli/... ./agent/..." - embedded-pg-path: "/tmp/tmpfs/embedded-pg" embedded-pg-cache: ${{ steps.embedded-pg-cache.outputs.embedded-pg-cache }} - name: Test with PostgreSQL Database (Windows) @@ -492,7 +469,6 @@ jobs: test-count: ${{ github.ref == 'refs/heads/main' && '1' || '' }} # Only the CLI and Agent are officially supported on Windows; the rest are too flaky. test-packages: "./cli/... ./enterprise/cli/... ./agent/..." - embedded-pg-path: "R:/temp/embedded-pg" embedded-pg-cache: ${{ steps.embedded-pg-cache.outputs.embedded-pg-cache }} - name: Upload failed test db dumps diff --git a/scripts/embedded-pg/main.go b/scripts/embedded-pg/main.go index 705fec712693f..470c837e4706f 100644 --- a/scripts/embedded-pg/main.go +++ b/scripts/embedded-pg/main.go @@ -7,6 +7,7 @@ import ( "log" "os" "path/filepath" + "runtime" "time" embeddedpostgres "github.com/fergusstrange/embedded-postgres" @@ -78,15 +79,63 @@ func main() { // Windows can't handle. // Related issue: // https://github.com/fergusstrange/embedded-postgres/issues/145 + // + // Optimized for CI: speed over durability, crash safety disabled. paramQueries := []string{ - `ALTER SYSTEM SET effective_cache_size = '1GB';`, + // Disable durability, data doesn't need to survive a crash. `ALTER SYSTEM SET fsync = 'off';`, + `ALTER SYSTEM SET synchronous_commit = 'off';`, `ALTER SYSTEM SET full_page_writes = 'off';`, + `ALTER SYSTEM SET wal_level = 'minimal';`, + `ALTER SYSTEM SET max_wal_senders = '0';`, + + // Minimize disk writes by batching and avoiding flushes. + `ALTER SYSTEM SET checkpoint_timeout = '30min';`, + `ALTER SYSTEM SET max_wal_size = '4GB';`, + `ALTER SYSTEM SET min_wal_size = '1GB';`, + `ALTER SYSTEM SET backend_flush_after = '0';`, + `ALTER SYSTEM SET checkpoint_flush_after = '0';`, + `ALTER SYSTEM SET wal_buffers = '64MB';`, + `ALTER SYSTEM SET bgwriter_lru_maxpages = '0';`, + + // Tests are short-lived and each uses its own database. + `ALTER SYSTEM SET autovacuum = 'off';`, + `ALTER SYSTEM SET track_activities = 'off';`, + `ALTER SYSTEM SET track_counts = 'off';`, + + // Reduce overhead from JIT and logging. + `ALTER SYSTEM SET jit = 'off';`, + `ALTER SYSTEM SET log_checkpoints = 'off';`, + `ALTER SYSTEM SET log_connections = 'off';`, + `ALTER SYSTEM SET log_disconnections = 'off';`, + `ALTER SYSTEM SET max_connections = '1000';`, - `ALTER SYSTEM SET shared_buffers = '1GB';`, - `ALTER SYSTEM SET synchronous_commit = 'off';`, `ALTER SYSTEM SET client_encoding = 'UTF8';`, } + + // Memory and I/O settings tuned per Depot runner specs: + // - macOS: 24GB RAM, fast disk (has disk accelerator) + // - Windows: 128GB RAM, slow disk (EBS, no accelerator) + switch runtime.GOOS { + case "darwin": + paramQueries = append(paramQueries, + `ALTER SYSTEM SET shared_buffers = '4GB';`, + `ALTER SYSTEM SET effective_cache_size = '8GB';`, + `ALTER SYSTEM SET work_mem = '32MB';`, + `ALTER SYSTEM SET maintenance_work_mem = '512MB';`, + `ALTER SYSTEM SET temp_buffers = '64MB';`, + `ALTER SYSTEM SET random_page_cost = '1.0';`, + ) + case "windows": + paramQueries = append(paramQueries, + `ALTER SYSTEM SET shared_buffers = '8GB';`, + `ALTER SYSTEM SET effective_cache_size = '32GB';`, + `ALTER SYSTEM SET work_mem = '64MB';`, + `ALTER SYSTEM SET maintenance_work_mem = '1GB';`, + `ALTER SYSTEM SET temp_buffers = '128MB';`, + `ALTER SYSTEM SET random_page_cost = '4.0';`, + ) + } db, err := sql.Open("postgres", "postgres://postgres:postgres@127.0.0.1:5432/postgres?sslmode=disable") if err != nil { log.Fatalf("Failed to connect to embedded postgres: %v", err) From b38159716efcb251a8603c163fb25b628ecfb7b3 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 11 Dec 2025 17:47:15 +0000 Subject: [PATCH 2/6] restore ramdisk --- .github/workflows/ci.yaml | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index dcb783f01a66a..68494f3d21cc1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -363,6 +363,12 @@ jobs: sudo mdutil -X / sudo launchctl bootout system /System/Library/LaunchDaemons/com.apple.metadata.mds.plist + # Set up RAM disks to speed up the rest of the job. This action is in + # a separate repository to allow its use before actions/checkout. + - name: Setup RAM Disks + if: runner.os == 'Windows' + uses: coder/setup-ramdisk-action@e1100847ab2d7bcd9d14bcda8f2d1b0f07b36f1b # v0.1.0 + - name: Checkout uses: actions/checkout@1af3b93b6815bc44a9784bd300feb67ff0d1eeb3 # v6.0.0 with: @@ -420,11 +426,27 @@ jobs: source scripts/normalize_path.sh normalize_path_with_symlinks "$RUNNER_TEMP/sym" "$(dirname "$(which terraform)")" - - name: Setup macOS (install google-chrome, silence zsh warning) + - name: Setup RAM disk for Embedded Postgres (Windows) + if: runner.os == 'Windows' + shell: bash + # The default C: drive is extremely slow: + # https://github.com/actions/runner-images/issues/8755 + run: mkdir -p "R:/temp/embedded-pg" + + - name: Setup RAM disk for Embedded Postgres (macOS) if: runner.os == 'macOS' shell: bash run: | + # Postgres runs faster on a ramdisk on macOS. + mkdir -p /tmp/tmpfs + sudo mount_tmpfs -o noowners -s 8g /tmp/tmpfs + + # Install google-chrome for scaletests. + # As another concern, should we really have this kind of external dependency + # requirement on standard CI? brew install google-chrome + + # macOS will output "The default interactive shell is now zsh" intermittently in CI. touch ~/.bash_profile && echo "export BASH_SILENCE_DEPRECATION_WARNING=1" >> ~/.bash_profile - name: Test with PostgreSQL Database (Linux) @@ -454,6 +476,7 @@ jobs: test-count: ${{ github.ref == 'refs/heads/main' && '1' || '' }} # Only the CLI and Agent are officially supported on macOS; the rest are too flaky. test-packages: "./cli/... ./enterprise/cli/... ./agent/..." + embedded-pg-path: "/tmp/tmpfs/embedded-pg" embedded-pg-cache: ${{ steps.embedded-pg-cache.outputs.embedded-pg-cache }} - name: Test with PostgreSQL Database (Windows) @@ -469,6 +492,7 @@ jobs: test-count: ${{ github.ref == 'refs/heads/main' && '1' || '' }} # Only the CLI and Agent are officially supported on Windows; the rest are too flaky. test-packages: "./cli/... ./enterprise/cli/... ./agent/..." + embedded-pg-path: "R:/temp/embedded-pg" embedded-pg-cache: ${{ steps.embedded-pg-cache.outputs.embedded-pg-cache }} - name: Upload failed test db dumps From cb358eb11d0b0ebf7164973d47e8b8c78966011a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 11 Dec 2025 17:53:45 +0000 Subject: [PATCH 3/6] wip --- scripts/embedded-pg/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/embedded-pg/main.go b/scripts/embedded-pg/main.go index 470c837e4706f..6ea6fb89dd6a5 100644 --- a/scripts/embedded-pg/main.go +++ b/scripts/embedded-pg/main.go @@ -111,6 +111,8 @@ func main() { `ALTER SYSTEM SET max_connections = '1000';`, `ALTER SYSTEM SET client_encoding = 'UTF8';`, + + `ALTER SYSTEM SET random_page_cost = '1.0';`, } // Memory and I/O settings tuned per Depot runner specs: @@ -124,7 +126,6 @@ func main() { `ALTER SYSTEM SET work_mem = '32MB';`, `ALTER SYSTEM SET maintenance_work_mem = '512MB';`, `ALTER SYSTEM SET temp_buffers = '64MB';`, - `ALTER SYSTEM SET random_page_cost = '1.0';`, ) case "windows": paramQueries = append(paramQueries, @@ -133,7 +134,6 @@ func main() { `ALTER SYSTEM SET work_mem = '64MB';`, `ALTER SYSTEM SET maintenance_work_mem = '1GB';`, `ALTER SYSTEM SET temp_buffers = '128MB';`, - `ALTER SYSTEM SET random_page_cost = '4.0';`, ) } db, err := sql.Open("postgres", "postgres://postgres:postgres@127.0.0.1:5432/postgres?sslmode=disable") From c3dd69888602ba810f42e7d0ae78183576b020f4 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 11 Dec 2025 18:36:05 +0000 Subject: [PATCH 4/6] linux --- Makefile | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 4997430f9dd1b..94d325bb7ac60 100644 --- a/Makefile +++ b/Makefile @@ -1113,17 +1113,15 @@ test-postgres-docker: done } - # Make sure to not overallocate work_mem and max_connections as each - # connection will be allowed to use this much memory. Try adjusting - # shared_buffers instead, if needed. + # Optimized for CI: data lives on tmpfs (ramdisk) so durability + # settings don't matter, but we still tune for speed. # - # - work_mem=8MB * max_connections=1000 = 8GB - # - shared_buffers=2GB + effective_cache_size=1GB = 3GB - # - # This leaves 5GB for the rest of the system _and_ storing the - # database in memory (--tmpfs). - # - # https://www.postgresql.org/docs/current/runtime-config-resource.html#GUC-WORK-MEM + # Memory budget (16GB container limit): + # - shared_buffers: 2GB + # - wal_buffers: 64MB + # - work_mem: 8MB per-operation (can add up with parallel queries) + # - maintenance_work_mem: 256MB + # - Remainder for OS, tmpfs, and postgres per-connection overhead docker run \ --env POSTGRES_PASSWORD=postgres \ --env POSTGRES_USER=postgres \ @@ -1137,13 +1135,24 @@ test-postgres-docker: --memory 16GB \ ${POSTGRES_IMAGE} \ -c shared_buffers=2GB \ - -c effective_cache_size=1GB \ + -c effective_cache_size=4GB \ -c work_mem=8MB \ + -c maintenance_work_mem=256MB \ -c max_connections=1000 \ -c fsync=off \ -c synchronous_commit=off \ -c full_page_writes=off \ - -c log_statement=all + -c wal_level=minimal \ + -c max_wal_senders=0 \ + -c wal_buffers=64MB \ + -c checkpoint_timeout=30min \ + -c max_wal_size=4GB \ + -c min_wal_size=1GB \ + -c bgwriter_lru_maxpages=0 \ + -c autovacuum=off \ + -c jit=off \ + -c track_activities=off \ + -c track_counts=off while ! pg_isready -h 127.0.0.1 do echo "$(date) - waiting for database to start" From 01ae0653ad45fe98e3182020d06bd83b8486f02a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 11 Dec 2025 19:20:28 +0000 Subject: [PATCH 5/6] 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 656ba86af15ee58dfb35b334f09c96b6d09b490e Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 11 Dec 2025 19:34:01 +0000 Subject: [PATCH 6/6] order by name --- cli/templatepush_test.go | 13 +++++++------ coderd/database/queries.sql.go | 2 +- .../database/queries/templateversionvariables.sql | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go index 1e58713a24232..c12e2180d0c40 100644 --- a/cli/templatepush_test.go +++ b/cli/templatepush_test.go @@ -1106,21 +1106,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;