Skip to content

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

Merged
lpcox merged 3 commits into
mainfrom
test-improver/health-monitor-coverage-643c372f7938b290
Jun 7, 2026
Merged

[test-improver] Improve tests for launcher/health_monitor#7124
lpcox merged 3 commits into
mainfrom
test-improver/health-monitor-coverage-643c372f7938b290

Conversation

@github-actions

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

Copy link
Copy Markdown
Contributor

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>
@lpcox
lpcox marked this pull request as ready for review June 7, 2026 16:52
Copilot AI review requested due to automatic review settings June 7, 2026 16:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves unit test coverage for internal/launcher’s health monitor restart logic by adding new test cases that exercise additional branches in handleErrorState, plus a small helper for standing up an HTTP MCP mock.

Changes:

  • Added a test case that drives the restart-failure counter to the maximum threshold.
  • Added a test case that performs a successful HTTP restart and verifies the failure counter is reset and the server returns to running.
  • Extracted an HTTP JSON-RPC mock handler helper to reduce duplication.
Show a summary per file
File Description
internal/launcher/health_monitor_test.go Adds two new health monitor tests and a reusable HTTP MCP mock handler helper.

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: 2

Comment thread internal/launcher/health_monitor_test.go Outdated
Comment thread internal/launcher/health_monitor_test.go
lpcox and others added 2 commits June 7, 2026 10:02
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox
lpcox merged commit f4103fa into main Jun 7, 2026
16 checks passed
@lpcox
lpcox deleted the test-improver/health-monitor-coverage-643c372f7938b290 branch June 7, 2026 17:05
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.

2 participants