Skip to content

Commit 63208a2

Browse files
snomiaoclaude
andcommitted
fix: resolve pre-existing test failures in gh.spec.ts and parseIssueUrl.spec.ts
- Fix gh.spec.ts import path (src/ghc.ts moved to lib/github/githubCached.ts in #139) - Fix doNotRetry values must be numbers not strings - string comparison against numeric error.status always failed, causing all 4xx responses to retry 3x - Replace server.use() error overrides with sentinel values in github-handlers.ts since MSW runtime handler overrides don't intercept in Bun - Rewrite 3 timeout tests to use expect().rejects with sentinel owner values - Fix parseIssueUrl trailing slash test to match current (correct) behavior - Remove unused params destructuring in github-handlers.ts Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent aefa824 commit 63208a2

4 files changed

Lines changed: 27 additions & 88 deletions

File tree

lib/github/createOctokit.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export function createOctokit({ auth, maxRetries = 3 }: OctokitOptions) {
3232
},
3333
},
3434
retry: {
35-
doNotRetry: ["429"], // Let throttling plugin handle rate limits
35+
doNotRetry: [400, 401, 403, 404, 422, 429], // 429 handled by throttling plugin
3636
},
3737
});
3838
}

src/gh.spec.ts

Lines changed: 14 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import { server } from "./test/msw-setup";
66
process.env.GH_TOKEN = "test-token-for-gh-spec";
77

88
// Dynamic import to ensure env var is set
9-
const ghModule = await import("./ghc");
10-
const { gh } = await import("./ghc");
9+
const ghModule = await import("../lib/github/githubCached");
10+
const { gh } = await import("../lib/github/githubCached");
1111
const { ghc, clearGhCache } = ghModule;
1212

1313
describe("GitHub API Client (gh)", () => {
@@ -344,27 +344,10 @@ describe("GitHub API Client (gh)", () => {
344344
});
345345

346346
it("should handle non-annotated tag errors gracefully", async () => {
347-
// Mock a 404 response for lightweight tags
348-
server.use(
349-
http.get("https://api.github.com/repos/:owner/:repo/git/tags/:tag_sha", () => {
350-
return new HttpResponse(null, {
351-
status: 404,
352-
statusText: "Not Found",
353-
});
354-
}),
355-
);
356-
357-
try {
358-
await gh.git.getTag({
359-
owner: "comfyanonymous",
360-
repo: "ComfyUI",
361-
tag_sha: "lightweight-tag",
362-
});
363-
// Should not reach here
364-
expect(true).toBe(false);
365-
} catch (error: unknown) {
366-
expect(error.status).toBe(404);
367-
}
347+
// tag_sha="lightweight-tag" is handled by github-handlers.ts to return 404
348+
await expect(
349+
gh.git.getTag({ owner: "comfyanonymous", repo: "ComfyUI", tag_sha: "lightweight-tag" }),
350+
).rejects.toMatchObject({ status: 404 });
368351
});
369352
});
370353

@@ -389,66 +372,17 @@ describe("GitHub API Client (gh)", () => {
389372

390373
describe("Error Handling", () => {
391374
it("should handle 404 errors", async () => {
392-
server.use(
393-
http.get("https://api.github.com/repos/:owner/:repo", () => {
394-
return new HttpResponse(
395-
JSON.stringify({
396-
message: "Not Found",
397-
documentation_url: "https://docs.github.com/rest",
398-
}),
399-
{
400-
status: 404,
401-
headers: {
402-
"Content-Type": "application/json",
403-
},
404-
},
405-
);
406-
}),
407-
);
408-
409-
try {
410-
await gh.repos.get({
411-
owner: "nonexistent",
412-
repo: "nonexistent",
413-
});
414-
// Should not reach here
415-
expect(true).toBe(false);
416-
} catch (error: unknown) {
417-
expect(error.status).toBe(404);
418-
}
375+
// owner="error-404" is handled by github-handlers.ts to return 404
376+
await expect(gh.repos.get({ owner: "error-404", repo: "any" })).rejects.toMatchObject({
377+
status: 404,
378+
});
419379
});
420380

421381
it("should handle rate limit errors", async () => {
422-
server.use(
423-
http.get("https://api.github.com/repos/:owner/:repo", () => {
424-
return new HttpResponse(
425-
JSON.stringify({
426-
message: "API rate limit exceeded",
427-
documentation_url:
428-
"https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting",
429-
}),
430-
{
431-
status: 403,
432-
headers: {
433-
"Content-Type": "application/json",
434-
"X-RateLimit-Limit": "60",
435-
"X-RateLimit-Remaining": "0",
436-
},
437-
},
438-
);
439-
}),
440-
);
441-
442-
try {
443-
await gh.repos.get({
444-
owner: "octocat",
445-
repo: "Hello-World",
446-
});
447-
// Should not reach here
448-
expect(true).toBe(false);
449-
} catch (error: unknown) {
450-
expect(error.status).toBe(403);
451-
}
382+
// owner="rate-limited" is handled by github-handlers.ts to return 403
383+
await expect(gh.repos.get({ owner: "rate-limited", repo: "any" })).rejects.toMatchObject({
384+
status: 403,
385+
});
452386
});
453387
});
454388
});

