Skip to content

Commit 05b2edc

Browse files
committed
test(backend): pin which GitHub response classes skip and which abort
#621 asks that a plain 403, a 403 carrying Retry-After, a rate-limit 403, a 429 and a cancellation are not swallowed into one outcome. The errors.Is change makes every wrapped ErrGitHubUnavailable skippable, so the visible difference between "skip this repository" and "abort the scan" now depends on how restGet classifies the response, not on this loop. Adds a table over the real request path covering all five classes, with request counts that also pin the retry budget (two attempts for a single-token pool): a plain 403 is requested once and skipped, Retry-After and rate-limit responses are retried before skipping or aborting, 429 and context.Canceled abort, and an aborted scan never touches a later repository. Refs #621
1 parent 14746dd commit 05b2edc

1 file changed

Lines changed: 101 additions & 0 deletions

File tree

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package backend
2+
3+
import (
4+
"context"
5+
"errors"
6+
"net/http"
7+
"testing"
8+
)
9+
10+
// #621 asks that a plain 403, a 403 carrying Retry-After, a rate-limited 403,
11+
// a 429 and a cancellation are not collapsed into one outcome. These cases pin
12+
// what the errors.Is change does at the boundary of the request layer's retry
13+
// budget (two attempts for a single-token pool, see request() in
14+
// github_client.go).
15+
func TestHydrateTopRepoEvidenceClassifiesRequestFailures(t *testing.T) {
16+
for _, scenario := range []struct {
17+
name string
18+
status int
19+
remaining string
20+
retryAfter string
21+
cancel bool
22+
wantSentinel error
23+
wantFirstReads int
24+
wantContinue bool
25+
}{
26+
{
27+
name: "plain 403 skips the repository and continues", status: http.StatusForbidden,
28+
remaining: "4998", wantSentinel: ErrGitHubUnavailable, wantFirstReads: 1, wantContinue: true,
29+
},
30+
{
31+
name: "403 with Retry-After is retried then skipped", status: http.StatusForbidden,
32+
remaining: "4998", retryAfter: "0.01", wantSentinel: ErrGitHubUnavailable, wantFirstReads: 2, wantContinue: true,
33+
},
34+
{
35+
name: "rate limited 403 aborts", status: http.StatusForbidden,
36+
remaining: "0", wantSentinel: ErrGitHubRateLimited, wantFirstReads: 2, wantContinue: false,
37+
},
38+
{
39+
name: "429 aborts", status: http.StatusTooManyRequests,
40+
remaining: "4998", wantSentinel: ErrGitHubRateLimited, wantFirstReads: 2, wantContinue: false,
41+
},
42+
{
43+
name: "cancellation aborts and is not swallowed", status: http.StatusForbidden,
44+
remaining: "4998", cancel: true, wantSentinel: context.Canceled, wantFirstReads: 0, wantContinue: false,
45+
},
46+
} {
47+
t.Run(scenario.name, func(t *testing.T) {
48+
requested := map[string]int{}
49+
client := newEvidenceTestClient(t, func(w http.ResponseWriter, r *http.Request) {
50+
requested[r.URL.Path]++
51+
w.Header().Set("Content-Type", "application/json")
52+
w.Header().Set("X-Ratelimit-Remaining", scenario.remaining)
53+
if scenario.retryAfter != "" {
54+
w.Header().Set("Retry-After", scenario.retryAfter)
55+
}
56+
switch r.URL.Path {
57+
case "/repos/alice/first/readme":
58+
w.WriteHeader(scenario.status)
59+
_, _ = w.Write([]byte(`{"message":"synthetic failure"}`))
60+
case "/repos/alice/second/readme":
61+
_, _ = w.Write([]byte(`{"path":"README.md","encoding":"base64","size":120,"content":"` + evidenceReadmeBase64 + `"}`))
62+
case "/repos/alice/second/languages":
63+
_, _ = w.Write([]byte(`{"TypeScript":800}`))
64+
default:
65+
w.WriteHeader(http.StatusNotFound)
66+
_, _ = w.Write([]byte(`{"message":"Not Found"}`))
67+
}
68+
})
69+
70+
ctx := context.Background()
71+
if scenario.cancel {
72+
canceled, cancel := context.WithCancel(ctx)
73+
cancel()
74+
ctx = canceled
75+
}
76+
repos := []TopRepo{
77+
{Name: "first", OwnerLogin: stringPointer("alice"), Stars: 40},
78+
{Name: "second", OwnerLogin: stringPointer("alice"), Stars: 20},
79+
}
80+
err := client.hydrateTopRepoEvidence(ctx, repos, "alice", 6)
81+
if got := requested["/repos/alice/first/readme"]; got != scenario.wantFirstReads {
82+
t.Errorf("first repository README requests = %d, want %d", got, scenario.wantFirstReads)
83+
}
84+
if scenario.wantContinue {
85+
if err != nil {
86+
t.Fatalf("collection should continue past this response, got %v", err)
87+
}
88+
if repos[1].Readme == nil {
89+
t.Error("later repository lost its evidence even though collection continued")
90+
}
91+
return
92+
}
93+
if !errors.Is(err, scenario.wantSentinel) {
94+
t.Fatalf("error = %v, want %v", err, scenario.wantSentinel)
95+
}
96+
if requested["/repos/alice/second/readme"] != 0 {
97+
t.Errorf("aborted scan requested later repositories: %v", requested)
98+
}
99+
})
100+
}
101+
}

0 commit comments

Comments
 (0)