Skip to content

🧪 Test handleRateLimit function in apiUtils #373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
is0692vs wants to merge 1 commit into
base: main
Choose a base branch
from
+45 −2
Open

🧪 Test handleRateLimit function in apiUtils #373

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions src/lib/__tests__/apiUtils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { handleErrorResponse, getAuthenticatedUser } from '../apiUtils';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { handleErrorResponse, getAuthenticatedUser, handleRateLimit } from '../apiUtils';
import { NextResponse } from 'next/server';
import { getServerSession } from "next-auth";

Expand Down Expand Up @@ -97,4 +97,47 @@ describe('apiUtils', () => {
expect(result).toBeNull();
});
});

describe('handleRateLimit', () => {
beforeEach(() => {
vi.useFakeTimers();
vi.setSystemTime(new Date(1600000000000)); // Timestamp: 1600000000
});

afterEach(() => {
vi.useRealTimers();
});

it('should throw RateLimitError using the reset timestamp from the X-RateLimit-Reset header', () => {
const resetTimestamp = 1600003600;
const headers = new Headers();
headers.set('X-RateLimit-Reset', resetTimestamp.toString());
const res = new Response(null, { headers });

expect(() => handleRateLimit(res)).toThrowError(
`GitHub API rate limit exceeded. Resets at ${new Date(resetTimestamp * 1000).toISOString()}`
);
});
Comment on lines +111 to +120

@greptile-apps greptile-apps Bot Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 ヘッダー値がフォールバック値と同一でテストが無意味化している

テストで使用している resetTimestamp = 1600003600 は、フリーズされたシステム時刻 (1600000000000 ms) から計算されるフォールバック値 Math.floor(1600000000000 / 1000) + 3600 = 1600003600 とまったく同じ値です。そのため、実装が X-RateLimit-Reset ヘッダーを完全に無視して常にフォールバックを使用するよう壊れていたとしても、このテストはパスしてしまいます。ヘッダーが正しく読まれていることを確認するには、フォールバック値と異なる値（例: 1600007200）を使う必要があります。

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/lib/__tests__/apiUtils.test.ts
Line: 111-120

Comment:
**ヘッダー値がフォールバック値と同一でテストが無意味化している**

テストで使用している `resetTimestamp = 1600003600` は、フリーズされたシステム時刻 (`1600000000000 ms`) から計算されるフォールバック値 `Math.floor(1600000000000 / 1000) + 3600 = 1600003600` とまったく同じ値です。そのため、実装が `X-RateLimit-Reset` ヘッダーを完全に無視して常にフォールバックを使用するよう壊れていたとしても、このテストはパスしてしまいます。ヘッダーが正しく読まれていることを確認するには、フォールバック値と異なる値（例: `1600007200`）を使う必要があります。

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +111 to +120

@greptile-apps greptile-apps Bot Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 スローされるエラー型の検証が欠如している

3つのテストはすべてエラーメッセージ文字列のみを照合しており、スローされるオブジェクトが RateLimitError のインスタンスであることを確認していません。RateLimitError を Error に置き換えても同じメッセージであればテストはパスします。expect(() => handleRateLimit(res)).toThrow(RateLimitError) のような型チェックを追加することで、型安全性も保証できます。

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/lib/__tests__/apiUtils.test.ts
Line: 111-120

Comment:
**スローされるエラー型の検証が欠如している**

3つのテストはすべてエラーメッセージ文字列のみを照合しており、スローされるオブジェクトが `RateLimitError` のインスタンスであることを確認していません。`RateLimitError` を `Error` に置き換えても同じメッセージであればテストはパスします。`expect(() => handleRateLimit(res)).toThrow(RateLimitError)` のような型チェックを追加することで、型安全性も保証できます。

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


it('should throw RateLimitError using default time + 3600s if X-RateLimit-Reset header is missing', () => {
const headers = new Headers();
const res = new Response(null, { headers });
const expectedResetTimestamp = Math.floor(Date.now() / 1000) + 3600;

expect(() => handleRateLimit(res)).toThrowError(
`GitHub API rate limit exceeded. Resets at ${new Date(expectedResetTimestamp * 1000).toISOString()}`
);
});

it('should throw RateLimitError using default time + 3600s if X-RateLimit-Reset header is invalid', () => {
const headers = new Headers();
headers.set('X-RateLimit-Reset', 'invalid');
const res = new Response(null, { headers });
const expectedResetTimestamp = Math.floor(Date.now() / 1000) + 3600;

expect(() => handleRateLimit(res)).toThrowError(
`GitHub API rate limit exceeded. Resets at ${new Date(expectedResetTimestamp * 1000).toISOString()}`
);
});
Comment on lines +111 to +141

@gemini-code-assist gemini-code-assist Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since the system time is mocked to a fixed value (1600000000000 ms / 1600000000 seconds) in beforeEach, the expected reset timestamp and the resulting ISO string are completely deterministic and static. Instead of dynamically calculating Date.now() and calling new Date().toISOString() inside the test assertions, we can hardcode the expected ISO string (2020-09-13T13:26:40.000Z). This significantly improves test readability, simplifies the assertions, and avoids redundant runtime calculations while preserving the rate limit error handling test coverage.

    it('should throw RateLimitError using the reset timestamp from the X-RateLimit-Reset header', () => {
      const resetTimestamp = 1600003600;
      const headers = new Headers();
      headers.set('X-RateLimit-Reset', resetTimestamp.toString());
      const res = new Response(null, { headers });

      expect(() => handleRateLimit(res)).toThrowError(
        'GitHub API rate limit exceeded. Resets at 2020-09-13T13:26:40.000Z'
      );
    });

    it('should throw RateLimitError using default time + 3600s if X-RateLimit-Reset header is missing', () => {
      const headers = new Headers();
      const res = new Response(null, { headers });

      expect(() => handleRateLimit(res)).toThrowError(
        'GitHub API rate limit exceeded. Resets at 2020-09-13T13:26:40.000Z'
      );
    });

    it('should throw RateLimitError using default time + 3600s if X-RateLimit-Reset header is invalid', () => {
      const headers = new Headers();
      headers.set('X-RateLimit-Reset', 'invalid');
      const res = new Response(null, { headers });

      expect(() => handleRateLimit(res)).toThrowError(
        'GitHub API rate limit exceeded. Resets at 2020-09-13T13:26:40.000Z'
      );
    });
References
  1. When refactoring API routes or their associated tests, ensure that existing test cases for error handling (such as rate limits) are preserved to maintain robust coverage.

});
});
Loading