From 66174223f0351b31c9b12198b70096c09b3ddf03 Mon Sep 17 00:00:00 2001 From: ZacxDev Date: Mon, 22 Jun 2026 16:30:26 -0500 Subject: [PATCH] fix(cli): make login tests hermetic (kill the 10-min network-flake) TestLoginEmptyTokenViaStdinFails was flaky and could time out at the 10-min Go test ceiling in CI. `civitai login` now DEFAULTS to the OAuth device flow: with no --token, RunE calls loginWithDevice -> StartDevice, a real network call to base_url. The test set stdin to "\n" expecting the OLD token-prompt "no token provided" error, but the default path ignores stdin. With no CIVITAI_BASE_URL override (only XDG_CONFIG_HOME was set), base_url defaulted to https://civitai.com and the test made a REAL production device-login attempt. When CI egress reached civitai.com, device-init succeeded and PollToken looped until the device-code expiry (~10 min) -> 10-min test timeout. When egress was blocked it failed in ~30s and passed. Hence the network-dependent flake. Replace it with TestLoginDeviceStartFailureReturnsError: stand up an httptest server whose device-init endpoint returns 4xx, pin the CLI at it via t.Setenv("CIVITAI_BASE_URL", srv.URL), run `login --no-browser`, and assert it errors fast (sub-second, no polling, never touching civitai.com). Asserts the device-init failure surfaces, nothing is persisted, and the poll endpoint is never reached. Audited the rest of internal/cmd and internal/api tests: all other HTTP-exercising tests already use httptest + CIVITAI_BASE_URL; this was the only test that could reach the real network. `go test ./...` no longer depends on outbound network. Proof: in an `unshare -rn` net namespace (empty route table, external TCP = "network is unreachable"), `go test ./internal/cmd/... ./internal/api/...` passes in ~2s (the 2s is two 1s device-flow interval sleeps against a LOCAL httptest server, not the network). No product code changed. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/cmd/cmd_test.go | 64 ++++++++++++++++++++++++++++++++++------ 1 file changed, 55 insertions(+), 9 deletions(-) diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index 059e5b3..903fa3c 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -2,12 +2,14 @@ package cmd import ( "bytes" + "encoding/json" "net/http" "net/http/httptest" "os" "path/filepath" "strings" "testing" + "time" ) // run executes the root command with args, capturing stdout+stderr. @@ -363,18 +365,62 @@ func TestLoginStoresToken(t *testing.T) { } } -func TestLoginEmptyTokenViaStdinFails(t *testing.T) { +// TestLoginDeviceStartFailureReturnsError drives the DEFAULT `civitai login` +// (no --token => OAuth device flow) against an httptest server whose device-init +// endpoint fails, and asserts login returns an error FAST. +// +// This replaces the old TestLoginEmptyTokenViaStdinFails, which assumed the +// default path read a token from stdin ("no token provided"). It no longer +// does: with no --token, login runs the device flow and makes a real network +// call to base_url. With no CIVITAI_BASE_URL override that call hit the REAL +// https://civitai.com — when egress was open, device-init succeeded and the +// poll looped until the device-code expired (~10 min), tripping Go's 10-min +// test timeout (network-dependent flake). Here we point the CLI at a local +// server (t.Setenv CIVITAI_BASE_URL = srv.URL) that returns 4xx on device-init, +// so StartDevice errors immediately, no polling, no real network, sub-second. +func TestLoginDeviceStartFailureReturnsError(t *testing.T) { + var gotDeviceInit bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Device-init is the first (and here, only) request the device flow makes. + if r.URL.Path == "/api/auth/oauth/device" { + gotDeviceInit = true + w.WriteHeader(http.StatusForbidden) + _ = json.NewEncoder(w).Encode(map[string]string{"error": "invalid_client"}) + return + } + // Reaching the poll endpoint would mean StartDevice wrongly succeeded. + t.Errorf("unexpected path %q — login should fail at device-init, never poll", r.URL.Path) + w.WriteHeader(http.StatusBadRequest) + })) + defer srv.Close() + dir := t.TempDir() t.Setenv("XDG_CONFIG_HOME", dir) t.Setenv("CIVITAI_TOKEN", "") - root := NewRootCmd() - var out, errb bytes.Buffer - root.SetOut(&out) - root.SetErr(&errb) - root.SetIn(strings.NewReader("\n")) // empty line at the prompt - root.SetArgs([]string{"login"}) - if err := root.Execute(); err == nil { - t.Fatal("expected error for empty token") + // Hermetic: pin the CLI at the local server so it NEVER touches civitai.com. + t.Setenv("CIVITAI_BASE_URL", srv.URL) + + start := time.Now() + // --no-browser so a failure can't spawn anything; the device flow still runs. + _, _, err := run(t, "login", "--no-browser") + elapsed := time.Since(start) + + if err == nil { + t.Fatal("expected an error when device-init fails") + } + if !strings.Contains(err.Error(), "device init failed") { + t.Errorf("error should report the device-init failure, got: %v", err) + } + if !gotDeviceInit { + t.Error("login should have hit the local device-init endpoint") + } + // Deterministic + fast: a clean device-init failure must not poll/wait. + if elapsed > 5*time.Second { + t.Errorf("login took %v — should fail fast without polling", elapsed) + } + // Nothing persisted on a failed login. + if _, statErr := os.Stat(filepath.Join(dir, "civitai", "config.yaml")); statErr == nil { + t.Error("no config should be written on a failed login") } }