go-testing-env-contamination-missingkey
Root cause. TestNew_MissingKeyFails used t.Parallel() and assumed OPENROUTER_API_KEY was absent from the test environment. When the ambient shell exports it (CI, editors, dev machines — including this one), os.Getenv returns the real key, New() builds a live provider, and the test fails with New() = <nil> instead of ErrProviderNotConfigured — and any assertion passing would silently hit the live API. The test was environment-dependent, not hermetic.
Why t.Parallel() must be dropped (not just reordered). The pinning tool, t.Setenv, is incompatible with t.Parallel in any order — Go 1.26 panics with testing: test using t.Setenv, t.Chdir, or cryptotest.SetGlobalRandom can not use t.Parallel (verified both Parallel → Setenv and Setenv → Parallel). Additionally, os env is process-global, so two parallel siblings mutating it would race; serializing the missing-key tests eliminates that.
The fix (mirrors commit 0bd7363) — drop t.Parallel() and force the key to empty with t.Setenv:
package provider
import "testing"
// Fix (UHLP tick #131, commit 0bd7363):
// - t.Parallel() dropped: incompatible with t.Setenv (Go panics) and it let
// the process-global env race between sibling tests.
// - t.Setenv(envAPIKey, "") pins the variable to empty so the test is
// hermetic regardless of the ambient shell.
func TestNew_MissingKeyFails(t *testing.T) {
t.Setenv(envAPIKey, "") // empty => ErrProviderNotConfigured, never the live API
_, err := New()
if err != ErrProviderNotConfigured {
t.Fatalf("New() = %v, want ErrProviderNotConfigured", err)
}
}
// Positive companion: pins a fake key so it never reads the real ambient key.
// t.Setenv auto-restores the previous value on teardown.
func TestNew_WithKeySucceeds(t *testing.T) {
t.Setenv(envAPIKey, "sk-test-not-a-real-key-0000")
p, err := New()
if err != nil {
t.Fatalf("New() error = %v, want configured provider", err)
}
if p.APIKey() != "sk-test-not-a-real-key-0000" {
t.Fatalf("APIKey() = %q, want the pinned test key", p.APIKey())
}
}
Supporting change in provider.go: the key name lives in one constant (const envAPIKey = "OPENROUTER_API_KEY") so tests and production can never drift on the string.
Reproduction environment: Go 1.26.0, module `uhlp/tick131` at `/tmp/uhlptick131`. Critically, **this shell's ambient env has `OPENROUTER_API_KEY` set**, exactly the contamination the tick describes. **1. Contamination reproduced** — buggy test (`t.Parallel()`, no pin), ambient key set: ``` provider_bug_test.go:16: New() = <nil>, want ErrProviderNotConfigured (live API would be hit!) --- FAIL: TestNew_MissingKeyFails ``` **2. Same buggy test passes with `env -u OPENROUTER_API_KEY`** — proving it was environment-dependent, i.e. the contamination is ambient-sourced, not a code bug: ``` --- PASS: TestNew_MissingKeyFails ``` **3. Why the fix must drop `t.Parallel()`** — scratch test `t.Parallel(); t.Setenv(...)`: ``` panic: testing: test using t.Setenv, t.Chdir, or cryptotest.SetGlobalRandom can not use t.Parallel ``` Verified the reverse order (`Setenv` before `Parallel`) panics identically — the combination is forbidden, full stop. **4. Fixed suite — all ambient scenarios pass:** | Scenario | `go test -count=1 ./...` | |---|---| | Ambient key **SET** (the original failing case) | `ok` | | Ambient key **unset** (`env -u`) | `ok` | | Ambient key set to **empty string** (`OPENROUTER_API_KEY=`) | `ok` | **5. Race/flake check** — `go test -race -count=20 ./...` → `ok` (40 executions, no races, no flakes). Since the env-mutating tests are sequential and each `t.Setenv` restores the prior value, there is no cross-test leakage in either direction. **Edge cases covered:** key absent / key set / key empty-string; positive path with a fake key (never touches the live API); the `Parallel+Setenv` panic path (documented in test comments so future contributors don't reintroduce it).
{"model": "deepseek-v4-flash", "problem_class": "go-testing-env-contamination-missingkey", "result": "passed", "tests": 2}