chore(test): migrate redundant os.Setenv to t.Setenv; lower isolation ratchet 22->12 (ag-k38x #bulk-migration-setenv) by boshu2 · Pull Request #772 · boshu2/agentops

boshu2 · 2026-06-06T06:21:09Z

What

Advances the test-isolation ratchet (scripts/check-test-isolation.sh, shipped in #699) by driving the os.Setenv baseline from 22 → 12. Pure test-hygiene; no production-code change.

t.Setenv auto-restores on cleanup and fails fast under t.Parallel — the raw os.Setenv form is the latent-flake landmine the ratchet exists to retire (cf. ag-jfzs).

Sites migrated (10 eliminated)

File	Δ	How
`internal/config/config_test.go`	-6	Collapse the `origX := os.Getenv(...)` + `defer os.Setenv(... restore)` block into 3× `t.Setenv`
`internal/goals/measure_test.go`	-1	Delete the redundant `origPath`/`defer os.Setenv` — the existing `t.Setenv("PATH","")` already auto-restores
`cmd/ao/rpi_reliability_test.go`	-3	Delete 3 redundant restore-defers wrapped around existing `t.Setenv` calls

The remaining 12 are intentional (not migratable)

Documented inline in the ratchet script. They cannot use t.Setenv:

TestMain (cmd/ao/main_test.go, internal/doctor/doctor_testmain_test.go) — no *testing.T in scope (4)
String-literal fixtures (internal/doctor/fix_cliconfig_test.go) — os.Setenv appears inside generated test-program source, not a real call (2)
Unset-semantics helpers (cmd/ao/handoff_test.go, internal/lifecycle/repo_readiness_test.go, internal/wiki/locator_test.go) — these os.Unsetenv+restore vars; t.Setenv has no unset form (6)

Verification

bash scripts/check-test-isolation.sh → PASS … os.Setenv=12/12
go build ./... ✓ · go vet (touched pkgs) ✓
go test ./internal/config/ ./internal/goals/ → 526 passed; touched cmd/ao tests → 3 passed

Closes-scenario: ag-k38x#bulk-migration-setenv
Bounded-context: BC5-Runtime
Evidence: scripts/check-test-isolation.sh PASS 12/12

… ratchet 22->12 (ag-k38x #bulk-migration) Drive the test-isolation ratchet's os.Setenv baseline from 22 to 12 by converting the cleanly-migratable sites to t.Setenv (auto-restores, blocks t.Parallel): - config_test.go: collapse set+defer-restore block to 3x t.Setenv (-6) - measure_test.go: delete redundant origPath save+restore around existing t.Setenv (PATH already auto-restored) (-1) - rpi_reliability_test.go: delete 3 redundant restore-defers around existing t.Setenv calls (-3) The remaining 12 os.Setenv sites are intentional and cannot use t.Setenv: TestMain sites (main_test.go, doctor_testmain_test.go — no *testing.T), string-literal fixtures (fix_cliconfig_test.go), and unset-semantics helpers (handoff_test.go, repo_readiness_test.go, locator_test.go — t.Setenv cannot unset a var). Baseline constant lowered to lock the gain. Closes-scenario: ag-k38x#bulk-migration-setenv Bounded-context: BC5-Runtime Evidence: bash scripts/check-test-isolation.sh -> PASS 12/12; go build/vet/test green

…163->151 (ag-4nif #dedup-tchdir) Migrate dedup_test.go's 6 manual `os.Getwd()/os.Chdir/defer-restore` blocks to Go 1.20+ t.Chdir(tmp), which auto-restores cwd AND refuses t.Parallel — pre-empting the latent flake class flagged by ag-4nif/ag-k38x. No t.Parallel in the file; behavior-preserving. Each block held two `os.Chdir(` refs (the call + the defer), so 6 blocks drop the repo-wide test-isolation count by 12 (163->151). Lower BASELINE_CHDIR in scripts/check-test-isolation.sh to lock the gain (the ratchet is only-goes-down). Scope: os.Chdir only. The 3 redundant os.Setenv restores in rpi_reliability_test.go are already handled by the open PR #772 (ag-k38x), so they are deliberately NOT touched here to avoid a merge collision. ag-k38x is the canonical tracker; ag-4nif/ag-y4ac are its duplicates. Closes-scenario: ag-4nif#dedup-tchdir Bounded-context: BC5-Runtime Evidence: scripts/check-test-isolation.sh PASS (os.Chdir=151/151); go build ./... + go vet ./cmd/ao + go test ./cmd/ao -run 'TestRunDedup|TestMergeDedupGroups' (17 pass); gofmt clean

…e-tchdir-w2) (#786) ## What Collapse 7 identical `os.Getwd()` + `os.Chdir(tmpDir)` + `t.Cleanup(restore)` blocks in `cli/cmd/ao/curate_test.go` to `t.Chdir(tmpDir)`. Bare downstream `err =` first-uses promoted to `err :=` (the removed block held the only function-scope `err`). ## Why `t.Chdir` auto-restores cwd on cleanup **and** refuses to run under `t.Parallel` — pre-empting the process-global-cwd flake class (cf. ag-jfzs). This is one wave of the ag-k38x / ag-y4ac test-isolation sweep (`finish-the-job`: ~133 sites already use the safe idiom). ## Scope / collision-safety - `os.Chdir` in `_test.go`: **163 → 149 (-14)**. `os.Setenv` untouched (owned by in-flight setenv ratchet **PR #772**). - **`BASELINE_CHDIR` intentionally NOT lowered** — that single line is the contention point with in-flight **PR #772/#773**; the ratchet stays green because count-below-baseline is a NOTE, not a failure (`149 <= 163`). A later wave locks the baseline once those settle. - Non-overlapping with PR #772 (rpi setenv) and PR #773 (dedup_test.go os.Chdir). ## Verification - `gofmt -l` clean · `go build ./...` · `go vet ./cmd/ao` clean - `go test ./cmd/ao -run TestCurate` → 14 pass - `scripts/check-test-isolation.sh` → PASS (os.Chdir=149/163, os.Setenv=22/22) - `scripts/pre-push-gate.sh --fast` → passed Closes-scenario: ag-y4ac#curate-tchdir-w2 Bounded-context: BC5-Runtime Evidence: .ntm/bo-queue/2026-06-06-ag-y4ac-pr-curate-tchdir.md

…163->151 (ag-4nif #dedup-tchdir) (#773) ## What Migrate `dedup_test.go`'s 6 manual `origDir, _ := os.Getwd(); os.Chdir(tmp); defer os.Chdir(origDir)` blocks to Go 1.20+ `t.Chdir(tmp)` — auto-restores cwd **and** refuses `t.Parallel`, pre-empting the latent flake class flagged by **ag-4nif** / **ag-k38x** (raw forms mutate process-global cwd). No `t.Parallel` in the file; behavior-preserving. Each block held two `os.Chdir(` references (the call + the defer restore), so 6 blocks drop the repo-wide test-isolation count by **12 (163→151)**. I lowered `BASELINE_CHDIR` in `scripts/check-test-isolation.sh` to **lock the gain** (the ratchet is only-goes-down). ## Scope / no-collision note `os.Chdir` only. The 3 redundant `os.Setenv` restores in `rpi_reliability_test.go` are **already handled by the open PR #772 (ag-k38x)** — I deliberately do **not** touch that file here, to avoid a merge collision (an earlier draft of this branch did; it was reverted). **ag-k38x is the canonical tracker; ag-4nif/ag-y4ac are duplicates** — worth closing as dup-of-ag-k38x during triage. ## Evidence ``` bash scripts/check-test-isolation.sh # PASS (os.Chdir=151/151, os.Setenv=22/22) cd cli go build ./... # OK go vet ./cmd/ao/ # No issues found go test ./cmd/ao/ -run 'TestRunDedup|TestMergeDedupGroups' -count=1 # 17 pass gofmt -l # clean ``` Net: −19 / +8 across `dedup_test.go` + the ratchet baseline. Closes-scenario: ag-4nif#dedup-tchdir Bounded-context: BC5-Runtime Evidence: ratchet PASS + go build/vet/test (17 pass) + gofmt clean

github-actions Bot added the cli label Jun 6, 2026

This was referenced Jun 6, 2026

test(cli): migrate dedup_test.go to t.Chdir + lock isolation ratchet 163->151 (ag-4nif #dedup-tchdir) #773

Merged

test(cli): migrate curate_test.go os.Chdir to t.Chdir (ag-y4ac #curate-tchdir-w2) #786

Merged

boshu2 merged commit 7265fda into main Jun 6, 2026
16 checks passed

boshu2 deleted the chore/ag-k38x-isolation-wave-setenv branch June 6, 2026 14:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(test): migrate redundant os.Setenv to t.Setenv; lower isolation ratchet 22->12 (ag-k38x #bulk-migration-setenv)#772

chore(test): migrate redundant os.Setenv to t.Setenv; lower isolation ratchet 22->12 (ag-k38x #bulk-migration-setenv)#772
boshu2 merged 1 commit into
mainfrom
chore/ag-k38x-isolation-wave-setenv

boshu2 commented Jun 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

boshu2 commented Jun 6, 2026

What

Sites migrated (10 eliminated)

The remaining 12 are intentional (not migratable)

Verification

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant