Skip to content

[test-improver] Improve tests for launcher/health_monitor#7124

Draft
github-actions[bot] wants to merge 1 commit into
mainfrom
test-improver/health-monitor-coverage-643c372f7938b290
Draft

[test-improver] Improve tests for launcher/health_monitor#7124
github-actions[bot] wants to merge 1 commit into
mainfrom
test-improver/health-monitor-coverage-643c372f7938b290

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented Jun 7, 2026

Test Improvements: health_monitor_test.go

File Analyzed

  • Test File: internal/launcher/health_monitor_test.go
  • Package: internal/launcher
  • Lines of Code: 141 → 219 (+78 lines)

Improvements Made

1. Increased Coverage

  • ✅ Added test for "reached max restart attempts" log path — exercises the inner if block inside handleErrorState that fires when a failed restart increments the counter to exactly maxConsecutiveRestartFailures.
  • ✅ Added test for successful restart — uses an httptest.Server speaking plain JSON-RPC so GetOrLaunch actually succeeds, covering the branch that resets consecutiveFailures to 0 and verifies the server transitions to "running".
Function Before After
handleErrorState 75.0% 100.0%
Package total 94.3% 95.4%

2. Cleaner Test Structure

  • ✅ Extracted mockMCPHandler as a reusable helper to avoid duplicating mock server setup in future tests.
  • ✅ Added require.Equal for the critical counter assertion in the max-failures test (stops the test immediately if the assertion fails, preventing confusing downstream failures).

Test Execution

All tests pass:

--- PASS: TestHealthMonitor_LogsErrorWhenMaxFailuresReached (0.00s)
--- PASS: TestHealthMonitor_SuccessfulRestartResetCounter (0.00s)
ok  github.com/github/gh-aw-mcpg/internal/launcher  17.376s  coverage: 95.4%

Why These Changes?

handleErrorState had two unreachable-from-existing-tests branches:

  1. The log emitted when a restart failure increments the counter to maxConsecutiveRestartFailures (not just above it). Existing tests either started at 0 (never reached max in one step) or started at max (early-return path, no restart attempted).

  2. The success path (consecutiveFailures = 0) which requires GetOrLaunch to actually return a connection. All existing tests used nonexistent-binary-xyz as the command, ensuring the restart always failed.

The new tests are deterministic, fast (sub-millisecond for the failure test; the HTTP test uses a local httptest.Server), and isolated.


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 · sonnet46 9.4M ·

…lure paths

Add two tests to health_monitor_test.go that cover the previously
uncovered branches in handleErrorState:

- TestHealthMonitor_LogsErrorWhenMaxFailuresReached: starts the
  consecutive-failure counter at maxConsecutiveRestartFailures-1 so
  the next failed restart pushes it exactly to the threshold,
  exercising the "reached max restart attempts" log path.

- TestHealthMonitor_SuccessfulRestartResetCounter: uses an in-process
  httptest.Server that speaks plain JSON-RPC to let GetOrLaunch
  succeed, covering the success branch that resets the counter to 0
  and transitions the server back to running state.

Also extracts mockMCPHandler as a reusable test helper and adds the
encoding/json, net/http, net/http/httptest imports required by the
new tests.

handleErrorState coverage: 75% → 100%
launcher package coverage: 94.3% → 95.4%

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants