Skip to content

Commit 6fd0413

Browse files
authored
test: raise codex package coverage to ≥80% (#24)
* test(procscan): cover IsAlive, FindChild, readStatus, isNumeric procscan had zero test coverage prior. Adds unit tests for both public entry points and the unexported helpers: - IsAlive: empty PID, non-numeric/non-positive PID, missing /proc/<pid>, and self-PID happy path - FindChild: empty-input guard and no-match scan - isNumeric: empty, all-digit, mixed, negative, non-ASCII rune cases - readStatus: self path success and missing-file fallthrough Brings package from 0.0% -> 88.9% statement coverage with no fakes beyond the running test process itself. * test(codex): cover process.go /proc scanner and discover edges process.go was at 0.0% coverage. Adds the same shape of unit tests as procscan_test.go for the codex-package copy (IsCodexAlive, FindCodexChild, isNumeric, readProcStatus), exercising the validation / missing-PID / self-PID happy paths. discover_extra_test.go fills three remaining gaps in discover.go: - dayDirsFor's midnight-clock-skew branch (00:02 UTC -> includes yesterday) and the outside-the-window negative branch (12:00 UTC, 00:05 UTC boundary) - newestMatchingRollout: directory-shaped-like-a-rollout-file is skipped via the IsDir() guard - newestMatchingRollout: non-.jsonl filename is skipped Brings package from 54.8% -> 92.6%. * test(health): cover CheckWorkdir branches CheckWorkdir is the pure-stdlib half of agent_check.go; the sibling CheckAgentProcess shells out to a live tmux server and walks /proc and is intentionally left to integration tests (it takes *tmux.Client by concrete type so no test seam is available without a production-code refactor outside the scope of this change). Covered branches: - empty workdir -> fail with 'not set' message + Fix hint - missing dir -> fail with 'does not exist' + mkdir Fix hint - file-not-dir -> fail with 'not a directory' - happy path -> pass with success message - stat error -> non-ENOENT path via unreadable parent (skipped on root) Brings agent_check.go coverage roughly half-of-file (CheckAgentProcess remains at 0%; documented above). * test(config,migrate): cover HookTimeout, malformed-input branches config/config.go: HookTimeout was uncovered. Adds tests for the zero/negative -> default fallback and the explicit-seconds passthrough. config.rewriteRequiredPathClaude: covers the early-return branches the v1->v2 migration step needs to be defensive about: - missing required_in_path key - empty raw value - malformed value (not a JSON array) -> step is no-op, not an error - non-string entries inside the array are passed through verbatim alongside the targeted 'claude' -> 'codex' rewrite migrate/migrate.go: adds two parser branches that were not exercised: - literal JSON 'null' parses into a nil map; the runner must re-init to {} and stamp schema_version, not panic - 'schema_version' present but not an integer -> clear parse error, not silent re-stamp config 75.0% -> 85.0%; migrate 81.4% -> 86.0%. * test(session): cover UpdateAgentSessionID idempotency, migration edges UpdateAgentSessionID's idempotent-noop branch (id already equals stored value) and not-found branch were the only uncovered branches. Adds happy-path, no-op, and missing-row tests. Migration step coverage: - stampAgentClaude with no 'sessions' key (early-return) - stampAgentClaude with a malformed 'sessions' blob (list instead of map) -> step surfaces a parse error - rewriteClaudeToCodex with no 'sessions' key (idempotency) Brings session package from 82.5% -> 84.4%.
1 parent 958348b commit 6fd0413

7 files changed

Lines changed: 655 additions & 0 deletions

File tree

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package codex
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
"time"
8+
)
9+
10+
// TestDayDirsFor_MidnightAddsPrevDay covers the clock-skew branch:
11+
// when t falls in the first 5 minutes of a UTC day, dayDirsFor should
12+
// also include the previous day's directory so a rollover-skewed file
13+
// is still found.
14+
func TestDayDirsFor_MidnightAddsPrevDay(t *testing.T) {
15+
root := "/codex/root"
16+
// 00:02 UTC on 2026-04-27 — well within the 5-minute window.
17+
t0 := time.Date(2026, 4, 27, 0, 2, 0, 0, time.UTC)
18+
dirs := dayDirsFor(root, t0)
19+
if len(dirs) != 2 {
20+
t.Fatalf("expected 2 dirs (today + previous), got %d: %v", len(dirs), dirs)
21+
}
22+
want0 := filepath.Join(root, "2026", "04", "27")
23+
want1 := filepath.Join(root, "2026", "04", "26")
24+
if dirs[0] != want0 {
25+
t.Errorf("dirs[0] = %q, want %q", dirs[0], want0)
26+
}
27+
if dirs[1] != want1 {
28+
t.Errorf("dirs[1] = %q, want %q", dirs[1], want1)
29+
}
30+
}
31+
32+
// TestDayDirsFor_OutsideMidnightWindow covers the negative branch of
33+
// the same conditional: at noon UTC only today's day-dir is returned.
34+
func TestDayDirsFor_OutsideMidnightWindow(t *testing.T) {
35+
root := "/codex/root"
36+
t0 := time.Date(2026, 4, 27, 12, 0, 0, 0, time.UTC)
37+
dirs := dayDirsFor(root, t0)
38+
if len(dirs) != 1 {
39+
t.Fatalf("expected 1 dir, got %d: %v", len(dirs), dirs)
40+
}
41+
}
42+
43+
// TestDayDirsFor_HourZeroPastFiveMinutes verifies the boundary: at
44+
// 00:05 UTC the previous-day include is gone (the conditional is
45+
// strict `< 5`).
46+
func TestDayDirsFor_HourZeroPastFiveMinutes(t *testing.T) {
47+
root := "/codex/root"
48+
t0 := time.Date(2026, 4, 27, 0, 5, 0, 0, time.UTC)
49+
dirs := dayDirsFor(root, t0)
50+
if len(dirs) != 1 {
51+
t.Fatalf("expected 1 dir at 00:05, got %d: %v", len(dirs), dirs)
52+
}
53+
}
54+
55+
// TestNewestMatchingRollout_DirectoryInDir covers the inner-loop
56+
// `e.IsDir() continue` branch: a sub-directory in the day-dir is
57+
// ignored even if its name superficially matches.
58+
func TestNewestMatchingRollout_DirectoryInDir(t *testing.T) {
59+
spawn := time.Now()
60+
day := fakeCodexHome(t, spawn)
61+
// Drop a directory shaped like a rollout file — must be skipped.
62+
rolloutDir := filepath.Join(day, "rollout-2026-05-14T12-00-00-deadbeef.jsonl")
63+
if err := os.MkdirAll(rolloutDir, 0o755); err != nil {
64+
t.Fatal(err)
65+
}
66+
id, _, ok := newestMatchingRollout(day, spawn)
67+
if ok {
68+
t.Fatalf("expected no match (only a dir present), got id=%q", id)
69+
}
70+
}
71+
72+
// TestNewestMatchingRollout_NonJsonlSkipped covers the
73+
// "name doesn't end with .jsonl" branch in newestMatchingRollout.
74+
func TestNewestMatchingRollout_NonJsonlSkipped(t *testing.T) {
75+
spawn := time.Now()
76+
day := fakeCodexHome(t, spawn)
77+
writeRollout(t, day, "rollout-2026-05-14T12-00-00-abc.txt", spawn.Add(50*time.Millisecond))
78+
id, _, ok := newestMatchingRollout(day, spawn)
79+
if ok {
80+
t.Fatalf("expected non-jsonl to be skipped, got id=%q", id)
81+
}
82+
}
83+
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
package codex
2+
3+
import (
4+
"os"
5+
"strconv"
6+
"testing"
7+
)
8+
9+
// process.go is a Linux-only /proc scanner. The tests here mirror the
10+
// shape of internal/procscan/procscan_test.go but target the codex copy
11+
// (which predates the shared procscan package and is still wired into
12+
// some callers).
13+
14+
func TestIsCodexAlive_EmptyPID(t *testing.T) {
15+
if _, err := IsCodexAlive(""); err == nil {
16+
t.Fatal("expected error on empty pid, got nil")
17+
}
18+
}
19+
20+
func TestIsCodexAlive_InvalidPID(t *testing.T) {
21+
// Same caveat as procscan: Sscanf accepts "12x" as 12, so the
22+
// branch we want is non-leading-digit + non-positive.
23+
for _, pid := range []string{"abc", "-1", "0", " "} {
24+
t.Run(pid, func(t *testing.T) {
25+
if _, err := IsCodexAlive(pid); err == nil {
26+
t.Fatalf("expected error for pid=%q", pid)
27+
}
28+
})
29+
}
30+
}
31+
32+
func TestIsCodexAlive_NonexistentPID(t *testing.T) {
33+
alive, err := IsCodexAlive("2147483647")
34+
if err != nil {
35+
t.Fatalf("expected nil err for absent pid: %v", err)
36+
}
37+
if alive {
38+
t.Fatal("expected alive=false for absent pid")
39+
}
40+
}
41+
42+
func TestIsCodexAlive_Self(t *testing.T) {
43+
pid := strconv.Itoa(os.Getpid())
44+
alive, err := IsCodexAlive(pid)
45+
if err != nil {
46+
t.Fatalf("unexpected err: %v", err)
47+
}
48+
if !alive {
49+
t.Fatalf("expected alive=true for self pid %s", pid)
50+
}
51+
}
52+
53+
func TestFindCodexChild_EmptyPID(t *testing.T) {
54+
if got := FindCodexChild(""); got != "" {
55+
t.Errorf("FindCodexChild(\"\") = %q, want \"\"", got)
56+
}
57+
}
58+
59+
// TestFindCodexChild_NoMatch exercises the full /proc scan returning ""
60+
// when no process matches the criteria. PPID "0" with no real codex
61+
// child guarantees nothing matches in CI.
62+
func TestFindCodexChild_NoMatch(t *testing.T) {
63+
if got := FindCodexChild("0"); got != "" {
64+
t.Errorf("FindCodexChild(0) = %q, want \"\"", got)
65+
}
66+
}
67+
68+
func TestIsNumeric_Codex(t *testing.T) {
69+
cases := []struct {
70+
in string
71+
want bool
72+
}{
73+
{"", false},
74+
{"0", true},
75+
{"12345", true},
76+
{"12a", false},
77+
{"-1", false},
78+
{"一", false},
79+
}
80+
for _, tc := range cases {
81+
if got := isNumeric(tc.in); got != tc.want {
82+
t.Errorf("isNumeric(%q) = %v, want %v", tc.in, got, tc.want)
83+
}
84+
}
85+
}
86+
87+
func TestReadProcStatus_Self(t *testing.T) {
88+
pid := strconv.Itoa(os.Getpid())
89+
ppid, procName, ok := readProcStatus("/proc/" + pid + "/status")
90+
if !ok {
91+
t.Fatal("readProcStatus on self returned ok=false")
92+
}
93+
if ppid == "" || procName == "" {
94+
t.Errorf("expected non-empty fields, got ppid=%q procName=%q", ppid, procName)
95+
}
96+
}
97+
98+
func TestReadProcStatus_MissingFile(t *testing.T) {
99+
_, _, ok := readProcStatus("/proc/2147483647/status")
100+
if ok {
101+
t.Error("expected ok=false for missing path")
102+
}
103+
}

