summaryrefslogtreecommitdiff
path: root/internal
diff options
context:
space:
mode:
authorPaul Buetow <paul@buetow.org>2026-04-26 18:16:11 +0300
committerPaul Buetow <paul@buetow.org>2026-04-26 18:16:11 +0300
commitc4b872c5ec54340b1e62d7578ace400340573ce2 (patch)
treed224707b3dd25ac68693d44f084dbe85a0edd685 /internal
parentf77d73244fa6585af0456fd374988b8b04b72646 (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.go69
-rw-r--r--internal/taskproxy/run_test.go155
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")
+ }
+}