Description
internal/github/oauth.go defines a package-level variable and exported accessor functions explicitly documented as a test seam for redirecting GitHub API calls to a mock server:
// Test helpers to override endpoints for mocking
var apiBase = "https://api.github.com"
// GetAPIBase returns the current API base URL (for tests).
func GetAPIBase() string {
return apiBase
}
// SetAPIBase sets the API base URL (for tests).
func SetAPIBase(base string) {
apiBase = base
}
But every actual API call in the package — GetRepo/GetRepoLanguages/GetReadme in repo.go, GetUser/GetUserEmails in api.go, CreateWebhook in webhook.go, AddIssueAssignees/RemoveIssueAssignees in assignees.go, CreateIssueComment/DeleteIssueComment/ListIssueComments in issues_comments.go/list.go, ListIssuesPage/ListPRsPage in list.go — hardcodes the literal string "https://api.github.com" directly rather than reading apiBase or calling GetAPIBase():
// internal/github/repo.go
u := "https://api.github.com/repos/" + url.PathEscape(owner) + "/" + url.PathEscape(repo)
grep-ing the entire codebase (including every test file) confirms apiBase/GetAPIBase/SetAPIBase are referenced only within oauth.go itself, where they're defined — nothing else in the repository ever calls SetAPIBase or reads GetAPIBase(). Its sibling, SetTokenEndpoint/tokenEndpoint, is genuinely wired into ExchangeCode and is used by real tests, which makes SetAPIBase especially misleading by proximity — it looks like the equivalent, working mechanism for redirecting the other ~10 API calls in the package to a mock server, but calling it silently does nothing. Any future test author who reasonably assumes github.SetAPIBase(mockServer.URL) will redirect GetRepo/GetUser/etc. calls to their httptest.Server will get a test that either makes real, uncontrolled network calls to api.github.com or hangs/fails in a confusing way, with no indication SetAPIBase was the reason.
Requirements
- Either
apiBase/GetAPIBase/SetAPIBase must actually control the base URL used by every API call in the package, or the dead accessors must be removed so they stop presenting as a working test seam.
Suggested execution
- Preferred: replace every hardcoded
"https://api.github.com" literal across repo.go, api.go, webhook.go, assignees.go, issues_comments.go, and list.go with GetAPIBase() (or a helper that reads apiBase), so SetAPIBase actually works as documented.
- Add a test that calls
SetAPIBase(mockServer.URL) and asserts at least one previously-hardcoded call (e.g. GetRepo) actually hits the mock server, proving the seam is now live — this test would fail against the current code.
- Restore
apiBase to "https://api.github.com" in a test cleanup/t.Cleanup to avoid cross-test pollution, consistent with how SetTokenEndpoint is already used.
Acceptance criteria
Guidelines
- Minimum 95% test coverage
- Timeframe: 96 hours
Description
internal/github/oauth.godefines a package-level variable and exported accessor functions explicitly documented as a test seam for redirecting GitHub API calls to a mock server:But every actual API call in the package —
GetRepo/GetRepoLanguages/GetReadmeinrepo.go,GetUser/GetUserEmailsinapi.go,CreateWebhookinwebhook.go,AddIssueAssignees/RemoveIssueAssigneesinassignees.go,CreateIssueComment/DeleteIssueComment/ListIssueCommentsinissues_comments.go/list.go,ListIssuesPage/ListPRsPageinlist.go— hardcodes the literal string"https://api.github.com"directly rather than readingapiBaseor callingGetAPIBase():grep-ing the entire codebase (including every test file) confirmsapiBase/GetAPIBase/SetAPIBaseare referenced only withinoauth.goitself, where they're defined — nothing else in the repository ever callsSetAPIBaseor readsGetAPIBase(). Its sibling,SetTokenEndpoint/tokenEndpoint, is genuinely wired intoExchangeCodeand is used by real tests, which makesSetAPIBaseespecially misleading by proximity — it looks like the equivalent, working mechanism for redirecting the other ~10 API calls in the package to a mock server, but calling it silently does nothing. Any future test author who reasonably assumesgithub.SetAPIBase(mockServer.URL)will redirectGetRepo/GetUser/etc. calls to theirhttptest.Serverwill get a test that either makes real, uncontrolled network calls toapi.github.comor hangs/fails in a confusing way, with no indicationSetAPIBasewas the reason.Requirements
apiBase/GetAPIBase/SetAPIBasemust actually control the base URL used by every API call in the package, or the dead accessors must be removed so they stop presenting as a working test seam.Suggested execution
"https://api.github.com"literal acrossrepo.go,api.go,webhook.go,assignees.go,issues_comments.go, andlist.gowithGetAPIBase()(or a helper that readsapiBase), soSetAPIBaseactually works as documented.SetAPIBase(mockServer.URL)and asserts at least one previously-hardcoded call (e.g.GetRepo) actually hits the mock server, proving the seam is now live — this test would fail against the current code.apiBaseto"https://api.github.com"in a test cleanup/t.Cleanupto avoid cross-test pollution, consistent with howSetTokenEndpointis already used.Acceptance criteria
SetAPIBasechanges the base URL used by all GitHub REST API calls in the package, not just a subset (or none).SetAPIBaseredirecting a real call (e.g.GetRepo) to a mock server.Guidelines