From ae98293c375aa296a498790479ebc37249f7a309 Mon Sep 17 00:00:00 2001 From: Vladi Date: Mon, 7 Sep 2026 18:07:41 +0200 Subject: [PATCH 1/6] ci: build the binary so the pty e2e tests actually run --- .github/workflows/ci.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f57ce5b..d65ef24 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,8 +27,30 @@ jobs: - name: go vet run: go vet ./... + # The pty end-to-end tests drive the real binary as a subprocess + # and skip themselves when bin/azform is missing. Without this + # step they silently skipped on every CI run — internal/ui + # finished in ~1.4s instead of ~60s — so the widget round trip + # was never actually exercised here. + - name: build test binary + run: make build + + # ubuntu-latest is the runner where every e2e prerequisite is + # guaranteed: bash 5.x ships with the image and zsh is installed + # here. That lets AZFORM_E2E_REQUIRED=1 below turn any skip into + # a hard failure, so a green run means the tests really ran. + # + # macos-latest is deliberately left permissive: it ships bash + # 3.2, which cannot run the bash widget at all, so the bash e2e + # test must stay free to skip there. + - name: install zsh (e2e prerequisite) + if: matrix.os == 'ubuntu-latest' + run: sudo apt-get update && sudo apt-get install -y zsh + - name: test run: go test -race -count=1 ./... + env: + AZFORM_E2E_REQUIRED: ${{ matrix.os == 'ubuntu-latest' && '1' || '' }} lint: name: lint From 9d8e0c8d9daaedf3fe110a7aea5d153cf82971ce Mon Sep 17 00:00:00 2001 From: Vladi Date: Mon, 7 Sep 2026 18:07:41 +0200 Subject: [PATCH 2/6] test: fail instead of skip e2e tests when AZFORM_E2E_REQUIRED=1 --- internal/ui/e2e_require_test.go | 30 +++++++++++++++++++++++++++++ internal/ui/vardump_bash_test.go | 2 +- internal/ui/widget_bash_e2e_test.go | 4 ++-- internal/ui/widget_e2e_test.go | 8 ++++---- 4 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 internal/ui/e2e_require_test.go diff --git a/internal/ui/e2e_require_test.go b/internal/ui/e2e_require_test.go new file mode 100644 index 0000000..df5d69f --- /dev/null +++ b/internal/ui/e2e_require_test.go @@ -0,0 +1,30 @@ +package ui_test + +import ( + "os" + "testing" +) + +// e2eRequired reports whether the environment demands that end-to-end +// tests actually execute. CI sets AZFORM_E2E_REQUIRED=1 on the runner +// where every prerequisite is guaranteed present. +// +// Why this exists: the pty e2e tests guard themselves on external +// prerequisites (a built bin/azform, a zsh, a bash >= 4) and skip when +// one is missing. A skipped test reports as success, so for a long +// time CI was green while the widget round trip never ran at all — +// internal/ui completed in ~1.4s instead of ~60s and nobody noticed. +// Under this flag a missing prerequisite is a hard failure, so "green" +// means the tests really ran. +func e2eRequired() bool { return os.Getenv("AZFORM_E2E_REQUIRED") == "1" } + +// skipOrFail skips the test with reason, or fails it when +// AZFORM_E2E_REQUIRED=1. Local runs and contributor machines that lack +// zsh or a modern bash keep skipping politely; CI does not. +func skipOrFail(t *testing.T, reason string) { + t.Helper() + if e2eRequired() { + t.Fatalf("AZFORM_E2E_REQUIRED=1 but prerequisite missing: %s", reason) + } + t.Skip(reason) +} diff --git a/internal/ui/vardump_bash_test.go b/internal/ui/vardump_bash_test.go index 4c69b21..fcf3ef8 100644 --- a/internal/ui/vardump_bash_test.go +++ b/internal/ui/vardump_bash_test.go @@ -52,7 +52,7 @@ func TestBashDumpVarsFiltersTypes(t *testing.T) { t.Parallel() bash := bashAtLeast4(t) if bash == "" { - t.Skip("no bash >= 4 available") + skipOrFail(t, "no bash >= 4 available") } out := t.TempDir() + "/vars" script := ` diff --git a/internal/ui/widget_bash_e2e_test.go b/internal/ui/widget_bash_e2e_test.go index c631a9f..c7540d3 100644 --- a/internal/ui/widget_bash_e2e_test.go +++ b/internal/ui/widget_bash_e2e_test.go @@ -26,11 +26,11 @@ import ( func TestE2EBashWidgetEnvOut(t *testing.T) { bash := bashAtLeast4(t) if bash == "" { - t.Skip("no bash >= 4 available") + skipOrFail(t, "no bash >= 4 available") } bin := repoBinary(t) if bin == "" { - t.Skip("bin/azform not built; run `make build` first") + skipOrFail(t, "bin/azform not built; run `make build` first") } tmp := t.TempDir() diff --git a/internal/ui/widget_e2e_test.go b/internal/ui/widget_e2e_test.go index 3c95c28..d77932d 100644 --- a/internal/ui/widget_e2e_test.go +++ b/internal/ui/widget_e2e_test.go @@ -28,11 +28,11 @@ import ( // binary isn't built. func TestE2EWidgetEnvOutRoundTrip(t *testing.T) { if _, err := exec.LookPath("zsh"); err != nil { - t.Skip("zsh not on PATH") + skipOrFail(t, "zsh not on PATH") } bin := repoBinary(t) if bin == "" { - t.Skip("bin/azform not built; run `make build` first") + skipOrFail(t, "bin/azform not built; run `make build` first") } tmpDir := t.TempDir() @@ -162,11 +162,11 @@ print -r -- "newVar=$newVar"`) // var didn't survive azform exit". func TestE2ECancelFlushesEnvOut(t *testing.T) { if _, err := exec.LookPath("zsh"); err != nil { - t.Skip("zsh not on PATH") + skipOrFail(t, "zsh not on PATH") } bin := repoBinary(t) if bin == "" { - t.Skip("bin/azform not built; run `make build` first") + skipOrFail(t, "bin/azform not built; run `make build` first") } // Use a fixed prefix under the system temp dir so the From a1d21badc6bfa56440855587cefba2af364fd2e3 Mon Sep 17 00:00:00 2001 From: Vladi Date: Mon, 7 Sep 2026 18:07:41 +0200 Subject: [PATCH 3/6] test: actually unset AZFORM_* vars in the doctor override test --- cmd/azform/doctor_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/cmd/azform/doctor_test.go b/cmd/azform/doctor_test.go index 427210b..25d78d1 100644 --- a/cmd/azform/doctor_test.go +++ b/cmd/azform/doctor_test.go @@ -159,10 +159,23 @@ func TestDoctorStateNoOverrides(t *testing.T) { // Make sure unrelated env vars are NOT shown — only AZFORM_*. t.Setenv("SHELL", "") t.Setenv("PATH", "/bin:/usr/bin") + // Clear every AZFORM_* override so the report has none to list. + // t.Setenv alone is not enough: it sets the variable to the empty + // string rather than removing it, and reportState walks os.Environ() + // which still lists "NAME=". The t.Setenv call is what registers the + // original value for restoration at cleanup; os.Unsetenv then + // actually takes it out of the environment for this test. + // + // Without the Unsetenv, exporting any AZFORM_* variable in your + // shell — including the documented AZFORM_NO_UPDATE_CHECK — made + // this test fail on an otherwise healthy tree. for _, kv := range os.Environ() { if strings.HasPrefix(kv, "AZFORM_") { name := kv[:strings.IndexByte(kv, '=')] t.Setenv(name, "") + if err := os.Unsetenv(name); err != nil { + t.Fatalf("unset %s: %v", name, err) + } } } From bd38e4fb6e8b5cf6e373b583b4fbfac327ad2bc4 Mon Sep 17 00:00:00 2001 From: Vladi Date: Mon, 7 Sep 2026 18:11:54 +0200 Subject: [PATCH 4/6] test: resolve the test binary path absolutely in the bash e2e test --- internal/ui/widget_bash_e2e_test.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/internal/ui/widget_bash_e2e_test.go b/internal/ui/widget_bash_e2e_test.go index c7540d3..2acf6b4 100644 --- a/internal/ui/widget_bash_e2e_test.go +++ b/internal/ui/widget_bash_e2e_test.go @@ -4,6 +4,7 @@ import ( "io" "os" "os/exec" + "path/filepath" "strings" "testing" "time" @@ -36,15 +37,29 @@ func TestE2EBashWidgetEnvOut(t *testing.T) { tmp := t.TempDir() keep := tmp + "/env-out" rc := tmp + "/rc" - rcBody := "source widget/widget.bash\nPS1='PROMPT> '\n" + widgetPath, err := filepath.Abs(repoRoot(t) + "/widget/widget.bash") + if err != nil { + t.Fatalf("resolve widget path: %v", err) + } + rcBody := "source " + widgetPath + "\nPS1='PROMPT> '\n" if err := os.WriteFile(rc, []byte(rcBody), 0o600); err != nil { t.Fatalf("write rc: %v", err) } + // Put the freshly built binary first on PATH, resolved absolutely. + // $PWD is the *inherited* shell working directory, not the test's, + // so building a path from it is wrong — and locally it was masked + // by ~/.local/bin/azform from `make install`, meaning this test was + // silently exercising the installed binary rather than bin/azform. + binDir, err := filepath.Abs(filepath.Dir(bin)) + if err != nil { + t.Fatalf("resolve binary dir: %v", err) + } + cmd := exec.Command(bash, "--noprofile", "--rcfile", rc, "-i") cmd.Dir = repoRoot(t) cmd.Env = append(os.Environ(), - "PATH="+os.Getenv("PWD")+"/../../bin:"+os.Getenv("PATH"), + "PATH="+binDir+":"+os.Getenv("PATH"), "TERM=xterm-256color", "AZFORM_NO_UPDATE_CHECK=1", "AZFORM_ENV_OUT_KEEP="+keep, From ee78928c374431194496ac0c030e3b455708519c Mon Sep 17 00:00:00 2001 From: Vladi Date: Mon, 7 Sep 2026 18:15:25 +0200 Subject: [PATCH 5/6] test: capture pty output in the bash e2e test for diagnosis --- internal/ui/widget_bash_e2e_test.go | 33 ++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/internal/ui/widget_bash_e2e_test.go b/internal/ui/widget_bash_e2e_test.go index 2acf6b4..84bd9e9 100644 --- a/internal/ui/widget_bash_e2e_test.go +++ b/internal/ui/widget_bash_e2e_test.go @@ -6,6 +6,7 @@ import ( "os/exec" "path/filepath" "strings" + "sync" "testing" "time" @@ -69,7 +70,33 @@ func TestE2EBashWidgetEnvOut(t *testing.T) { t.Fatalf("pty start: %v", err) } defer func() { _ = f.Close() }() - go func() { _, _ = io.Copy(io.Discard, f) }() + + // Capture the pty stream instead of discarding it. The widget runs + // inside bash, so anything it or its helpers write to the terminal + // — command-not-found, mktemp errors, azform diagnostics — only + // surfaces here. Discarding it made a CI-only failure impossible to + // diagnose from the logs. + var mu sync.Mutex + var ptyOut strings.Builder + go func() { + buf := make([]byte, 4096) + for { + n, err := f.Read(buf) + if n > 0 { + mu.Lock() + ptyOut.Write(buf[:n]) + mu.Unlock() + } + if err != nil { + return + } + } + }() + dumpPty := func() string { + mu.Lock() + defer mu.Unlock() + return ptyOut.String() + } seq := func(s string, perByte, settle time.Duration) { for _, b := range []byte(s) { @@ -98,9 +125,9 @@ func TestE2EBashWidgetEnvOut(t *testing.T) { data, err := os.ReadFile(keep) if err != nil { - t.Fatalf("read env-out: %v", err) + t.Fatalf("read env-out: %v\n--- pty output ---\n%s\n--- end ---", err, dumpPty()) } if !strings.Contains(string(data), "bashVar='value1'") { - t.Fatalf("env-out missing queued var; got:\n%s", data) + t.Fatalf("env-out missing queued var; got:\n%s\n--- pty output ---\n%s\n--- end ---", data, dumpPty()) } } From 17a47fe5a2ffce2659843a01a207d96b3edf856c Mon Sep 17 00:00:00 2001 From: Vladi Date: Mon, 7 Sep 2026 18:19:35 +0200 Subject: [PATCH 6/6] widget: use a portable mktemp template so the widgets work on Linux --- widget/widget.bash | 12 +++++++++--- widget/widget.zsh | 12 +++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/widget/widget.bash b/widget/widget.bash index 0f514d8..3a00a34 100644 --- a/widget/widget.bash +++ b/widget/widget.bash @@ -61,9 +61,15 @@ fi azform-widget() { local out vars env line - out=$(mktemp -t azform-out) - vars=$(mktemp -t azform-vars) - env=$(mktemp -t azform-env) + # Explicit template rather than `mktemp -t azform-out`: BSD mktemp + # (macOS) treats -t's argument as a prefix and appends its own random + # suffix, but GNU coreutils (Linux) requires the template to contain + # at least three X's and errors with "too few X's in template", + # leaving the variable empty and every redirect below writing to "". + # This form is correct on both. + out=$(mktemp "${TMPDIR:-/tmp}/azform-out.XXXXXX") + vars=$(mktemp "${TMPDIR:-/tmp}/azform-vars.XXXXXX") + env=$(mktemp "${TMPDIR:-/tmp}/azform-env.XXXXXX") azform_bash_dump_vars "$vars" diff --git a/widget/widget.zsh b/widget/widget.zsh index a34800e..0020eb7 100644 --- a/widget/widget.zsh +++ b/widget/widget.zsh @@ -9,9 +9,15 @@ # via the g-popup land in your session until you unset them. azform-widget() { local out vars env - out=$(mktemp -t azform-out) - vars=$(mktemp -t azform-vars) - env=$(mktemp -t azform-env) + # Explicit template rather than `mktemp -t azform-out`: BSD mktemp + # (macOS) treats -t's argument as a prefix and appends its own random + # suffix, but GNU coreutils (Linux) requires the template to contain + # at least three X's and errors with "too few X's in template", + # leaving the variable empty and every redirect below writing to "". + # This form is correct on both. + out=$(mktemp "${TMPDIR:-/tmp}/azform-out.XXXXXX") + vars=$(mktemp "${TMPDIR:-/tmp}/azform-vars.XXXXXX") + env=$(mktemp "${TMPDIR:-/tmp}/azform-env.XXXXXX") # Denylist: zsh built-in specials + prompt/theme noise. RANDOM intentionally kept. local -A azform_deny=( SECONDS 1 EPOCHSECONDS 1 EPOCHREALTIME 1