internal/config/config_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,92 @@ func TestMigration_V1ToV2_Idempotent(t *testing.T) {
333333
}
334334
}
335335

336+
// TestHookTimeout_ZeroIsDefault and TestHookTimeout_ExplicitRespected
337+
// cover both branches of the HookTimeout duration resolver. Zero (and
338+
// negative) values fall back to the package default; positive seconds
339+
// are returned verbatim.
340+
func TestHookTimeout_ZeroIsDefault(t *testing.T) {
341+
for _, n := range []int{0, -3} {
342+
c := Config{HookTimeoutSec: n}
343+
want := time.Duration(DefaultHookTimeoutSec) * time.Second
344+
if got := c.HookTimeout(); got != want {
345+
t.Errorf("HookTimeout(%d) = %v, want %v", n, got, want)
346+
}
347+
}
348+
}
349+
350+
func TestHookTimeout_ExplicitRespected(t *testing.T) {
351+
c := Config{HookTimeoutSec: 42}
352+
if got := c.HookTimeout(); got != 42*time.Second {
353+
t.Errorf("HookTimeout(42) = %v, want 42s", got)
354+
}
355+
}
356+
357+
// TestRewriteRequiredPathClaude_MissingKey covers the early-return
358+
// branch when obj has no required_in_path key at all (or an empty raw
359+
// value). A v0/v1 config that never customized the list looks exactly
360+
// like this.
361+
func TestRewriteRequiredPathClaude_MissingKey(t *testing.T) {
362+
in := map[string]json.RawMessage{} // no required_in_path
363+
if err := rewriteRequiredPathClaude(in); err != nil {
364+
t.Fatalf("expected no-op nil, got %v", err)
365+
}
366+
if _, present := in["required_in_path"]; present {
367+
t.Error("step should not invent the key")
368+
}
369+
}
370+
371+
// TestRewriteRequiredPathClaude_MalformedArray covers the
372+
// "json.Unmarshal into []json.RawMessage failed" branch: the malformed
373+
// value is left untouched (jsonstrict will surface the error on the
374+
// typed Load that follows). The step must not error.
375+
func TestRewriteRequiredPathClaude_MalformedArray(t *testing.T) {
376+
in := map[string]json.RawMessage{
377+
"required_in_path": json.RawMessage(`"not-an-array"`),
378+
}
379+
original := in["required_in_path"]
380+
if err := rewriteRequiredPathClaude(in); err != nil {
381+
t.Fatalf("expected no-op on malformed value, got %v", err)
382+
}
383+
if string(in["required_in_path"]) != string(original) {
384+
t.Errorf("malformed value mutated: got %s", in["required_in_path"])
385+
}
386+
}
387+
388+
// TestRewriteRequiredPathClaude_NonStringEntries covers the inner-loop
389+
// branch where an individual entry is not a JSON string (defensive). It
390+
// is passed through verbatim.
391+
func TestRewriteRequiredPathClaude_NonStringEntries(t *testing.T) {
392+
in := map[string]json.RawMessage{
393+
"required_in_path": json.RawMessage(`["claude",123,{"x":1}]`),
394+
}
395+
if err := rewriteRequiredPathClaude(in); err != nil {
396+
t.Fatalf("step error: %v", err)
397+
}
398+
var list []json.RawMessage
399+
if err := json.Unmarshal(in["required_in_path"], &list); err != nil {
400+
t.Fatalf("unmarshal: %v", err)
401+
}
402+
if string(list[0]) != `"codex"` {
403+
t.Errorf("first entry (claude) not rewritten: %s", list[0])
404+
}
405+
if string(list[1]) != `123` {
406+
t.Errorf("non-string entry mutated: %s", list[1])
407+
}
408+
}
409+
410+
// TestRewriteRequiredPathClaude_EmptyRawValue covers the explicit
411+
// "raw exists but len==0" branch (defensive — Marshal would never
412+
// emit this, but a hand-edited config might).
413+
func TestRewriteRequiredPathClaude_EmptyRawValue(t *testing.T) {
414+
in := map[string]json.RawMessage{
415+
"required_in_path": json.RawMessage(``),
416+
}
417+
if err := rewriteRequiredPathClaude(in); err != nil {
418+
t.Fatalf("expected no-op, got %v", err)
419+
}
420+
}
421+
336422
// TestMigration_V1ToV2_FullPlanRewritesLegacyConfig verifies end-to-
337423
// end behavior via the migrate runner: a v1 config.json with
338424
// `claude` in required_in_path lands at v2 with `codex`, no backup.
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
package health
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
)
9+
10+
// CheckWorkdir is the pure-stdlib half of agent_check.go — its
11+
// CheckAgentProcess sibling shells out to a tmux server and walks
12+
// /proc, both of which are sandbox-hostile. CheckWorkdir is fully
13+
// unit-testable with t.TempDir.
14+
15+
func TestCheckWorkdir_Empty(t *testing.T) {
16+
r := CheckWorkdir("")
17+
if r.Passed() {
18+
t.Fatal("expected fail on empty workdir")
19+
}
20+
if r.Name != "workdir" {
21+
t.Errorf("Name = %q, want workdir", r.Name)
22+
}
23+
if !strings.Contains(r.Message, "not set") {
24+
t.Errorf("Message should mention not-set, got %q", r.Message)
25+
}
26+
if r.Fix == "" {
27+
t.Error("expected non-empty Fix hint for empty workdir")
28+
}
29+
}
30+
31+
func TestCheckWorkdir_Missing(t *testing.T) {
32+
missing := filepath.Join(t.TempDir(), "does-not-exist")
33+
r := CheckWorkdir(missing)
34+
if r.Passed() {
35+
t.Fatal("expected fail on missing dir")
36+
}
37+
if !strings.Contains(r.Message, "does not exist") {
38+
t.Errorf("Message should mention does-not-exist, got %q", r.Message)
39+
}
40+
if !strings.Contains(r.Fix, "mkdir") {
41+
t.Errorf("Fix should mention mkdir, got %q", r.Fix)
42+
}
43+
}
44+
45+
func TestCheckWorkdir_FileNotDir(t *testing.T) {
46+
dir := t.TempDir()
47+
file := filepath.Join(dir, "afile")
48+
if err := os.WriteFile(file, []byte("x"), 0o600); err != nil {
49+
t.Fatal(err)
50+
}
51+
r := CheckWorkdir(file)
52+
if r.Passed() {
53+
t.Fatal("expected fail on file-as-workdir")
54+
}
55+
if !strings.Contains(r.Message, "not a directory") {
56+
t.Errorf("Message should mention not-a-directory, got %q", r.Message)
57+
}
58+
}
59+
60+
func TestCheckWorkdir_Happy(t *testing.T) {
61+
dir := t.TempDir()
62+
r := CheckWorkdir(dir)
63+
if !r.Passed() {
64+
t.Fatalf("expected pass on real dir, got %+v", r)
65+
}
66+
if r.Status != StatusPass {
67+
t.Errorf("Status = %q, want %q", r.Status, StatusPass)
68+
}
69+
if !strings.Contains(r.Message, "exists and is a directory") {
70+
t.Errorf("Message should mention success, got %q", r.Message)
71+
}
72+
}
73+
74+
// TestCheckWorkdir_StatError exercises the "stat returned an error
75+
// that is not IsNotExist" branch. On Linux, attempting to stat a path
76+
// under an unreadable directory yields EACCES rather than ENOENT.
77+
//
78+
// Skips on root (root can read everything).
79+
func TestCheckWorkdir_StatError(t *testing.T) {
80+
if os.Geteuid() == 0 {
81+
t.Skip("running as root; cannot exercise permission-denied branch")
82+
}
83+
parent := filepath.Join(t.TempDir(), "locked")
84+
if err := os.Mkdir(parent, 0o700); err != nil {
85+
t.Fatal(err)
86+
}
87+
target := filepath.Join(parent, "child")
88+
if err := os.Mkdir(target, 0o700); err != nil {
89+
t.Fatal(err)
90+
}
91+
// Strip parent's execute bit so stat on child fails with EACCES.
92+
if err := os.Chmod(parent, 0o000); err != nil {
93+
t.Fatal(err)
94+
}
95+
t.Cleanup(func() { _ = os.Chmod(parent, 0o700) })
96+
97+
r := CheckWorkdir(target)
98+
if r.Passed() {
99+
t.Fatal("expected fail when stat returns non-NotExist error")
100+
}
101+
if !strings.Contains(r.Message, "error checking") {
102+
t.Errorf("Message should mention stat error, got %q", r.Message)
103+
}
104+
}

0 commit comments

Comments
 (0)