Skip to content

[test-improver] Improve tests for proxy handler#8251

Merged
lpcox merged 2 commits into
mainfrom
test-improver/proxy-handler-coverage-629249e9ab406404
Jun 29, 2026
Merged

[test-improver] Improve tests for proxy handler#8251
lpcox merged 2 commits into
mainfrom
test-improver/proxy-handler-coverage-629249e9ab406404

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Test Improvements: handler_test.go

File Analyzed

  • Test File: internal/proxy/handler_test.go
  • Package: internal/proxy
  • Lines Added: +80 lines (3 new tests + 1 helper type)

Improvements Made

1. Increased Coverage

Three previously uncovered code paths in handler.go are now exercised:

Function Before After
handleUnrecognizedPassthrough 0% 100%
forwardAndReadBody partial 100%
handleWithDIFC ~86% 95.1%
Package total 91.4% 92.2%

2. New Tests Added

TestForwardAndReadBody_BodyReadError

  • Covers lines 432–435 of handler.go: the io.ReadAll error path inside forwardAndReadBody
  • Uses a new bodyErrorTransport (custom http.RoundTripper) that returns a valid HTTP response whose body is an io.Pipe immediately closed with an error
  • Verifies the handler writes a 502 bad_gateway / "failed to read upstream response" JSON error

TestHandleUnrecognizedPassthrough_UpstreamFails

  • Covers lines 147–149 of handler.go: the if resp == nil { return } early exit in handleUnrecognizedPassthrough
  • Points the proxy at (127.0.0.1/redacted) (unreachable) with an unrecognized path /v1/unknown/endpoint`
  • Verifies the handler writes a 502 error and returns without panicking on nil response

TestHandleWithDIFC_RateLimitDetected

  • Covers lines 230–237 of handler.go: the rate-limit span annotation block inside handleWithDIFC
  • Upstream returns 429 Too Many Requests with X-Ratelimit-Remaining: 0 and a valid X-Ratelimit-Reset Unix timestamp
  • Exercises both the outer rateLimited branch and the inner !resetAt.IsZero() branch
  • Verifies the 429 is passed through and Retry-After is injected by writeResponse

3. Helper Type Added

bodyErrorTransport — a minimal http.RoundTripper that synthesises a response with a failing body reader (via io.Pipe). Clean, no network I/O required, no TCP hijacking.

Test Execution

ok  github.com/github/gh-aw-mcpg/internal/proxy  0.088s  coverage: 92.5% of statements

All 160+ tests across all packages pass.

Why These Changes?

handler.go had three distinct code paths that were entirely untested:

  1. The body-read failure in forwardAndReadBody — requires a transport-level mock since httptest.Server bodies never fail
  2. The nil-response early return in handleUnrecognizedPassthrough — requires an unreachable upstream for a non-DIFC path
  3. The rate-limit tracing annotations in handleWithDIFC — requires a 429 response through the full DIFC pipeline

All three paths are now covered, and the tests are self-contained, fast, and deterministic.


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 · 2K AIC · ⊞ 29.6K ·

…Passthrough, and rate-limit paths

- Add TestForwardAndReadBody_BodyReadError: exercises io.ReadAll error path
  (lines 432-435 of handler.go) using a custom http.RoundTripper that returns
  a valid response whose body pipe is closed with an error. forwardAndReadBody
  now reaches 100% coverage.

- Add TestHandleUnrecognizedPassthrough_UpstreamFails: exercises the early
  resp == nil return (lines 147-149 of handler.go) by pointing the proxy at
  an unreachable upstream for an unrecognized URL. handleUnrecognizedPassthrough
  now reaches 100% coverage.

- Add TestHandleWithDIFC_RateLimitDetected: exercises the rate-limit span
  annotation block in handleWithDIFC (lines 230-237 of handler.go) via an
  upstream 429 with X-Ratelimit-Reset header. handleWithDIFC coverage improves
  from 86% to 95.1%.

- Add bodyErrorTransport type (RoundTripper that pipes a read error) to support
  the BodyReadError test without any external setup or hijacking.

Overall proxy package coverage: 91.4% → 92.2%

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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

This PR improves unit test coverage for internal/proxy/handler.go by adding targeted tests that exercise previously-uncovered error and rate-limit branches in the proxy handler implementation.

Changes:

  • Added a bodyErrorTransport http.RoundTripper to simulate an upstream response whose body fails during io.ReadAll.
  • Added a test for forwardAndReadBody’s body-read error path and a test for DIFC rate-limit detection behavior.
  • Added a test intended to cover the resp == nil early return in handleUnrecognizedPassthrough when upstream forwarding fails.
Show a summary per file
File Description
internal/proxy/handler_test.go Adds helper transport and new tests to cover additional error/rate-limit branches in the proxy handler.

Review details

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment thread internal/proxy/handler_test.go Outdated
@lpcox

lpcox commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
GitHub Advanced Security started work on behalf of lpcox June 29, 2026 03:09 View session
GitHub Advanced Security finished work on behalf of lpcox June 29, 2026 03:10

Copilot AI commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

@copilot address review feedback

Done. The test now creates a real httptest.Server, records its URL, closes it immediately, and passes that closed URL as the upstream — ensuring the dial always fails deterministically. Commit 96edd30 has the change; all three new tests pass.

Copilot finished work on behalf of lpcox June 29, 2026 03:11
Copilot AI requested a review from lpcox June 29, 2026 03:11
@lpcox
lpcox merged commit 1c1ac57 into main Jun 29, 2026
24 checks passed
@lpcox
lpcox deleted the test-improver/proxy-handler-coverage-629249e9ab406404 branch June 29, 2026 03:12
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.

3 participants