diff --git a/AGENTS.md b/AGENTS.md index 4321ff0..4cffa00 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -120,6 +120,8 @@ license-compatible, no GPL/AGPL; name the one chosen, or why none fit. never poll the control plane or the LocalAPI in a tight loop. - Work consciously skipped is said out loud, never left as a TODO comment or as speculative code. +- Unit tests and e2e tests need to be included with all new functionality. + Test the expected ideal behavior, not the current implementation details. Being lazy about the solution is the goal. Being lazy about understanding it is not: trace the flow a change touches before picking an approach, because the diff --git a/Makefile b/Makefile index 7525a73..5f61486 100644 --- a/Makefile +++ b/Makefile @@ -1,14 +1,14 @@ .PHONY: build test lint check clean install BUILD_DATE := $(shell date -u +"%Y-%m-%dT%H:%M:%SZ") -GIT_HEIGHT := $(shell git rev-list --count HEAD 2>/dev/null || echo 0) +GIT_VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo dev) GIT_DESC := $(shell git describe --always) ifneq ($(shell git status --porcelain),) GIT_DESC := $(GIT_DESC)-dirty endif -LDFLAGS := -X main.buildVersion=B$(GIT_HEIGHT) -X main.buildCommit=$(GIT_DESC) -X main.buildDate=$(BUILD_DATE) +LDFLAGS := -X main.buildVersion=$(GIT_VERSION) -X main.buildCommit=$(GIT_DESC) -X main.buildDate=$(BUILD_DATE) build: go build -ldflags "$(LDFLAGS)" -o .build/aperture ./cmd/aperture diff --git a/cmd/aperture/main.go b/cmd/aperture/main.go index f37997e..b928cf3 100644 --- a/cmd/aperture/main.go +++ b/cmd/aperture/main.go @@ -47,8 +47,8 @@ func init() { } if buildVersion == "B0-dev" { - if height := gitCommitHeight(); height != "" { - buildVersion = "B" + height + if desc := gitDescribe(); desc != "" { + buildVersion = desc } else if info.Main.Version != "" && info.Main.Version != "(devel)" { buildVersion = info.Main.Version } @@ -79,14 +79,17 @@ func init() { } } -func gitCommitHeight() string { +// gitDescribe reports the release version of the checkout containing this +// source file: the tag on an exact release commit, or the nearest tag with +// the distance and commit appended. It returns "" outside a checkout. +func gitDescribe() string { _, file, _, ok := runtime.Caller(0) if !ok { return "" } for dir := filepath.Dir(file); ; dir = filepath.Dir(dir) { if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { - return gitCommitHeightInDir(dir) + return gitDescribeInDir(dir) } parent := filepath.Dir(dir) if parent == dir { @@ -95,23 +98,14 @@ func gitCommitHeight() string { } } -func gitCommitHeightInDir(dir string) string { - cmd := exec.Command("git", "rev-list", "--count", "HEAD") +func gitDescribeInDir(dir string) string { + cmd := exec.Command("git", "describe", "--tags", "--always", "--dirty") cmd.Dir = dir out, err := cmd.Output() if err != nil { return "" } - height := strings.TrimSpace(string(out)) - if height == "" { - return "" - } - for _, r := range height { - if r < '0' || r > '9' { - return "" - } - } - return height + return strings.TrimSpace(string(out)) } // startRunLog points slog at the run log and returns a function that closes diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go new file mode 100644 index 0000000..fbb9aac --- /dev/null +++ b/e2e/e2e_test.go @@ -0,0 +1,128 @@ +package e2e + +import ( + "io" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +var apertureBin string + +// TestMain builds the binary under test once. The tests exercise what we +// ship, so they run the real build rather than recompiling main's guts +// into the test binary. +func TestMain(m *testing.M) { + dir, err := os.MkdirTemp("", "aperture-e2e") + if err != nil { + panic(err) + } + apertureBin = filepath.Join(dir, "aperture") + build := exec.Command("go", "build", "-o", apertureBin, "../cmd/aperture") + if out, err := build.CombinedOutput(); err != nil { + panic(string(out)) + } + code := m.Run() + _ = os.RemoveAll(dir) + os.Exit(code) +} + +// modelsJSON is the smallest GET /v1/models payload that walks one provider +// through discovery: one provider, one model, one wire endpoint. With one +// of each, the launch flow asks no follow-up questions — selecting the +// client launches it. +const modelsJSON = `{ + "object": "list", + "data": [ + { + "id": "test-model", + "supported_endpoints": ["/v1/responses"], + "metadata": {"provider": {"id": "test-provider", "name": "Test Provider", "upstream": "test"}} + } + ] +}` + +// fakeAperture serves the discovery contract and nothing else. +func fakeAperture(t *testing.T) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/models" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, modelsJSON) + })) + t.Cleanup(srv.Close) + return srv +} + +func TestVersion(t *testing.T) { + stdout, _, err := run(t, apertureBin, hermeticEnv(t, ""), "-version") + if err != nil { + t.Fatalf("-version: %v", err) + } + if stdout == "" || stdout == "B0-dev" { + t.Errorf("version output = %q, want a release version", stdout) + } +} + +// A bad endpoint URL has to fail before the TUI takes the terminal: the +// script that passed it reads stderr and the exit code, not a painted error. +func TestBadEndpointExitsBeforeTUI(t *testing.T) { + _, stderr, err := run(t, apertureBin, hermeticEnv(t, ""), "-endpoint", "://nope") + exitErr, ok := err.(*exec.ExitError) + if !ok || exitErr.ExitCode() != 1 { + t.Fatalf("exit = %v, want exit code 1", err) + } + if !strings.Contains(stderr, "aperture:") { + t.Errorf("stderr = %q, want the failure reported", stderr) + } +} + +// The happy path: connect to a fake Aperture, pick the only installed +// client, and watch the launch reach the stub binary with the generated +// provider extension pointing back at the fake Aperture. +func TestLaunchPi(t *testing.T) { + srv := fakeAperture(t) + binDir := t.TempDir() + recordDir := installStubPi(t, binDir) + env := append(hermeticEnv(t, binDir), "APERTURE_E2E_RECORD="+recordDir) + + term := spawn(t, apertureBin, []string{"-endpoint", srv.URL}, env) + // With the stub as the only installed client, the picker opens with Pi + // as row [1] and no quick-select. + term.waitFor(t, "Which editor do you want to use?") + term.waitFor(t, "[1] Pi") + term.send("1") + + // One provider, one backend, one model: no follow-up menus, the + // selection launches straight into the stub. + argv := waitForFile(t, filepath.Join(recordDir, "argv")) + extension := waitForFile(t, filepath.Join(recordDir, "extension")) + + // "q" quits only from the root menu: a clean exit here also proves the + // TUI took the terminal back after the child exited. + term.send("q") + term.waitExit(t) + + if !strings.Contains(argv, "-e\n") { + t.Errorf("stub argv = %q, want the extension loaded with -e", argv) + } + if !strings.Contains(extension, srv.URL) { + t.Errorf("extension routes to %q, want the Aperture at %s", extension, srv.URL) + } +} + +// An unreachable endpoint paints the failure banner rather than hanging or +// exiting; the launcher stays up so the user can pick another endpoint. +func TestUnreachableEndpoint(t *testing.T) { + term := spawn(t, apertureBin, []string{"-endpoint", "http://127.0.0.1:1"}, hermeticEnv(t, "")) + term.waitFor(t, "Could not reach") + term.send("\x03") + term.waitExit(t) +} diff --git a/e2e/harness.go b/e2e/harness.go new file mode 100644 index 0000000..ece45ac --- /dev/null +++ b/e2e/harness.go @@ -0,0 +1,193 @@ +// Package e2e exercises the built aperture binary end to end: real process, +// real PTY, a fake Aperture over HTTP, and stub agent binaries on PATH. +// Nothing here touches the user's real home, config, or tailnet. +package e2e + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/charmbracelet/x/ansi" + "github.com/creack/pty" +) + +// terminal is a running aperture process attached to a PTY, with the output +// stream captured for assertions. +type terminal struct { + cmd *exec.Cmd + ptmx *os.File + + mu sync.Mutex + out bytes.Buffer +} + +// spawn starts bin on a PTY with args and env and begins capturing output. +func spawn(t *testing.T, bin string, args []string, env []string) *terminal { + t.Helper() + cmd := exec.Command(bin, args...) + cmd.Env = env + ptmx, err := pty.StartWithSize(cmd, &pty.Winsize{Rows: 40, Cols: 120}) + if err != nil { + t.Fatalf("spawning %s: %v", bin, err) + } + term := &terminal{cmd: cmd, ptmx: ptmx} + go func() { + var buf [4096]byte + for { + n, err := ptmx.Read(buf[:]) + if n > 0 { + term.mu.Lock() + term.out.Write(buf[:n]) + term.mu.Unlock() + } + if err != nil { + return + } + } + }() + return term +} + +// send types keys into the terminal. +func (term *terminal) send(keys string) { + if _, err := term.ptmx.WriteString(keys); err != nil { + // The process may have exited between the last waitFor and this + // send; waitExit reports the real outcome. + return + } +} + +// screen is everything the process has printed so far, escape sequences +// stripped, so assertions match words rather than terminal control bytes. +func (term *terminal) screen() string { + term.mu.Lock() + defer term.mu.Unlock() + return ansi.Strip(term.out.String()) +} + +// waitFor blocks until substr appears on screen, failing with the full +// screen after a generous timeout. The timeout covers bridge bring-up on a +// slow CI box; locally every screen arrives in milliseconds. +func (term *terminal) waitFor(t *testing.T, substr string) { + t.Helper() + deadline := time.Now().Add(15 * time.Second) + for time.Now().Before(deadline) { + if strings.Contains(term.screen(), substr) { + return + } + time.Sleep(20 * time.Millisecond) + } + t.Fatalf("timed out waiting for %q; screen:\n%s", substr, term.screen()) +} + +// waitExit expects the process to exit 0 within the timeout. +func (term *terminal) waitExit(t *testing.T) { + t.Helper() + done := make(chan error, 1) + go func() { done <- term.cmd.Wait() }() + select { + case err := <-done: + term.ptmx.Close() + if err != nil { + t.Fatalf("exit: %v; screen:\n%s", err, term.screen()) + } + case <-time.After(15 * time.Second): + _ = term.cmd.Process.Kill() + t.Fatalf("process still running; screen:\n%s", term.screen()) + } +} + +// run executes bin without a PTY and returns its streams. For the flag +// paths that resolve before the TUI takes the terminal. +func run(t *testing.T, bin string, env []string, args ...string) (string, string, error) { + t.Helper() + cmd := exec.Command(bin, args...) + cmd.Env = env + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err := cmd.Run() + return stdout.String(), stderr.String(), err +} + +// hermeticEnv builds the environment for one run: a throwaway home so +// config, state, the run log and generated extensions land in a temp dir, +// and a PATH of binDir plus the system dirs only. The system dirs keep git +// reachable for version stamping while hiding every agent binary the dev +// box happens to have installed, so the client picker shows exactly the +// stubs the test installed. +// +// TERM=dumb, not xterm: startup asks the terminal its color profile and a +// PTY never answers, which costs every run a five-second query timeout. +// Assertions strip escape sequences anyway, so color buys nothing here. +func hermeticEnv(t *testing.T, binDir string) []string { + t.Helper() + home := t.TempDir() + drop := map[string]bool{ + "HOME": true, "XDG_CONFIG_HOME": true, "TERM": true, "PATH": true, + "APERTURE_ENDPOINT": true, "APERTURE_BRIDGE": true, + } + var env []string + for _, e := range os.Environ() { + k, _, _ := strings.Cut(e, "=") + if !drop[k] { + env = append(env, e) + } + } + path := string(filepath.ListSeparator) + "/usr/bin" + string(filepath.ListSeparator) + "/bin" + if binDir != "" { + path = binDir + path + } + return append(env, + "HOME="+home, + "XDG_CONFIG_HOME="+filepath.Join(home, ".config"), + "TERM=dumb", + "PATH="+path, + ) +} + +// stubPi is a stand-in for the pi binary: it records its argv and +// environment, and captures the generated provider extension while it still +// exists — aperture removes the extension when the child exits. +const stubPi = `#!/bin/sh +printf '%s\n' "$@" > "$APERTURE_E2E_RECORD/argv" +env > "$APERTURE_E2E_RECORD/env" +prev="" +for a in "$@"; do + if [ "$prev" = "-e" ]; then + cat "$a" > "$APERTURE_E2E_RECORD/extension" + fi + prev="$a" +done +` + +// installStubPi writes the stub into binDir and returns the directory the +// stub records into. +func installStubPi(t *testing.T, binDir string) string { + t.Helper() + recordDir := t.TempDir() + if err := os.WriteFile(filepath.Join(binDir, "pi"), []byte(stubPi), 0o755); err != nil { + t.Fatal(err) + } + return recordDir +} + +// waitForFile blocks until path exists and is non-empty. +func waitForFile(t *testing.T, path string) string { + t.Helper() + deadline := time.Now().Add(15 * time.Second) + for time.Now().Before(deadline) { + if data, err := os.ReadFile(path); err == nil && len(data) > 0 { + return string(data) + } + time.Sleep(20 * time.Millisecond) + } + t.Fatalf("timed out waiting for %s", path) + return "" +} diff --git a/go.mod b/go.mod index 3a697de..0af0e28 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/charmbracelet/bubbletea v1.3.10 github.com/charmbracelet/lipgloss v1.1.0 github.com/charmbracelet/x/ansi v0.11.6 + github.com/creack/pty v1.1.24 golang.org/x/sys v0.47.0 tailscale.com v1.102.3 )