feat: Ensure compatibility with encoding/json/v2 experiment by munlicode · Pull Request #4029 · google/go-github
This is a really solid, proactive update. Getting ahead of the Go 1.25 encoding/json/v2 experiment will definitely save us some headaches down the line.
Looking through the diff, a couple of things stand out. First, swapping map[any]any for func() in github_test.go is a super smart catch. Since v2 is way more flexible and can actually marshal map[any]any under the right conditions, using a function is basically a bulletproof way to guarantee an encoding failure across all JSON versions.
Second, shifting to strings.Contains() for the error assertions (like in security_advisories_test.go and repos_hooks_deliveries_test.go) is just good testing hygiene. Hardcoding exact stdlib error strings is a classic anti-pattern since the internal formatting shifts around all the time. Good use of short-circuiting there too (err == nil || !strings.Contains(...)) so we don't accidentally panic on a nil error.
I do have one tiny nitpick—completely non-blocking. In github/github_test.go, you yanked the type assertion check entirely. As it stands now, it just asserts that some error occurred. It's highly unlikely to fail for another reason here (since "GET" and "." are perfectly valid for NewRequest), but dropping the validation completely makes the test feel a little loose.
To keep it resilient but still focused, maybe just swap it for a quick string check? Something like: if err == nil { t.Error("Expected error to be returned.") } else if !strings.Contains(err.Error(), "unsupported") && !strings.Contains(err.Error(), "json") { t.Errorf("Expected a JSON marshaling error; got %v", err) }
Honestly though, it's a minor nit and not worth stalling the PR over unless we want to be hyper-strict on the assertions.
Overall—changes are clean, strictly confined to the test files so there's zero prod risk, and the CI scripts are perfectly green. Since gmlewis already gave the initial approval, I'm good with this. LGTM!