src/parseIssueUrl.spec.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,8 @@ describe("parseIssueUrl", () => {
8787
});
8888

8989
it("should handle URLs with trailing slashes", () => {
90-
// Note: The current regex doesn't handle trailing slashes, so this should fail
9190
const url = "https://github.com/owner/repo/pull/123/";
9291

93-
expect(() => parseIssueUrl(url)).toThrow();
92+
expect(parseIssueUrl(url)).toMatchObject({ owner: "owner", repo: "repo", issue_number: 123 });
9493
});
9594
});

src/test/github-handlers.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,13 @@ export const githubHandlers = [
1010
// ==================== REPOS ====================
1111

1212
// GET /repos/:owner/:repo - Get a repository
13+
// Use owner="error-404" to simulate 404, owner="rate-limited" to simulate 403 rate limit
1314
http.get(`${GITHUB_API_BASE}/repos/:owner/:repo`, ({ params }) => {
1415
const { owner, repo } = params;
16+
if (owner === "error-404")
17+
return HttpResponse.json({ message: "Not Found", documentation_url: "https://docs.github.com/rest" }, { status: 404 });
18+
if (owner === "rate-limited")
19+
return HttpResponse.json({ message: "API rate limit exceeded" }, { status: 403 });
1520
return HttpResponse.json({
1621
id: 123456,
1722
name: repo,
@@ -426,7 +431,7 @@ export const githubHandlers = [
426431
http.post(
427432
`${GITHUB_API_BASE}/repos/:owner/:repo/pulls/:pull_number/requested_reviewers`,
428433
async ({ params, request }) => {
429-
const { owner, repo, pull_number } = params;
434+
const { pull_number } = params;
430435
const body = (await request.json()) as Record<string, unknown>;
431436

432437
return HttpResponse.json({
@@ -694,8 +699,7 @@ export const githubHandlers = [
694699
// POST /repos/:owner/:repo/issues/:issue_number/labels - Add labels
695700
http.post(
696701
`${GITHUB_API_BASE}/repos/:owner/:repo/issues/:issue_number/labels`,
697-
async ({ params, request }) => {
698-
const { owner, repo, issue_number } = params;
702+
async ({ request }) => {
699703
const body = (await request.json()) as Record<string, unknown>;
700704

701705
return HttpResponse.json(
@@ -710,8 +714,7 @@ export const githubHandlers = [
710714
// GET /repos/:owner/:repo/issues/:issue_number/timeline - List timeline events
711715
http.get(
712716
`${GITHUB_API_BASE}/repos/:owner/:repo/issues/:issue_number/timeline`,
713-
({ params, request }) => {
714-
const { owner, repo, issue_number } = params;
717+
({ request }) => {
715718
const url = new URL(request.url);
716719
const page = parseInt(url.searchParams.get("page") || "1");
717720
const perPage = parseInt(url.searchParams.get("per_page") || "100");
@@ -760,8 +763,11 @@ export const githubHandlers = [
760763
// ==================== GIT ====================
761764

762765
// GET /repos/:owner/:repo/git/tags/:tag_sha - Get a tag (annotated)
766+
// Use tag_sha="lightweight-tag" to simulate a non-annotated (lightweight) tag returning 404
763767
http.get(`${GITHUB_API_BASE}/repos/:owner/:repo/git/tags/:tag_sha`, ({ params }) => {
764768
const { owner, repo, tag_sha } = params;
769+
if (tag_sha === "lightweight-tag")
770+
return HttpResponse.json({ message: "Not Found" }, { status: 404 });
765771
return HttpResponse.json({
766772
node_id: "MDM6VGFn",
767773
tag: "v1.0.0",

0 commit comments

Comments
 (0)