From c4b872c5ec54340b1e62d7578ace400340573ce2 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Sun, 26 Apr 2026 18:16:11 +0300 Subject: 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) --- cmd/hexai-mcp-server/main.go | 53 +++++++++++-------- cmd/hexai-mcp-server/main_test.go | 107 +++++++++++++++++++++++++++++--------- 2 files changed, 114 insertions(+), 46 deletions(-) (limited to 'cmd/hexai-mcp-server') diff --git a/cmd/hexai-mcp-server/main.go b/cmd/hexai-mcp-server/main.go index b32c18c..ac88178 100644 --- a/cmd/hexai-mcp-server/main.go +++ b/cmd/hexai-mcp-server/main.go @@ -29,10 +29,10 @@ var ( runBackfill = hexaimcp.RunBackfill ) -// printDeprecationWarning outputs a deprecation notice to stderr explaining -// that hexai-mcp-server is experimental and not actively maintained. -func printDeprecationWarning() { - warning := ` +// deprecationWarning is the notice runMain emits on every startup so users +// see this binary is experimental. Kept as a constant (not printf'd) so +// tests can assert on its contents directly. +const deprecationWarning = ` ⚠️ DEPRECATION NOTICE ⚠️ hexai-mcp-server is currently EXPERIMENTAL and NOT ACTIVELY MAINTAINED. @@ -50,8 +50,6 @@ Use at your own risk. ──────────────────────────────────────────────────────────────────────── ` - fmt.Fprintln(os.Stderr, warning) -} // mcpOptions holds the parsed command-line flags for the MCP server. type mcpOptions struct { @@ -64,22 +62,34 @@ type mcpOptions struct { showVersion bool } -func main() { - printDeprecationWarning() +func main() { os.Exit(runMain(os.Args[1:], os.Stdin, os.Stdout, os.Stderr)) } + +// runMain prints the deprecation warning, parses flags, and delegates to +// run. It returns the process exit code: 2 for flag-parse errors (matching +// stdlib `flag.ExitOnError`), 1 for state-dir or run failures, 0 on success. +// Pulling this out of main keeps it testable without touching package-level +// flag state. +func runMain(args []string, stdin io.Reader, stdout, stderr io.Writer) int { + fmt.Fprint(stderr, deprecationWarning) defaultLog, err := defaultLogPath() if err != nil { - fmt.Fprintf(os.Stderr, "error: %v\n", err) - os.Exit(1) + fmt.Fprintf(stderr, "error: %v\n", err) + return 1 + } + + fs := flag.NewFlagSet("hexai-mcp-server", flag.ContinueOnError) + fs.SetOutput(stderr) + logPath := fs.String("log", defaultLog, "path to log file (optional)") + configPath := fs.String("config", "", "path to config file (optional)") + promptsDir := fs.String("prompts-dir", "", "path to prompts directory (optional)") + slashCommandSync := fs.Bool("slashcommand-sync", false, "enable slash command sync") + slashCommandDir := fs.String("slashcommand-dir", "", "directory for slash command files") + syncAll := fs.Bool("sync-all", false, "backfill all existing prompts and exit") + showVersion := fs.Bool("version", false, "print version and exit") + if err := fs.Parse(args); err != nil { + return 2 } - logPath := flag.String("log", defaultLog, "path to log file (optional)") - configPath := flag.String("config", "", "path to config file (optional)") - promptsDir := flag.String("prompts-dir", "", "path to prompts directory (optional)") - slashCommandSync := flag.Bool("slashcommand-sync", false, "enable slash command sync") - slashCommandDir := flag.String("slashcommand-dir", "", "directory for slash command files") - syncAll := flag.Bool("sync-all", false, "backfill all existing prompts and exit") - showVersion := flag.Bool("version", false, "print version and exit") - flag.Parse() opts := mcpOptions{ logPath: *logPath, @@ -90,10 +100,11 @@ func main() { syncAll: *syncAll, showVersion: *showVersion, } - if err := run(opts, os.Stdin, os.Stdout, os.Stderr); err != nil { - fmt.Fprintf(os.Stderr, "error: %v\n", err) - os.Exit(1) + if err := run(opts, stdin, stdout, stderr); err != nil { + fmt.Fprintf(stderr, "error: %v\n", err) + return 1 } + return 0 } // run executes the MCP server logic with the given options and I/O streams. diff --git a/cmd/hexai-mcp-server/main_test.go b/cmd/hexai-mcp-server/main_test.go index 3d85fbb..33f662d 100644 --- a/cmd/hexai-mcp-server/main_test.go +++ b/cmd/hexai-mcp-server/main_test.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "io" - "os" "strings" "testing" @@ -12,31 +11,12 @@ import ( "codeberg.org/snonux/hexai/internal/hexaimcp" ) -func TestPrintDeprecationWarning(t *testing.T) { - r, w, err := os.Pipe() - if err != nil { - t.Fatalf("failed to create pipe: %v", err) - } - - oldStderr := os.Stderr - os.Stderr = w - defer func() { os.Stderr = oldStderr }() - - printDeprecationWarning() - - if err := w.Close(); err != nil { - t.Fatalf("failed to close pipe writer: %v", err) - } - - b, err := io.ReadAll(r) - if err != nil { - t.Fatalf("failed to read pipe: %v", err) - } - - output := string(b) +// The deprecation banner is unconditional: it must reach stderr on every +// invocation so users notice this binary is unmaintained. +func TestDeprecationWarning_Content(t *testing.T) { for _, want := range []string{"DEPRECATION NOTICE", "EXPERIMENTAL", "NOT ACTIVELY MAINTAINED"} { - if !strings.Contains(output, want) { - t.Errorf("expected %q in output, got %q", want, output) + if !strings.Contains(deprecationWarning, want) { + t.Errorf("expected %q in deprecationWarning", want) } } } @@ -168,3 +148,80 @@ func TestRun_MCPServerError(t *testing.T) { t.Fatalf("expected server error, got: %v", err) } } + +// runMain version path: -version must short-circuit and write the version +// to stdout (not stderr, so it stays scriptable). Stderr still gets the +// deprecation banner — that's correct since the binary is leaving anyway. +func TestRunMain_VersionFlag(t *testing.T) { + var stdout, stderr bytes.Buffer + code := runMain([]string{"-version"}, nil, &stdout, &stderr) + if code != 0 { + t.Fatalf("runMain code = %d, want 0", code) + } + if got := strings.TrimSpace(stdout.String()); got != internal.Version { + t.Fatalf("stdout = %q, want version %q", got, internal.Version) + } + if !strings.Contains(stderr.String(), "DEPRECATION NOTICE") { + t.Fatalf("stderr missing deprecation banner: %q", stderr.String()) + } +} + +// runMain --sync-all path: forwards parsed options to runBackfill and +// returns 0 on success. +func TestRunMain_SyncAllSuccess(t *testing.T) { + old := runBackfill + t.Cleanup(func() { runBackfill = old }) + + var gotLog string + runBackfill = func(logPath string, _ string, _ hexaimcp.MCPOverrides) error { + gotLog = logPath + return nil + } + + var stdout, stderr bytes.Buffer + code := runMain([]string{"-sync-all", "-log", "/tmp/sync.log"}, nil, &stdout, &stderr) + if code != 0 { + t.Fatalf("runMain code = %d, want 0; stderr=%q", code, stderr.String()) + } + if gotLog != "/tmp/sync.log" { + t.Fatalf("logPath forwarded = %q, want /tmp/sync.log", gotLog) + } +} + +// runMain run-error path: when the underlying server fails, runMain must +// return 1 (the production exit code) and write the error to stderr. +func TestRunMain_ServerErrorReturnsOne(t *testing.T) { + old := runMCP + t.Cleanup(func() { runMCP = old }) + runMCP = func(string, string, hexaimcp.MCPOverrides, io.Reader, io.Writer, io.Writer) error { + return errors.New("mcp boom") + } + + var stdout, stderr bytes.Buffer + code := runMain(nil, nil, &stdout, &stderr) + if code != 1 { + t.Fatalf("runMain code = %d, want 1", code) + } + if !strings.Contains(stderr.String(), "mcp boom") { + t.Fatalf("stderr missing error: %q", stderr.String()) + } +} + +// Bad flag must yield exit 2 without ever invoking the server stub. +func TestRunMain_BadFlagReturnsTwo(t *testing.T) { + old := runMCP + t.Cleanup(func() { runMCP = old }) + called := false + runMCP = func(string, string, hexaimcp.MCPOverrides, io.Reader, io.Writer, io.Writer) error { + called = true + return nil + } + var stdout, stderr bytes.Buffer + code := runMain([]string{"--bogus"}, nil, &stdout, &stderr) + if code != 2 { + t.Fatalf("runMain code = %d, want 2", code) + } + if called { + t.Fatal("runMCP must not be called on flag-parse failure") + } +} -- cgit v1.2.3