test: remove credential-gated GitHub tests superseded by contract tests

[jumboduck]

internal/github/github_test.go contained three test methods that called
testHelpers.SkipIfEnvVarUnset(t, []string{"KOSLI_GITHUB_TOKEN"}):

• TestPREvidenceForCommit
• TestPullRequestsForCommit
• TestGetPullRequestApprovers

These tests hit the real GitHub API and were silently skipped in any environment
without a token, giving false confidence in CI runs that lacked the credential.

Decision: delete the skipping tests, rely on fakes and contract tests

We removed all three methods and replaced the coverage with the
fake-and-contract pattern already established for AWS Lambda
(internal/aws/fake_lambda.go + internal/aws/lambda_contract_test.go).

Why the deleted tests were the wrong layer

TestPullRequestsForCommit and TestGetPullRequestApprovers tested
GithubConfig methods (PullRequestsForCommit, GetPullRequestApprovers)
that are internal implementation details — they are not on the
types.PRRetriever interface. Testing implementation details couples the test
suite to the HTTP client internals of go-github, requiring real credentials
just to verify plumbing that is already exercised indirectly by the interface
contract.

TestPREvidenceForCommit tested GithubConfig.PREvidenceForCommitV2, which
is on the interface — but the contract tests already cover it completely.

The pattern we follow

Layer	File	Always runs?
Interface	internal/types/types.go (PRRetriever)	—
Fake	internal/github/fake_github.go (FakeGitHubClient)	—
Contract (fake)	internal/github/github_contract_test.go (TestGitHubContract_Fake)	yes
Contract (real)	internal/github/github_contract_test.go (TestGitHubContract_RealGitHub)	no — skips without KOSLI_GITHUB_TOKEN
Factory injection	internal/github/github.go (NewGithubRetrieverFunc)	—

TestGitHubContract_Fake verifies the full V1/V2 contract — happy path,
unknown commit, error injection, ProviderAndLabel — without any network
access, every time the suite runs.

TestGitHubContract_RealGitHub runs the identical assertions against the live
GitHub API when KOSLI_GITHUB_TOKEN is present (nightly CI, developer
machines with the token set). This is the one legitimate place that requires
real credentials, exactly mirroring TestLambdaContract_RealAWS.

Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com

[claude]

Claude finished @jumboduck's task in 2m 21s —— View job

---

PR Review: Remove credential-gated GitHub tests

• Phase 1: Static analysis (go build, go vet, go mod tidy)
• Phase 2: Read and review changed files
• Phase 3: Post review feedback

---

Static Analysis

All checks pass — go build ./..., go vet ./..., and go mod tidy report no issues.

---

Review Summary

This is a clean, well-motivated PR. The deletion of 181 lines of credential-gated tests in internal/github/github_test.go is justified by the contract test coverage in github_contract_test.go, which was introduced in the parent PR (#807).

What's good

• Correct layering. TestPullRequestsForCommit and TestGetPullRequestApprovers tested internal GithubConfig methods not on the PRRetriever interface — removing them tightens the test surface to what matters. TestPREvidenceForCommitV2 is now fully covered by TestGitHubContract_Fake (always runs) and TestGitHubContract_RealGitHub (with token).
• Import cleanup is correct. The os and testHelpers imports were removed from github_test.go, and the remaining context import is still needed by TestNewGithubClientFromToken.
• Dynamic commit resolution in attestPRGithub_test.go. Replacing hardcoded SHAs (480e5a..., 7d1db1...) with gitview.ResolveRevision("HEAD") / "HEAD~1" eliminates brittleness — the test no longer breaks if those specific commits are force-pushed away or the repo is shallow-cloned in CI.

One observation (non-blocking)

In attestPRGithub_test.go:29, the gitview.New("../..") path assumes the test's working directory is always cmd/kosli/. This is safe for go test ./cmd/kosli/... (Go sets the working directory to the package under test), so it works correctly. Just noting the implicit assumption for context — no change needed.

Verdict

No issues found. The PR correctly follows the fake-and-contract pattern established for AWS Lambda. The deleted tests are fully superseded and the attestPRGithub_test.go change is a clear improvement to test stability.