Skip to content

Commit aab9175

Browse files
committed
test: add regression test for nil invitation on 204 response
Covers the v6.12.0 panic scenario by standing up an httptest server that returns 204 No Content from PUT /repos/{owner}/{repo}/collaborators/{username} and driving updateUserCollaboratorsAndInvites against it. Asserts the function completes without panicking and records no invitation. Follow-up to @deiga's review on #3371.
1 parent f63aae6 commit aab9175

1 file changed

Lines changed: 98 additions & 0 deletions

File tree

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
package github
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"net/url"
8+
"testing"
9+
10+
"github.com/google/go-github/v85/github"
11+
)
12+
13+
// TestUnitUpdateUserCollaboratorsAndInvites_NilInvitationFor204 is the
14+
// regression guard for the v6.12.0 panic: when GitHub's
15+
// PUT /repos/{owner}/{repo}/collaborators/{username} returns 204 No Content
16+
// (the invitee is an organization member gaining direct access without an
17+
// invitation), the go-github client surfaces that as (*Invitation == nil,
18+
// err == nil). Prior to the fix, updateUserCollaboratorsAndInvites
19+
// unconditionally dereferenced inv.ID and panicked with SIGSEGV. This test
20+
// stands up a fake API returning exactly that response shape and asserts
21+
// the function completes without panicking and records no invitation.
22+
func TestUnitUpdateUserCollaboratorsAndInvites_NilInvitationFor204(t *testing.T) {
23+
const (
24+
owner = "testowner"
25+
repo = "testrepo"
26+
login = "orgmember"
27+
)
28+
29+
collaboratorsPath := "/repos/" + owner + "/" + repo + "/collaborators"
30+
invitationsPath := "/repos/" + owner + "/" + repo + "/invitations"
31+
putCollaboratorPath := collaboratorsPath + "/" + login
32+
33+
var (
34+
listCollaboratorsCalls int
35+
listInvitationsCalls int
36+
putCollaboratorCalls int
37+
)
38+
39+
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
40+
w.Header().Set("Content-Type", "application/json")
41+
42+
switch {
43+
case r.Method == http.MethodGet && r.URL.Path == collaboratorsPath:
44+
listCollaboratorsCalls++
45+
_, _ = w.Write([]byte(`[]`))
46+
47+
case r.Method == http.MethodGet && r.URL.Path == invitationsPath:
48+
listInvitationsCalls++
49+
_, _ = w.Write([]byte(`[]`))
50+
51+
case r.Method == http.MethodPut && r.URL.Path == putCollaboratorPath:
52+
putCollaboratorCalls++
53+
// Mirror GitHub's behavior for an existing org member: 204 with
54+
// no body. go-github decodes this into (nil, resp, nil).
55+
w.WriteHeader(http.StatusNoContent)
56+
57+
default:
58+
t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
59+
w.WriteHeader(http.StatusTeapot)
60+
}
61+
}))
62+
defer ts.Close()
63+
64+
client := github.NewClient(nil)
65+
base, err := url.Parse(ts.URL + "/")
66+
if err != nil {
67+
t.Fatalf("parse test server URL: %v", err)
68+
}
69+
client.BaseURL = base
70+
71+
inUsers := userCollaborators{{
72+
userIdentity: userIdentity{login: login},
73+
permission: "admin",
74+
}}
75+
76+
ghInvites, err := updateUserCollaboratorsAndInvites(context.Background(), client, owner, repo, inUsers)
77+
if err != nil {
78+
t.Fatalf("updateUserCollaboratorsAndInvites returned error: %v", err)
79+
}
80+
81+
// 204 means no invitation was issued, so nothing should be tracked.
82+
if len(ghInvites) != 0 {
83+
t.Errorf("expected 0 tracked invitations on 204 response, got %d: %+v",
84+
len(ghInvites), ghInvites)
85+
}
86+
87+
// Sanity check the full flow executed end-to-end.
88+
if listCollaboratorsCalls != 2 { // direct + outside affiliations
89+
t.Errorf("expected 2 GET collaborators calls (direct + outside), got %d",
90+
listCollaboratorsCalls)
91+
}
92+
if listInvitationsCalls != 1 {
93+
t.Errorf("expected 1 GET invitations call, got %d", listInvitationsCalls)
94+
}
95+
if putCollaboratorCalls != 1 {
96+
t.Errorf("expected 1 PUT collaborator call, got %d", putCollaboratorCalls)
97+
}
98+
}

0 commit comments

Comments
 (0)