Skip to content

[test-improver] Improve tests for proxy package#6770

Merged
lpcox merged 1 commit into
mainfrom
test-improver/upstreamhost-coverage-6d59a9112f9bd9cd
May 31, 2026
Merged

[test-improver] Improve tests for proxy package#6770
lpcox merged 1 commit into
mainfrom
test-improver/upstreamhost-coverage-6d59a9112f9bd9cd

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Test Improvements: forward_to_github_test.go

File Analyzed

  • Test File: internal/proxy/forward_to_github_test.go
  • Package: internal/proxy
  • Lines Added: +63

Improvements Made

1. Increased Coverage

  • ✅ Added TestUpstreamHost table-driven test covering all three parsing branches of Server.upstreamHost()
  • Previous Coverage: upstreamHost 37.5% (3/8 statements)
  • New Coverage: upstreamHost 100% (8/8 statements)
  • Package Improvement: 90.8% → 91.5% total

2. Better Testing Patterns

  • ✅ Table-driven test with 7 cases covering distinct code paths
  • ✅ Descriptive case names explaining why each case exercises a specific branch
  • ✅ Inline comments documenting which parsing strategy each case exercises

3. Coverage of All Three Parsing Strategies

The upstreamHost function uses three strategies in order:

  1. Full URL with scheme (https://api.github.com) — url.Parse returns non-empty Host directly
  2. Scheme-less hostname (api.github.com, api.github.com/api/v3) — first parse gives empty Host (treated as relative path), so the function prepends https:// and re-parses
  3. Unresolvable fallback (empty string) — both parse attempts yield empty Host, falls back to strings.Cut

Previously only the first strategy was exercised (by indirect coverage through forwardToGitHub tests using a httptest.Server URL).

Test Execution

All tests pass:

=== RUN   TestUpstreamHost
--- PASS: TestUpstreamHost (0.00s)
    --- PASS: TestUpstreamHost/full_URL_with_scheme_returns_hostname
    --- PASS: TestUpstreamHost/full_URL_with_port_strips_port_from_hostname
    --- PASS: TestUpstreamHost/http_URL_with_port_strips_port_from_hostname
    --- PASS: TestUpstreamHost/scheme-less_hostname_uses_second-parse_path
    --- PASS: TestUpstreamHost/scheme-less_hostname_with_path_strips_path
    --- PASS: TestUpstreamHost/leading_slashes_are_stripped_before_second_parse
    --- PASS: TestUpstreamHost/empty_URL_returns_empty_string_via_fallback
ok  github.com/github/gh-aw-mcpg/internal/proxy  0.075s  coverage: 91.5%

Why These Changes?

upstreamHost had 37.5% coverage despite being used in production code to populate the server.address OpenTelemetry attribute. The scheme-less and fallback paths — designed to handle common deployment configurations like api.github.com or GHES without an explicit https:// prefix — were completely untested. The new tests verify all three parsing strategies and document the intended behaviour for each URL format.


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 3.5M ·

Add table-driven tests for Server.upstreamHost() in forward_to_github_test.go,
covering all three parsing strategies:
- Full URL with scheme (fast path via url.Parse)
- Scheme-less hostname (fallback prepending https://)
- Empty/unresolvable URL (strings.Cut fallback)

Coverage improvement: upstreamHost 37.5% → 100%
Package total: 90.8% → 91.5%

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review May 31, 2026 13:16
Copilot AI review requested due to automatic review settings May 31, 2026 13:16
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 improves test coverage and clarity for Server.upstreamHost() in the internal/proxy package, ensuring all parsing branches are exercised and the expected hostname extraction behavior is documented through table-driven cases.

Changes:

  • Added a new table-driven TestUpstreamHost covering scheme URLs, scheme-less hostnames (with/without paths), and empty input fallback behavior.
  • Expanded branch coverage for upstreamHost() without introducing network dependencies or non-deterministic test behavior.
Show a summary per file
File Description
internal/proxy/forward_to_github_test.go Adds comprehensive table-driven coverage for Server.upstreamHost() hostname extraction branches.

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

@lpcox lpcox merged commit 976c008 into main May 31, 2026
23 checks passed
@lpcox lpcox deleted the test-improver/upstreamhost-coverage-6d59a9112f9bd9cd branch May 31, 2026 13:19
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