Skip to content

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

Description

@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

Activity

  1. github-actions commented on Jul 20, 2026

    @github-actions
    ContributorAuthor

    🍪 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 · ◷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions