Skip to content

[test-improver] Improve tests for httputil package#4383

Open
github-actions[bot] wants to merge 1 commit intomainfrom
test-improver/httputil-write-error-coverage-7e8706f027ff09e9
Open

[test-improver] Improve tests for httputil package#4383
github-actions[bot] wants to merge 1 commit intomainfrom
test-improver/httputil-write-error-coverage-7e8706f027ff09e9

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Test Improvements: httputil_test.go

File Analyzed

  • Test File: internal/httputil/httputil_test.go
  • Package: internal/httputil
  • Lines of Code: +62 lines added

Improvements Made

1. Increased Coverage

  • ✅ Added errorResponseWriter mock http.ResponseWriter to simulate write failures and short writes
  • ✅ Added test for write-error path (w.Write returns an error — previously unreachable)
  • ✅ Added test for short-write path (w.Write returns n < len(data) — previously unreachable)
  • Previous Coverage: WriteJSONResponse at 78.6% → package at 88.9%
  • New Coverage: WriteJSONResponse at 100% → package at 100.0%
  • Improvement: +11.1 percentage points (package), +21.4pp for WriteJSONResponse

2. Better Testing Patterns

  • ✅ Uses require.NotPanics to verify graceful error handling
  • ✅ Mock writer verifies actual byte-count and header state after error conditions
  • ✅ Follows existing subtests-inside-TestWriteJSONResponse pattern

3. Cleaner & More Stable Tests

  • ✅ Mock is self-contained in the test file (no new dependencies)
  • newErrorResponseWriter / newShortResponseWriter constructors keep test cases readable
  • ✅ Tests are deterministic and free of external dependencies

Test Execution

All httputil tests pass:

=== RUN   TestWriteJSONResponse/write_error_does_not_panic
--- PASS: TestWriteJSONResponse/write_error_does_not_panic (0.00s)
=== RUN   TestWriteJSONResponse/short_write_does_not_panic
--- PASS: TestWriteJSONResponse/short_write_does_not_panic (0.00s)
...
PASS
coverage: 100.0% of statements
ok      github.com/github/gh-aw-mcpg/internal/httputil  0.005s  coverage: 100.0% of statements

Note: A pre-existing failure in internal/config.TestFetchAndFixSchema_NetworkError (unrelated to this PR) exists on main and is not caused by these changes.

Why These Changes?

WriteJSONResponse had two error-handling branches that were never tested: the path where w.Write returns an error, and the path where it performs a short write. These are realistic failure modes in production (e.g., broken connections, buffers full). The missing coverage was identified via go test -coverprofile, and a small mock ResponseWriter was the minimal addition needed to exercise both paths without introducing new dependencies.


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:

  • invalidhostthatdoesnotexist12345.com

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

network:
  allowed:
    - defaults
    - "invalidhostthatdoesnotexist12345.com"

See Network Configuration for more information.

Generated by Test Improver · ● 2.5M ·

…esponse

Add a mock http.ResponseWriter (errorResponseWriter) and two subtests
to exercise the previously uncovered branches in WriteJSONResponse:
- write error path (w.Write returns an error)
- short write path (w.Write returns n < len(data))

Coverage for internal/httputil improved from 88.9% → 100%.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review April 23, 2026 16:44
Copilot AI review requested due to automatic review settings April 23, 2026 16:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR strengthens internal/httputil test coverage by adding deterministic tests that exercise previously untested error-handling branches in WriteJSONResponse, using a minimal custom http.ResponseWriter test double.

Changes:

  • Added a minimal errorResponseWriter to simulate Write errors and short writes.
  • Added subtests ensuring WriteJSONResponse does not panic on write failure or short-write conditions.
Show a summary per file
File Description
internal/httputil/httputil_test.go Adds a mock ResponseWriter plus new subtests for write-error and short-write branches in WriteJSONResponse.

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

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.

1 participant