[testify-expert] Improve Test Quality: pkg/parser/github_host_test.go

[github-actions]

Summary

File: pkg/parser/github_host_test.go
Source pair: pkg/parser/github.go
Test functions: 1 (TestGetGitHubHostForRepo_PublicOrgFallback)
LOC: 60

Strengths

• Clean table-driven pattern with descriptive case names.
• Uses t.Setenv correctly for hermetic env isolation.

Prioritized Improvements

1. Missing tests — GetGitHubHost is entirely uncovered

GetGitHubHost reads four env vars in priority order and normalises the URL. None of its behaviour is tested. High-value cases:

Scenario	Input	Expected
GITHUB_SERVER_URL wins over others	GITHUB_SERVER_URL=(acme.ghe.com/redacted)	(acme.ghe.com/redacted)
Host without scheme gets https:// prefix	GITHUB_ENTERPRISE_HOST=acme.ghe.com	(acme.ghe.com/redacted)
Trailing slash stripped	GITHUB_HOST=acme.ghe.com/	(acme.ghe.com/redacted)
GH_HOST used when others are empty	GH_HOST=acme.ghe.com	(acme.ghe.com/redacted)
All vars empty → public fallback	(none set)	https://github.com

Before/After example

Before: no GetGitHubHost tests exist.

After:

func TestGetGitHubHost(t *testing.T) {
    tests := []struct {
        name     string
        envs     map[string]string
        expected string
    }{
        {
            name:     "GITHUB_SERVER_URL takes priority",
            envs:     map[string]string{"GITHUB_SERVER_URL": "(acme.ghe.com/redacted)"},
            expected: "(acme.ghe.com/redacted)",
        },
        {
            name:     "host without scheme gets https prefix",
            envs:     map[string]string{"GITHUB_ENTERPRISE_HOST": "acme.ghe.com"},
            expected: "(acme.ghe.com/redacted)",
        },
        {
            name:     "trailing slash stripped",
            envs:     map[string]string{"GITHUB_HOST": "acme.ghe.com/"},
            expected: "(acme.ghe.com/redacted)",
        },
        {
            name:     "GH_HOST used as last resort",
            envs:     map[string]string{"GH_HOST": "acme.ghe.com"},
            expected: "(acme.ghe.com/redacted)",
        },
        {
            name:     "defaults to public github when no env set",
            envs:     map[string]string{},
            expected: "https://github.com",
        },
        {
            name: "GITHUB_SERVER_URL shadows GITHUB_ENTERPRISE_HOST",
            envs: map[string]string{
                "GITHUB_SERVER_URL":      "(primary.ghe.com/redacted)",
                "GITHUB_ENTERPRISE_HOST": "(secondary.ghe.com/redacted)",
            },
            expected: "(primary.ghe.com/redacted)",
        },
    }

    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            for _, key := range []string{"GITHUB_SERVER_URL", "GITHUB_ENTERPRISE_HOST", "GITHUB_HOST", "GH_HOST"} {
                t.Setenv(key, "")
            }
            for k, v := range tt.envs {
                t.Setenv(k, v)
            }
            require.Equal(t, tt.expected, GetGitHubHost())
        })
    }
}

2. Testify assertion upgrade — assert → require in table loop

Fatal failures in one sub-test should not bleed into others; require.Equal short-circuits the current sub-test immediately.

Before:

assert.Equal(t, tt.expectedHost, host, "GetGitHubHostForRepo(%q, %q)", tt.owner, tt.repo)

After:

require.Equal(t, tt.expectedHost, host, "GetGitHubHostForRepo(%q, %q)", tt.owner, tt.repo)

Also add "github.com/stretchr/testify/require" to the import block (currently only assert is imported).

3. Missing edge cases for GetGitHubHostForRepo

The existing table omits:

• Empty gheHost: all env vars blank → should return https://github.com for any owner.
• Non-fallback owner with no GHE env: owner="acme", no env set → https://github.com.
• microsoft org explicitly: verify public host is used (no case present).

{
    name:         "empty gheHost falls back to public for non-fallback owner",
    owner:        "acme",
    repo:         "repo",
    gheHost:      "",
    expectedHost: "https://github.com",
},
{
    name:         "microsoft owner always uses public host",
    owner:        "microsoft",
    repo:         "vscode",
    gheHost:      "myorg.ghe.com",
    expectedHost: "https://github.com",
},

4. Build tag consistency

The file carries //go:build !integration but tests only env vars — no network or filesystem access. The tag may cause the tests to be excluded when running targeted unit builds without the integration tag cleared. Consider removing the tag or aligning with the naming conventions in neighbouring files that do need it (e.g., import_cache_integration_test.go).

Acceptance Checklist

• TestGetGitHubHost added covering priority order, scheme normalisation, trailing slash, and empty-env fallback.
• require.Equal used inside all sub-test loops.
• microsoft owner and empty-gheHost edge cases added to TestGetGitHubHostForRepo_PublicOrgFallback.
• Build tag reviewed — remove //go:build !integration if no integration resources are needed.
• make test-unit passes with no regressions.

Generated by 🧪 Daily Testify Uber Super Expert · 19.3 AIC · ⌖ 12.3 AIC · ⊞ 5.2K · ◷

• expires on Jul 22, 2026, 10:37 AM UTC-08:00

[github-actions]

🍪 Issue Monster selected this for Copilot

I've identified this issue as a good candidate for automated resolution and requested assignment to the Copilot coding agent.

If assignment succeeds, the Copilot coding agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• pi.dev

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

network:
  allowed:
    - defaults
    - "pi.dev"

See Network Configuration for more information.

🍪 Om nom nom by Issue Monster · 1.55 AIC · ⌖ 4.83 AIC · ⊞ 7.8K · ◷