Update unit tests so that GODEBUG=httpmuxgo121=1 is not required

[gmlewis]

Go version 1.22 introduced significant changes to the pattern syntax and matching
behavior of http.ServerMux which causes a large number of legacy unit tests to break.
(See https://pkg.go.dev/net/http#hdr-Compatibility-ServeMux for more information.)
As a result, testing of this repo is currently performed by setting this env variable:

export GODEBUG=httpmuxgo121=1

It would be greatly appreciated if an experienced Go developer would update this repo's
legacy unit tests so that go test ./... passes without the use of setting the GODEBUG
environment variable.

Ideally, no unit tests would need to be deleted, however it is possible that some tests
no longer make sense. If that is the case, it would be nice to see a detailed explanation
for any removed unit tests as to why it no longer makes sense to keep.

Thank you in advance for your contributions! See CONTRIBUTING.md for helpful details
for contributing to this repo.

[DiegoDev2]

Hello, do you have a deadline or something like that? @gmlewis

[gmlewis]

Hi @DiegoDev2 , no, I don't have a deadline... this would just be a nice-to-have.
Since you responded almost immediately, would you mind performing a code review of #3410 which is related?
Would you like me to assign this issue to you?

[DiegoDev2]

Yes, assign it to me, but something else is needed, I'm reviewing the PR #3410 @gmlewis

[gmlewis]

Yes, assign it to me, but something else is needed, I'm reviewing the PR #3410 @gmlewis

Thank you, @DiegoDev2 !
It's yours.

[DiegoDev2]

Well, excuse me, what files do I have to work on and were you part of the issue? @gmlewis

[DiegoDev2]

I was a little confused, but that's it. Don't worry.

[gmlewis]

I'm not sure I understand what you mean.

Are you clear now on what I'm asking to be done in the description above?

[gmlewis]

To hopefully be more clear, #3410 did not cause the problem.

Go 1.22 caused the problem.

We would like to keep this repo on the most two recent (minor) versions of Go without any odd workarounds.
To accomplish this, we first needed to write #3410 to move this repo to Go 1.22.10 (and Go 1.23.4 for the maintenance tools).
But in writing #3410, I came to the realization that we needed a workaround because of what is described above, and therefore I wrote this issue.
Does that help any?

[DiegoDev2]

When I ran the tests, I encountered the following errors:

go version:

go1.23.4 darwin/amd64

go test ./...
?   	github.com/google/go-github/v68/test/fields	[no test files]
?   	github.com/google/go-github/v68/test/integration	[no test files]
--- FAIL: TestRepositoriesService_GetBranchProtection_noDismissalRestrictions (0.01s)
    repos_test.go:1350: Repositories.GetBranchProtection returned error: GET http://127.0.0.1:54908/api-v3/repos/o/r/branches/feat%2Fbranch-50%25/protection: 404  []
    repos_test.go:1383: Repositories.GetBranchProtection returned <nil>, want &{RequiredStatusChecks:0xc0020693b0 RequiredPullRequestReviews:0xc0020693e0 EnforceAdmins:0xc002060ea0 Restrictions:0xc00012e460 RequireLinearHistory:<nil> AllowForcePushes:<nil> AllowDeletions:<nil> RequiredConversationResolution:<nil> BlockCreations:<nil> LockBranch:<nil> AllowForkSyncing:<nil> RequiredSignatures:<nil> URL:<nil>}
--- FAIL: TestRepositoriesService_GetBranch (0.01s)
    repos_test.go:940: Repositories.GetBranch returned error: unexpected status code: 404 Not Found
    repos_test.go:960: Repositories.GetBranch returned <nil>, want &{Name:0xc002061310 Commit:github.RepositoryCommit{SHA:"s", Commit:github.Commit{Message:"m"}} Protected:0xc0020f3898 Protection:0xc00201ff10}
--- FAIL: TestRepositoriesService_GetContents_DirectoryWithSpaces (0.00s)
--- FAIL: TestRepositoriesService_GetContents_DirectoryWithPlusChars (0.00s)
panic: parsing "/repos/o/r/contents/some directory/file.go": at offset 0: invalid method "/repos/o/r/contents/some" [recovered]
	panic: parsing "/repos/o/r/contents/some directory/file.go": at offset 0: invalid method "/repos/o/r/contents/some"

goroutine 4878 [running]:
testing.tRunner.func1.2({0x3a36820, 0xc001adc7e0})
	/usr/local/Cellar/go/1.23.4/libexec/src/testing/testing.go:1632 +0x230
testing.tRunner.func1()
	/usr/local/Cellar/go/1.23.4/libexec/src/testing/testing.go:1635 +0x35e
panic({0x3a36820?, 0xc001adc7e0?})
	/usr/local/Cellar/go/1.23.4/libexec/src/runtime/panic.go:785 +0x132
net/http.(*ServeMux).register(...)
	/usr/local/Cellar/go/1.23.4/libexec/src/net/http/server.go:2797
net/http.(*ServeMux).HandleFunc(0xc001530b60?, {0x37f50f4?, 0x605?}, 0x604?)
	/usr/local/Cellar/go/1.23.4/libexec/src/net/http/server.go:2771 +0x65
github.com/google/go-github/v68/github.TestRepositoriesService_GetContents_DirectoryWithSpaces(0xc001530b60)
	/Users/mac/programming/go-github/github/repos_contents_test.go:475 +0x8f
testing.tRunner(0xc001530b60, 0x3b00010)
	/usr/local/Cellar/go/1.23.4/libexec/src/testing/testing.go:1690 +0xf4
created by testing.(*T).Run in goroutine 1
	/usr/local/Cellar/go/1.23.4/libexec/src/testing/testing.go:1743 +0x390
FAIL	github.com/google/go-github/v68/github	0.510s

It looks like I need to work on the files where the errors are appearing, and I'll address any other issues that come up along the way.

Thanks in advance!

[gmlewis]

Yes, exactly right. If you decide that you are not interested in working on this issue, please let me know and I'm happy to unassign this issue and let another Go developer work on it.

[DiegoDev2]

Yes, exactly right. If you decide that you are not interested in working on this issue, please let me know and I'm happy to unassign this issue and let another Go developer work on it.

I am happy to solve the problem. Thanks for your explanation. And merry Christmas. I'm already working on it.

[DiegoDev2]

I have had to eliminate some cases from one of the tests. Therefore do I add a comment or where do I document where it was deleted? and excuse me. I ask to avoid problems for when it is delivered. @gmlewis

[gmlewis]

I have had to eliminate some cases from one of the tests. Therefore do I add a comment or where do I document where it was deleted?

Hmmm. It would be nice if we could have some community discussion around each case that needs deleting... and I've found that the easiest place to have those discussions is within the PR itself amidst the code.

Therefore, I recommend that you start out by simply commenting out the case itself and add a comment line before it with your argument as to why it needs to be removed. At that point we can discuss the removal. Finally, if there are no objections or proposed alternatives, we can clean up the code with full removal.

Please never use force-push while developing your PR so that everyone can see the full progression.

Does that make sense to you?

[DiegoDev2]

I have had to eliminate some cases from one of the tests. Therefore do I add a comment or where do I document where it was deleted?

Hmmm. It would be nice if we could have some community discussion around each case that needs deleting... and I've found that the easiest place to have those discussions is within the PR itself amidst the code.

Therefore, I recommend that you start out by simply commenting out the case itself and add a comment line before it with your argument as to why it needs to be removed. At that point we can discuss the removal. Finally, if there are no objections or proposed alternatives, we can clean up the code with full removal.

Please never use force-push while developing your PR so that everyone can see the full progression.

Does that make sense to you?

Alright. That seems perfect to me.