📦 [Consolidated] PR #60, #62~#65 のテスト統合・レビュー指摘対応#61
Conversation
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Added comprehensive test cases for buildHourlyHeatmapFromCommitDates, getMostActiveHour, and getMostActiveDayFromCalendar. Coverage includes happy paths, empty states, invalid date strings, and edge-case behaviors like tie-breaking logic. Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the reliability of the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughダッシュボードAPIルート（stats、summary、year）に対する包括的なテスト群と、GitHubビューア取得・年間レビュー用ユーティリティの単体テストを追加。さらに yearInReviewUtils に入力検証の防御的ロジックを追加しました。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a comprehensive set of tests for yearInReviewUtils, significantly improving test coverage. The tests are well-structured and cover many important scenarios, including empty inputs and tie-breaking logic. I've identified a couple of areas where the functions under test could be made more robust against malformed input, and have suggested adding test cases to cover these scenarios.
Added missing test coverage for `src/app/api/dashboard/year/route.ts` including the error path when `fetchYearInReviewData` throws, as well as the happy path, 400, and 401 scenarios to fully cover the route. Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Fixed TypeScript error in route.test.ts by casting `mockData` using `Awaited<ReturnType<typeof fetchYearInReviewData>>` instead of `any` to satisfy lint rules. Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/api/dashboard/stats/route.test.ts`:
- Around line 1-26: Add a mock for "@/lib/auth" similar to the one in
src/app/api/dashboard/year/route.test.ts so the test file that exercises GET
(and imports authOptions) won't hit import errors; specifically, add a vi.mock
call that exports a stubbed authOptions object (matching the symbol name
authOptions) before running tests so GET's imports resolve consistently with the
other test file.
In `@src/app/api/dashboard/summary/route.test.ts`:
- Around line 1-21: The tests are missing a mock for the auth options used by
the route; add a vi.mock for "@/lib/auth" that exports the authOptions object
(used by src/app/api/dashboard/summary/route.ts) and/or any helper the route
imports so authentication behavior can be simulated, then update the GET route
tests to explicitly mock getServerSession to return a valid Session for the
success path and null/undefined for the unauthenticated path asserting a 401
response; reference the GET handler and authOptions/getServerSession mocks to
locate where to wire these mocks in route.test.ts.
In `@src/app/api/dashboard/year/route.test.ts`:
- Around line 35-51: Add boundary tests for year validation in
src/app/api/dashboard/year/route.test.ts: the current test only covers a
non-numeric year, but the implementation in GET (exported GET in ./route) checks
year < 2008 and year > new Date().getUTCFullYear(); add tests that call GET with
year=2007 (expect 400), year=2008 (expect success), year=currentYear (expect
success), and year=currentYear+1 (expect 400), reusing the same getServerSession
mock and NextRequest construction pattern used in the existing invalid-year test
so behavior and assertions (response.status and response.json()) match the other
tests.
- Around line 53-95: Add a test in src/app/api/dashboard/year/route.test.ts that
mirrors the non-Error-throwing case from stats tests: mock getServerSession as
in the other tests, mock fetchYearInReviewData to reject with a non-Error value
(e.g., a string), call the route GET handler with the same NextRequest, assert
the response.status is 500 and the JSON body equals { error: "Unknown error" }
so the GET route's non-Error exception path is covered.
In `@src/lib/__tests__/githubViewer.test.ts`:
- Around line 37-56: The test calls fetchViewerLogin twice to inspect the thrown
GitHubApiError; instead, call it once into a variable (e.g., const p =
fetchViewerLogin("invalid-token")) and run multiple Jest rejects assertions
against that single promise (e.g., await
expect(p).rejects.toBeInstanceOf(GitHubApiError); await
expect(p).rejects.toHaveProperty("status", 401); await
expect(p).rejects.toHaveProperty("message", "Failed to resolve current GitHub
user")). Update the final mockFetch assertion to expect one call
(expect(mockFetch).toHaveBeenCalledTimes(1)); this keeps mockFetch,
fetchViewerLogin, and GitHubApiError as the points of reference.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4a24f01a-8486-4ecd-8be9-91399da25b2d
📒 Files selected for processing (5)
src/app/api/dashboard/stats/route.test.tssrc/app/api/dashboard/summary/route.test.tssrc/app/api/dashboard/year/route.test.tssrc/lib/__tests__/githubViewer.test.tssrc/lib/__tests__/yearInReviewUtils.test.ts
is0692vs
changed the title
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
Add invalid date checks in getMostActiveDayFromCalendar and malformed matrix checks in getMostActiveHour. Added corresponding tests to yearInReviewUtils.test.ts. Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/api/dashboard/stats/route.test.ts`:
- Around line 135-152: Add a test case that covers the failure path where
fetchViewerLogin rejects: mock getServerSession to return a session with no
user.login and accessToken "mock-token", mock fetchViewerLogin to reject (e.g.,
throw), and ensure fetchCommitActivityHeatmap is not called and the route
handler GET returns a 500 response; reference the existing test structure using
GET, getServerSession, fetchViewerLogin, fetchCommitActivityHeatmap, and
createRequest so the new it("should return 500 when fetchViewerLogin fails",
...) follows the same pattern and assertions for status and call expectations.
In `@src/app/api/dashboard/summary/route.test.ts`:
- Around line 62-87: Add a new test in route.test.ts that simulates
fetchViewerLogin rejecting and asserts GET returns a 500; mock getServerSession
to return an accessToken (as in the existing test), mock
vi.mocked(fetchViewerLogin).mockRejectedValueOnce(new Error("something")) and
ensure fetchUserSummary is not called, then call GET(), assert response.status
=== 500 and optionally assert error shape/message; use the same imports/mocking
pattern as the existing "returns summary using fetchViewerLogin if session user
login is missing" test so symbols to edit are GET, getServerSession,
fetchViewerLogin, and fetchUserSummary.
In `@src/app/api/dashboard/year/route.test.ts`:
- Around line 167-188: Add a test that covers the fallback path when
getServerSession returns a session without user.login: mock getServerSession to
return accessToken but no user.login, mock fetchViewerLogin (from
"@/lib/githubViewer") to resolve to a login string, and assert that
fetchViewerLogin is called with the session.accessToken and that
fetchYearInReviewData (from "@/lib/githubYearInReview") is subsequently called
with the resolved login; then call GET(req) and verify the response is 200 and
contains the mocked data. Ensure you reference the existing symbols
getServerSession, fetchViewerLogin, fetchYearInReviewData, and GET when adding
assertions and mocks.
In `@src/lib/__tests__/githubViewer.test.ts`:
- Around line 2-3: Replace the relative imports in the test with the project
alias: change import { fetchViewerLogin } from "../githubViewer" to import {
fetchViewerLogin } from "@/lib/githubViewer" and change import { GitHubApiError
} from "../types" to import { GitHubApiError } from "@/lib/types"; ensure the
module specifiers use the "@/..." alias (matching tsconfig/webpack settings) so
imports resolve from src.
In `@src/lib/__tests__/yearInReviewUtils.test.ts`:
- Around line 51-54: Add tests for non-finite values so getMostActiveHour
returns 0 for heatmaps containing NaN or Infinity: update
src/lib/__tests__/yearInReviewUtils.test.ts to include cases like
getMostActiveHour([[NaN, 1, 2]]) and getMostActiveHour([[1, Infinity, 2]]) (and
any nested/malformed shapes you already test) and assert they return 0; this
ensures the Number.isFinite branch in getMostActiveHour is covered and prevents
regressions.
- Around line 3-6: Update the import of buildHourlyHeatmapFromCommitDates,
getMostActiveHour, and getMostActiveDayFromCalendar to use the project `@/` path
alias instead of a relative import; locate the import that currently references
"../yearInReviewUtils" and replace it with the corresponding `@/` alias import
from the src tree (e.g., "@/lib/yearInReviewUtils") so all src imports in this
test file use the `@/` alias.
In `@src/lib/yearInReviewUtils.ts`:
- Around line 16-22: The current isValidHeatmap check can throw a TypeError when
a heatmap row is not an array because row.every is invoked unguarded; update the
validation used by isValidHeatmap so that it verifies heatmap is an array of
length 7 and each row passes Array.isArray(row) && row.length === 24 and then
uses row.every(count => Number.isFinite(count)); ensure this guarded check (used
where isValidHeatmap is computed) returns false for any non-array row so the
existing fallback return 0 remains safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cfe51688-6cf9-4706-a438-40a4fda0f1fc
📒 Files selected for processing (6)
src/app/api/dashboard/stats/route.test.tssrc/app/api/dashboard/summary/route.test.tssrc/app/api/dashboard/year/route.test.tssrc/lib/__tests__/githubViewer.test.tssrc/lib/__tests__/yearInReviewUtils.test.tssrc/lib/yearInReviewUtils.ts
Add NaN/Infinity edge case assertions to yearInReviewUtils tests and harden getMostActiveHour matrix validation with Array.isArray and Number.isFinite checks. Also updated test imports to use the project @/ alias consistently. Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Add NaN/Infinity edge case assertions to yearInReviewUtils tests and harden getMostActiveHour matrix validation with Array.isArray and Number.isFinite checks. Also updated test imports to use the project @/ alias consistently. Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
…52745 # Conflicts: # .github/copilot-instructions.md
1 participant
概要
このPRは、同じくテスト追加・テスト補強を目的とした以下5個のPRを一つのブランチにて統合（マージ）し、さらにレビューコメントで妥当だった指摘を取り込んだうえで、テスト系の変更をまとめて確認できるようにするためのものです。
統合元のPR（これらは本作業に伴いクローズ対象です）:
🧪 add tests for yearInReviewUtils functions🧪 Add dashboard stats API tests🧪 [Test] Add unit tests for fetchViewerLogin🧪 [testing] add dashboard summary API route tests🧪 Add test coverage for dashboard/year API endpoint対応した修正点 (レビュー指摘への対応)
src/lib/__tests__/yearInReviewUtils.test.ts(PR 🧪 add tests for yearInReviewUtils functions #60 指摘対応)src/lib/__tests__/yearInReviewUtils.test.ts(PR 🧪 add tests for yearInReviewUtils functions #60 指摘対応)forEachを使う書き方を避け、toEqualベースの明示的な比較へ寄せることで、可読性と lint/レビュー観点の妥当性を両立しました。API / ライブラリのテスト追加を一本化
src/app/api/dashboard/stats/route.test.tssrc/app/api/dashboard/summary/route.test.tssrc/app/api/dashboard/year/route.test.tssrc/lib/__tests__/githubViewer.test.ts上記の追加を同一ブランチへ集約し、関連するテスト変更を一つのPRで確認できるよう整理しました。
確認事項