🧪 test: add missing tests for useCardSettings hook - #533
Conversation
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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Warning Review limit reached
Next review available in: 23 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 |
PR Summary by QodoAdd comprehensive test coverage for useCardSettings hook
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // We should not trigger the internal useEffect hydration logice, | ||
| // though initial state is still loaded during useState init. | ||
| // We mainly verify layout matches. | ||
| expect(result.current.layout).toEqual(mockLayout); |
There was a problem hiding this comment.
このアサーションは useState の初期化だけで成立するため、mounted ガードが壊れて hydration や persistence が実行されてもテストが成功します。loadCardSettings の呼び出し回数と saveCardSettings が呼ばれないことを検証し、未マウント時のライフサイクル契約を直接保護してください。
Knowledge Base Used: Dashboard flow
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/hooks/__tests__/useCardSettings.test.ts
Line: 52
Comment:
**未マウント時の副作用を未検証**
このアサーションは `useState` の初期化だけで成立するため、`mounted` ガードが壊れて hydration や persistence が実行されてもテストが成功します。`loadCardSettings` の呼び出し回数と `saveCardSettings` が呼ばれないことを検証し、未マウント時のライフサイクル契約を直接保護してください。
**Knowledge Base Used:** [Dashboard flow](https://app.greptile.com/hiroki-org/-/custom-context/knowledge-base/hiroki-org/github-user-summary/-/docs/dashboard.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Code Review by Qodo
1. Relative import of useCardSettings
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| // @vitest-environment jsdom | ||
| import { renderHook, act } from "@testing-library/react"; | ||
| import { describe, expect, it, vi, beforeEach } from "vitest"; | ||
| import { useCardSettings } from "../useCardSettings"; |
There was a problem hiding this comment.
1. Relative import of usecardsettings 📘 Rule violation ✧ Quality
The new test imports useCardSettings via a relative path even though the target module is under src/, which should be imported using the @/ alias. This can lead to inconsistent import patterns and brittle refactors across the src tree.
Agent Prompt
## Issue description
A frontend `src/` import uses a relative path (`../useCardSettings`) instead of the required `@/` alias.
## Issue Context
The repository already uses `@/` alias imports in this file (e.g., `@/lib/cardSettings`), so `useCardSettings` should also be imported via `@/` to match the project rule and avoid deep relative paths.
## Fix Focus Areas
- src/hooks/__tests__/useCardSettings.test.ts[4-4]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| it("should save settings when layout or options change", () => { | ||
| const { result } = renderHook(() => useCardSettings(true)); | ||
|
|
||
| // Initially called on mount due to useEffect persistence | ||
| expect(saveCardSettings).toHaveBeenCalledWith(mockLayout, mockOptions); | ||
|
|
There was a problem hiding this comment.
2. Brittle save effect assertion 🐞 Bug ☼ Reliability
useCardSettings only calls saveCardSettings after the hydration useEffect sets isHydrated=true, but the test asserts saveCardSettings immediately after renderHook without waiting for that two-effect sequence to complete. This makes the test dependent on Testing Library’s internal effect-flushing behavior and can become flaky or start emitting act warnings if the hook’s hydration/persist timing changes.
Agent Prompt
### Issue description
The test asserts `saveCardSettings` synchronously right after `renderHook`, but the hook persists only after hydration completes (`isHydrated` becomes `true`). Make the test explicitly wait for the expected call(s) so it’s robust to React effect scheduling and future hook changes.
### Issue Context
`useCardSettings` performs hydration in a `useEffect`, then persists in a second `useEffect` gated by `isHydrated`.
### Fix Focus Areas
- src/hooks/__tests__/useCardSettings.test.ts[2-3]
- src/hooks/__tests__/useCardSettings.test.ts[55-71]
### Suggested change
- Import `waitFor` from `@testing-library/react`.
- Make the test `async` and use:
- `await waitFor(() => expect(saveCardSettings).toHaveBeenCalledWith(mockLayout, mockOptions))`
- after state updates, `await waitFor(() => expect(saveCardSettings).toHaveBeenCalledWith(...))`
This aligns with existing hook tests that wait for effect-driven updates.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
1 participant
🎯 What: The testing gap for the
useCardSettingshook insrc/hooks/useCardSettings.tshas been addressed by providing a comprehensive test suite.📊 Coverage: The new tests cover initial state hydration from storage, conditional hydration handling (when not mounted), settings persistence on change, and accurate testing for block and display options toggling and visibility queries.
✨ Result: A significant improvement in test coverage that increases the overall codebase reliability.
PR created automatically by Jules for task 15749146415318158028 started by @is0692vs
Greptile Summary
useCardSettingsの初期化、永続化、表示切り替え、可視性照会を対象とするテストスイートを追加しています。Confidence Score: 4/5
本番コードは変更されておらずマージを妨げる問題はありませんが、未マウント時の副作用ガードを直接検証するアサーションの追加を推奨します。
追加テストのうち未マウント時のケースは、初期化時にも得られるレイアウトだけを確認するため、hydration や persistence が誤って実行される回帰を検出できません。
Files Needing Attention: src/hooks/tests/useCardSettings.test.ts
Important Files Changed
useCardSettingsの主要操作を広くテストしていますが、未マウント時のケースは hydration／persistence が抑止されたことを検証できていません。Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "test: add missing tests for useCardSetti..." | Re-trigger Greptile
Context used: