ci: make the e2e tests actually run

.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

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)
+			}
 		}
 	}
 

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)
+}

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 := `

internal/ui/widget_bash_e2e_test.go

@@ -4,7 +4,9 @@ import (
 	"io"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -26,25 +28,39 @@ 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()
 	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,
@@ -54,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) {
@@ -83,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())
 	}
 }

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

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"
 

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