Commit Graph

3 Commits

Author SHA1 Message Date
Bo 3bab897009 test(cli): migrate dedup_test.go to t.Chdir + lock isolation ratchet 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
2026-06-06 11:23:54 -04:00
Bo 7265fda1b1 chore(test): migrate redundant os.Setenv to t.Setenv; lower isolation ratchet 22->12 (ag-k38x #bulk-migration-setenv) (#772)
## 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
2026-06-06 10:17:21 -04:00
Bo 81aec49b1c chore(tests): test-isolation ratchet + migrate testutil chdir helpers to t.Chdir (ag-k38x #lint-ratchet) (#699)
## What
Lands the durable mechanism for ag-k38x (codebase-audit P2-1/P2-2): raw
`os.Chdir`/`os.Setenv` in tests mutate process-global cwd/env — a latent
flake landmine once any test opts into `t.Parallel` (cf. **ag-jfzs**,
the memory-sync flake of exactly this class).

The repo has ~163 `os.Chdir` + 22 `os.Setenv` inline sites across 34
files in **non-uniform patterns** (`defer` / `t.Cleanup` / `defer
func(){}()` with reversed order) — a blind sweep would break the suite.
So this ships the **durable gate + the canonical lever**, not a risky
80-site mass-edit:

- **`scripts/check-test-isolation.sh`** — a baseline ratchet (163/22)
wired into the **process-hygiene** job: raw `os.Chdir`/`os.Setenv` in
`*_test.go` may not **grow**; new code must use `t.Chdir`/`t.Setenv` or
the testutil helpers. This pre-empts the flake class from spreading.
- **`testutil_test.go`** — `chdirTemp`/`chdirTo` migrated from
`os.Chdir`+`t.Cleanup` to `t.Chdir` (the shared helpers most tests
delegate to; canonical example).

## Follow-up
The bulk inline-site migration drives the baseline toward zero
incrementally — **ag-k38x stays open** for it (scenario
`#bulk-migration`). Decomposing an ~80-site, non-uniform migration into
*gate-the-growth now + reduce-incrementally* is the responsible path; a
one-shot sed would be the reckless one.

## Verification
Ratchet PASSes at baseline AND catches a synthetic increase; shellcheck
+ `validate-ci-policy-parity` green; `go vet` clean; chdir-helper-caller
tests pass under `-race`.

Closes-scenario: ag-k38x#lint-ratchet
Bounded-context: BC2-Validation
Evidence: scripts/check-test-isolation.sh
2026-06-03 15:14:36 +00:00