diff options
| author | Paul Buetow <paul@buetow.org> | 2026-04-26 18:16:11 +0300 |
|---|---|---|
| committer | Paul Buetow <paul@buetow.org> | 2026-04-26 18:16:11 +0300 |
| commit | c4b872c5ec54340b1e62d7578ace400340573ce2 (patch) | |
| tree | d224707b3dd25ac68693d44f084dbe85a0edd685 /internal | |
| parent | f77d73244fa6585af0456fd374988b8b04b72646 (diff) | |
test: bring every package above the 80% coverage target
Per-package coverage was below the AGENTS.md target in six packages:
cmd/ask 0.0% -> 83.3%
cmd/hexai-tmux-edit 10.0% -> 93.3%
cmd/hexai-tmux-action 27.8% -> 95.7%
cmd/hexai-mcp-server 41.9% -> 88.2%
internal/taskproxy 61.8% -> 98.2%
internal/filelock 77.3% -> 100.0%
The four cmd packages each had a main() that mixed flag parsing, struct
construction, and runtime delegation, so nothing called from a test
hit those statements. Each main() is now a one-line wrapper around a
testable runMain(args, stdin, stdout, stderr) int that uses
flag.NewFlagSet (instead of the global flag.Parse) so tests can drive
it repeatedly. The deprecation banner in hexai-mcp-server is now a
package-level constant, kept identical, so tests can assert on it
directly without redirecting os.Stderr.
The internal packages got new tests for paths that were previously
unreachable: filelock's retry-then-success and non-EWOULDBLOCK error
branches, and taskproxy's NewRunner / findTaskBinary / detectRepoRoot /
runTaskCommand helpers (the ones that shell out to git and task).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Diffstat (limited to 'internal')
| -rw-r--r-- | internal/filelock/filelock_test.go | 69 | ||||
| -rw-r--r-- | internal/taskproxy/run_test.go | 155 |
2 files changed, 224 insertions, 0 deletions
diff --git a/internal/filelock/filelock_test.go b/internal/filelock/filelock_test.go index f1f5b65..a5ca520 100644 --- a/internal/filelock/filelock_test.go +++ b/internal/filelock/filelock_test.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "testing" + "time" ) func TestTryExclusive_SecondDescriptorWouldBlock(t *testing.T) { @@ -65,3 +66,71 @@ func TestAcquireExclusive_ContextCancelledWhileBlocked(t *testing.T) { t.Fatal(err) } } + +// Exercises the retry-then-success path inside AcquireExclusive: the lock is +// initially held, the helper releases it after a short wait, and the waiting +// caller must observe a Flock retry succeed and return a working unlock fn. +func TestAcquireExclusive_RetriesAndSucceeds(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "lock") + + fHeld, err := os.OpenFile(p, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = fHeld.Close() }) + if err := TryExclusive(fHeld); err != nil { + t.Fatal(err) + } + + fWait, err := os.OpenFile(p, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = fWait.Close() }) + + released := make(chan struct{}) + go func() { + time.Sleep(20 * time.Millisecond) + _ = UnlockExclusive(fHeld) + close(released) + }() + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + unlock, err := AcquireExclusive(ctx, fWait) + if err != nil { + t.Fatalf("AcquireExclusive = %v, want nil after retry", err) + } + <-released + if err := unlock(); err != nil { + t.Fatalf("unlock returned: %v", err) + } +} + +// Drives the non-EWOULDBLOCK error path: closing the file before calling +// AcquireExclusive yields EBADF, which must propagate up unchanged instead of +// being mapped to ErrWouldBlock or causing an infinite retry. +func TestAcquireExclusive_NonBlockingError_ReturnsImmediately(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "lock") + f, err := os.OpenFile(p, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + _, err = AcquireExclusive(ctx, f) + if err == nil { + t.Fatalf("AcquireExclusive on closed fd: got nil, want non-nil error") + } + if errors.Is(err, ErrWouldBlock) { + t.Fatalf("AcquireExclusive on closed fd: got ErrWouldBlock, want underlying syscall error") + } + if errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("AcquireExclusive on closed fd: got DeadlineExceeded, want immediate syscall error") + } +} diff --git a/internal/taskproxy/run_test.go b/internal/taskproxy/run_test.go index 15c5fbb..aa71023 100644 --- a/internal/taskproxy/run_test.go +++ b/internal/taskproxy/run_test.go @@ -5,7 +5,9 @@ import ( "context" "errors" "io" + "os" "os/exec" + "path/filepath" "reflect" "strings" "testing" @@ -139,3 +141,156 @@ func TestRunnerRun_EmptyRepoName_IsActionable(t *testing.T) { t.Fatalf("expected actionable project-name error, got %v", err) } } + +// NewRunner must wire all three function-typed fields and trim the command +// name; the rest of Runner relies on these defaults when callers don't +// override them in tests. +func TestNewRunner_DefaultsAreWiredAndCommandTrimmed(t *testing.T) { + r := NewRunner(" ask ") + if r.CommandName != "ask" { + t.Fatalf("CommandName = %q, want %q", r.CommandName, "ask") + } + if r.findTaskBinary == nil || r.detectRepoRoot == nil || r.runCommand == nil { + t.Fatalf("NewRunner did not wire all defaults: %+v", r) + } +} + +// normalizeRunner must fall back to "task" when CommandName is empty; the +// branch is otherwise unreachable through the public Run path because callers +// always pass a label. +func TestNormalizeRunner_FillsDefaultsForZeroValue(t *testing.T) { + got := normalizeRunner(Runner{}) + if got.CommandName != "task" { + t.Fatalf("CommandName = %q, want %q", got.CommandName, "task") + } + if got.findTaskBinary == nil || got.detectRepoRoot == nil || got.runCommand == nil { + t.Fatalf("normalizeRunner left a nil func field: %+v", got) + } + if label := got.commandLabel(); label != "task" { + t.Fatalf("commandLabel = %q, want %q", label, "task") + } +} + +// Whitespace-only CommandName must be treated as unset by commandLabel; this +// keeps error messages readable when callers accidentally pass " ". +func TestCommandLabel_TrimsToFallback(t *testing.T) { + r := Runner{CommandName: " "} + if got := r.commandLabel(); got != "task" { + t.Fatalf("commandLabel = %q, want %q", got, "task") + } +} + +// exitCodeFor must wrap non-ExitError failures (e.g. fork/exec errors) so the +// caller can distinguish "Taskwarrior ran and exited N" from "we never got to +// run Taskwarrior at all". +func TestExitCodeFor_NonExitError(t *testing.T) { + r := Runner{CommandName: "ask"} + code, err := r.exitCodeFor(errors.New("fork failed")) + if code != 1 { + t.Fatalf("exitCodeFor(non-ExitError) code = %d, want 1", code) + } + if err == nil || !strings.Contains(err.Error(), "failed to run Taskwarrior") { + t.Fatalf("expected wrap message, got %v", err) + } +} + +// findTaskBinary success path: stage a fake "task" executable on PATH and +// confirm LookPath resolves to it. +func TestFindTaskBinary_FoundInPath(t *testing.T) { + dir := t.TempDir() + fake := filepath.Join(dir, "task") + if err := os.WriteFile(fake, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", dir) + got, err := findTaskBinary() + if err != nil { + t.Fatalf("findTaskBinary: %v", err) + } + if got != fake { + t.Fatalf("findTaskBinary = %q, want %q", got, fake) + } +} + +// findTaskBinary failure path: an empty PATH must yield the actionable +// "install Taskwarrior" error users see when the binary is missing. +func TestFindTaskBinary_NotFound_IsActionable(t *testing.T) { + t.Setenv("PATH", "") + _, err := findTaskBinary() + if err == nil { + t.Fatalf("expected error for missing task binary") + } + if !strings.Contains(err.Error(), "install Taskwarrior and retry") { + t.Fatalf("error not actionable: %v", err) + } +} + +// detectRepoRoot must return the repo top level when invoked inside a real +// git checkout. Reuses the current process' git repository (this test file +// lives inside the hexai checkout) to avoid needing to git-init a temp dir. +func TestDetectRepoRoot_InsideGitRepo(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + root, err := detectRepoRoot(context.Background()) + if err != nil { + t.Fatalf("detectRepoRoot: %v", err) + } + if root == "" || !strings.Contains(root, "hexai") { + t.Fatalf("unexpected repo root %q", root) + } +} + +// detectRepoRoot outside a git repo: chdir into a temp dir that has no .git +// and confirm the actionable error fires. We restore cwd via t.Chdir which +// the test runner reverts automatically on test exit. +func TestDetectRepoRoot_OutsideGitRepo(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + dir := t.TempDir() + t.Chdir(dir) + t.Setenv("GIT_CEILING_DIRECTORIES", filepath.Dir(dir)) + _, err := detectRepoRoot(context.Background()) + if err == nil { + t.Fatalf("expected error outside git repo") + } + if !strings.Contains(err.Error(), "must be run inside a git repository") { + t.Fatalf("error not actionable: %v", err) + } +} + +// runTaskCommand must wire stdin/stdout/stderr to the spawned process. We +// invoke /bin/sh to echo from stdin and confirm both streams round-trip. +func TestRunTaskCommand_StreamsAndReturnNil(t *testing.T) { + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("sh not available") + } + var stdout, stderr bytes.Buffer + err := runTaskCommand( + context.Background(), + "sh", + []string{"-c", "cat; echo err 1>&2"}, + strings.NewReader("hello\n"), + &stdout, + &stderr, + ) + if err != nil { + t.Fatalf("runTaskCommand returned error: %v", err) + } + if got := stdout.String(); got != "hello\n" { + t.Fatalf("stdout = %q, want %q", got, "hello\n") + } + if got := stderr.String(); got != "err\n" { + t.Fatalf("stderr = %q, want %q", got, "err\n") + } +} + +// runTaskCommand surfaces the underlying exec error when the binary doesn't +// exist; Run() relies on this to map to a non-zero exit code. +func TestRunTaskCommand_BinaryNotFound(t *testing.T) { + err := runTaskCommand(context.Background(), "/no/such/binary", nil, nil, &bytes.Buffer{}, &bytes.Buffer{}) + if err == nil { + t.Fatalf("expected error from missing binary") + } +} |
