[test-improver] Improve tests for cmd/serve package

[github-actions]

Test Improvements: serve_test.go

File Analyzed

• Test File: internal/cmd/serve_test.go
• Package: internal/cmd
• Lines of Code: 52 → 124 (+72 lines)

Improvements Made

1. Better Testing Patterns

• ✅ Added descriptive test names following TestFunctionName_Scenario convention
• ✅ Used assert (non-fatal) for signal callback and error value checks, require (fatal) for setup and primary error checks
• ✅ Added explanatory comments documenting what each new test verifies

2. Increased Coverage

• ✅ Added TestServeAndWait_OnShutdownSignalCalled: exercises the if onShutdownSignal != nil { onShutdownSignal() } branch that was previously always skipped (existing test passed nil)
• ✅ Added TestServeAndWait_ServeFnError: exercises the error propagation path where serveFn returns an unexpected error (not http.ErrServerClosed), covering the cancel() call in the goroutine and the return serveErr branch
• Previous Coverage: serveAndWait 68.0%
• New Coverage: serveAndWait 84.0%
• Improvement: +16%

3. Cleaner & More Stable Tests

• ✅ TestServeAndWait_ServeFnError is fully synchronous — serveFn returns immediately, the goroutine inside serveAndWait calls cancel(), and the function returns quickly without timing dependencies
• ✅ TestServeAndWait_OnShutdownSignalCalled follows the same pattern as the existing graceful shutdown test, reusing the proven require.Eventually readiness check

Test Execution

All tests pass:

=== RUN   TestServeAndWait_GracefulShutdown
--- PASS: TestServeAndWait_GracefulShutdown (0.00s)
=== RUN   TestServeAndWait_OnShutdownSignalCalled
--- PASS: TestServeAndWait_OnShutdownSignalCalled (0.00s)
=== RUN   TestServeAndWait_ServeFnError
--- PASS: TestServeAndWait_ServeFnError (0.00s)
PASS
github.com/github/gh-aw-mcpg/internal/cmd/serve.go:13: serveAndWait 84.0%

Why These Changes?

serve_test.go was selected because it had only one test covering the happy path of serveAndWait (the core HTTP server lifecycle function), while two important branches had zero coverage:

1. The onShutdownSignal callback — always passed as nil in the existing test, so the if onShutdownSignal != nil branch was never executed
2. The unexpected serve error path — where a non-ErrServerClosed error from serveFn should trigger cancel() and be returned to the caller

Both gaps represent real production behavior worth verifying: the shutdown signal callback is used to notify callers when shutdown begins, and robust error propagation from the serve goroutine is critical for graceful error handling.

---

Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• index.crates.io

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "index.crates.io"

See Network Configuration for more information.

Generated by Test Improver · 1.1K AIC · ⊞ 29.5K · ◷

[Copilot]

Pull request overview

This PR enhances unit test coverage for the internal/cmd HTTP server lifecycle helper serveAndWait, focusing on previously untested branches to improve confidence in shutdown behavior and error propagation.

Changes:

• Adds a new test covering the onShutdownSignal callback branch during shutdown.
• Adds a new test covering the non-http.ErrServerClosed error path from serveFn.
• Updates the test file to use both assert and require for clearer test intent.

Show a summary per file

File	Description
internal/cmd/serve_test.go	Adds new unit tests to exercise shutdown callback invocation and unexpected serve error handling in serveAndWait.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 1/1 changed files
• Comments generated: 1