test: add E2E and Storybook coverage for SPEC.md gaps#159
Merged
ryota-murakami merged 3 commits intomainfrom Feb 27, 2026
Merged
test: add E2E and Storybook coverage for SPEC.md gaps#159ryota-murakami merged 3 commits intomainfrom
ryota-murakami merged 3 commits intomainfrom
Conversation
…tate - Fix all path references to use src/ prefix (app/ → src/app/, lib/ → src/lib/) - Add missing routes: boards/favorites, boards/new, public/[slug], (landing) - Add missing server actions: public-board.ts, user-settings.ts - Update MSW handlers.ts → handlers/ directory structure - Remove withAuth() from auth guard docs (eliminated in #150) - Add "Move to Another Board" feature to SPEC and README - Update CDP helper list and E2E test table - Bump SPEC version to v1.5 (2026-02-27)
- Add 6 new E2E spec files: public-board, account-profile, command-palette, keyboard-shortcuts, maintenance-crud, maintenance-view-toggle - Add 2 Storybook story files: AccountClient, PublicBoardClient - Fix existing add-repository-pagination tests (test isolation via resetRepoCards) - Add db-query helpers: resetBoardPublicState, resetMaintenanceItems - Update seed.sql with public board and maintenance seed data
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughMoves project code from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
e2e/logged-in/keyboard-shortcuts.spec.ts (1)
79-82: Replace fixed timeout with explicit wait.
waitForTimeout(300)is a flaky pattern. Prefer waiting for a specific condition.💡 Suggested fix
// Dismiss any open modals first await page.keyboard.press('Escape') - await page.waitForTimeout(300) + // Wait for any modal to close before proceeding + await page.waitForLoadState('networkidle')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/logged-in/keyboard-shortcuts.spec.ts` around lines 79 - 82, After pressing Escape with page.keyboard.press('Escape'), remove the flaky await page.waitForTimeout(300) and replace it with an explicit wait for the modal to be dismissed — e.g. identify the modal/dialog selector used in the app (such as '[role="dialog"]', '.modal', or '.modal-backdrop') and call await page.waitForSelector('<modal-selector>', { state: 'hidden' }) or use await page.locator('<modal-selector>').waitFor({ state: 'hidden' }) so the test proceeds only after the modal is gone.e2e/logged-in/account-profile.spec.ts (1)
50-59: CSS class selector is fragile.
.text-2xl.font-boldties the test to Tailwind utility classes. If the design changes, this test will fail even though the functionality works. Consider addingdata-testidattributes to stat values in the component.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/logged-in/account-profile.spec.ts` around lines 50 - 59, The test is using the fragile Tailwind selector '.text-2xl.font-bold' (statValues via main.locator), so update the component to add stable attributes (e.g., data-testid="stat-value" on each stat value) and change the test to locate with main.locator('[data-testid="stat-value"]') (keep the existing count and loop assertions against statValues.nth(i).textContent()). Ensure the data-testid string matches exactly between component and test.src/app/account/AccountClient.stories.tsx (1)
119-124: Consider scoping the zero-count assertion.
getAllByText('0')could match unrelated text. Scoping to the stats section would make this more resilient.💡 Suggested refinement
// All three stat counts should display "0" - const zeros = canvas.getAllByText('0') - expect(zeros.length).toBe(3) + // Stats are rendered with .text-2xl.font-bold class + const statValues = canvasElement.querySelectorAll('.text-2xl.font-bold') + expect(statValues.length).toBe(3) + for (const el of statValues) { + expect(el.textContent).toBe('0') + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/account/AccountClient.stories.tsx` around lines 119 - 124, The test assertion using canvas.getAllByText('0') is too broad; narrow it to the stats container so unrelated "0" text won't be matched. Update the story test to first locate the stats section (e.g., using canvas.getByTestId('account-stats') or canvas.getByRole/... for the stats element) and then call within(statsElement).getAllByText('0') and assert length === 3; if the component lacks a stable selector, add a data-testid like "account-stats" to the AccountClient stats container so the test can target it reliably.e2e/unauthenticated/public-board-happy-path.spec.ts (1)
12-16: Unused import:BOARD_IDSis imported but never used.Only
PUBLIC_BOARD_SLUGandresetBoardPublicStateare used in this file.🧹 Remove unused import
import { PUBLIC_BOARD_SLUG, - BOARD_IDS, resetBoardPublicState, } from '../helpers/db-query'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/unauthenticated/public-board-happy-path.spec.ts` around lines 12 - 16, Remove the unused import BOARD_IDS from the import list; in the import statement that currently reads "PUBLIC_BOARD_SLUG, BOARD_IDS, resetBoardPublicState" keep only PUBLIC_BOARD_SLUG and resetBoardPublicState so the file imports the two symbols actually used.e2e/helpers/db-query.ts (1)
831-842: Hardcoded projectinfo ID should be a constant.The ID
'00000000-0000-0000-0000-000000000601'is used inline but not defined inPROJECT_INFO_IDS. For consistency with the rest of the file, consider adding it as a named constant.🧹 Add constant for maintenance projectinfo ID
/** Project info UUIDs */ export const PROJECT_INFO_IDS = { projinfo1: '00000000-0000-0000-0000-000000000401', projinfo2: '00000000-0000-0000-0000-000000000402', projinfo3: '00000000-0000-0000-0000-000000000403', projinfo4: '00000000-0000-0000-0000-000000000404', + maintenanceProjinfo1: '00000000-0000-0000-0000-000000000601', } as constThen reference it in
resetMaintenanceItems:const { error: piError } = await supabase.from('projectinfo').upsert( { - id: '00000000-0000-0000-0000-000000000601', + id: PROJECT_INFO_IDS.maintenanceProjinfo1, maintenance_id: MAINTENANCE_IDS.maintenance1,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/helpers/db-query.ts` around lines 831 - 842, Hardcoded projectinfo ID string in the upsert inside resetMaintenanceItems should be moved to a named constant in PROJECT_INFO_IDS for consistency; add a new constant (e.g. PROJECT_INFO_IDS.maintenance1ProjectInfo) alongside existing PROJECT_INFO_IDS entries and replace the inline literal in the upsert call within resetMaintenanceItems so the upsert uses that constant instead of the hardcoded '00000000-0000-0000-0000-000000000601'.e2e/logged-in/maintenance-view-toggle.spec.ts (1)
53-55: CSS selector for grid container may be brittle.The selector
.grid.gap-6relies on specific Tailwind classes. If the styling changes, this test could break. Consider using a data-testid or checking for the grid button's aria-pressed state instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/logged-in/maintenance-view-toggle.spec.ts` around lines 53 - 55, The test uses a brittle CSS selector via page.locator('.grid.gap-6') (bound to gridContainer); replace this with a more resilient check — either target a data-testid attribute on the grid container or, better, assert the grid view toggle button's state (e.g., the grid toggle element's aria-pressed or selected state) rather than relying on Tailwind classes; update the spec to locate the toggle button and assert its aria-pressed value (and only fall back to a data-testid on the container if necessary).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@e2e/helpers/db-query.ts`:
- Around line 831-842: Hardcoded projectinfo ID string in the upsert inside
resetMaintenanceItems should be moved to a named constant in PROJECT_INFO_IDS
for consistency; add a new constant (e.g.
PROJECT_INFO_IDS.maintenance1ProjectInfo) alongside existing PROJECT_INFO_IDS
entries and replace the inline literal in the upsert call within
resetMaintenanceItems so the upsert uses that constant instead of the hardcoded
'00000000-0000-0000-0000-000000000601'.
In `@e2e/logged-in/account-profile.spec.ts`:
- Around line 50-59: The test is using the fragile Tailwind selector
'.text-2xl.font-bold' (statValues via main.locator), so update the component to
add stable attributes (e.g., data-testid="stat-value" on each stat value) and
change the test to locate with main.locator('[data-testid="stat-value"]') (keep
the existing count and loop assertions against statValues.nth(i).textContent()).
Ensure the data-testid string matches exactly between component and test.
In `@e2e/logged-in/keyboard-shortcuts.spec.ts`:
- Around line 79-82: After pressing Escape with page.keyboard.press('Escape'),
remove the flaky await page.waitForTimeout(300) and replace it with an explicit
wait for the modal to be dismissed — e.g. identify the modal/dialog selector
used in the app (such as '[role="dialog"]', '.modal', or '.modal-backdrop') and
call await page.waitForSelector('<modal-selector>', { state: 'hidden' }) or use
await page.locator('<modal-selector>').waitFor({ state: 'hidden' }) so the test
proceeds only after the modal is gone.
In `@e2e/logged-in/maintenance-view-toggle.spec.ts`:
- Around line 53-55: The test uses a brittle CSS selector via
page.locator('.grid.gap-6') (bound to gridContainer); replace this with a more
resilient check — either target a data-testid attribute on the grid container
or, better, assert the grid view toggle button's state (e.g., the grid toggle
element's aria-pressed or selected state) rather than relying on Tailwind
classes; update the spec to locate the toggle button and assert its aria-pressed
value (and only fall back to a data-testid on the container if necessary).
In `@e2e/unauthenticated/public-board-happy-path.spec.ts`:
- Around line 12-16: Remove the unused import BOARD_IDS from the import list; in
the import statement that currently reads "PUBLIC_BOARD_SLUG, BOARD_IDS,
resetBoardPublicState" keep only PUBLIC_BOARD_SLUG and resetBoardPublicState so
the file imports the two symbols actually used.
In `@src/app/account/AccountClient.stories.tsx`:
- Around line 119-124: The test assertion using canvas.getAllByText('0') is too
broad; narrow it to the stats container so unrelated "0" text won't be matched.
Update the story test to first locate the stats section (e.g., using
canvas.getByTestId('account-stats') or canvas.getByRole/... for the stats
element) and then call within(statsElement).getAllByText('0') and assert length
=== 3; if the component lacks a stable selector, add a data-testid like
"account-stats" to the AccountClient stats container so the test can target it
reliably.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
CLAUDE.mdREADME.mdSPEC.mde2e/helpers/db-query.tse2e/logged-in/account-profile.spec.tse2e/logged-in/add-repository-pagination.spec.tse2e/logged-in/command-palette.spec.tse2e/logged-in/keyboard-shortcuts.spec.tse2e/logged-in/maintenance-crud.spec.tse2e/logged-in/maintenance-view-toggle.spec.tse2e/unauthenticated/public-board-happy-path.spec.tssrc/app/account/AccountClient.stories.tsxsrc/app/public/[slug]/PublicBoardClient.stories.tsxsupabase/seed.sql
🧪 E2E Coverage Report (Sharded: 12 parallel jobs)
📊 Full report available in workflow artifacts |
- Replace waitForTimeout(300) with explicit dialog wait (keyboard-shortcuts)
- Remove unused BOARD_IDS import (public-board-happy-path)
- Extract hardcoded projectinfo ID to PROJECT_INFO_IDS constant (db-query)
- Skipped: CSS class selectors (.text-2xl.font-bold, .grid.gap-6) — would
require component changes beyond PR scope
- Skipped: getAllByText('0') scoping — MSW-mocked story has controlled data
Contributor
Author
|
@coderabbitai resolve |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
e2e/logged-in/keyboard-shortcuts.spec.ts (1)
81-83: Don’t swallow assertion failures in setup steps.At Line 81-83,
.catch(() => {})can hide real failures and make this test silently flaky.Suggested fix
- await expect(page.getByRole('dialog')) - .not.toBeVisible({ timeout: 5000 }) - .catch(() => {}) + await expect(page.locator('[role="dialog"]:visible')).toHaveCount(0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/logged-in/keyboard-shortcuts.spec.ts` around lines 81 - 83, The test is swallowing failures by appending .catch(() => {}) to the assertion on the dialog locator (page.getByRole('dialog')), which hides real test errors; remove the .catch(() => {}) so the await expect(page.getByRole('dialog')).not.toBeVisible({ timeout: 5000 }) can fail loudly, or if the dialog may or may not exist, replace this pattern with an explicit conditional check using the locator's isVisible()/count() and assert the expected state instead (e.g., await locator.isVisible() and then expect(...) or assert count === 0) to avoid suppressing assertion failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/helpers/db-query.ts`:
- Around line 803-849: resetMaintenanceItems currently uses upsert which leaves
any extra maintenance rows created by prior tests; instead make the helper
deterministically reset state by first deleting all rows in the maintenance
table (e.g., supabase.from('maintenance').delete(...)) and then inserting
exactly the seedItems (use insert, not upsert) so only the expected rows exist;
likewise, delete any projectinfo rows linked to those maintenance entries (or
all projectinfo rows if appropriate) before re-inserting the specific
PROJECT_INFO_IDS. Update the logic in resetMaintenanceItems to perform
delete-then-insert for both maintenance and projectinfo (and wrap in a
transaction if supported) so tests get an exact, reproducible seed.
In `@e2e/logged-in/keyboard-shortcuts.spec.ts`:
- Around line 100-109: The negative assertion for the shortcuts modal is
performed after pressing 'Escape', risking a false pass; ensure you check that
pressing '?' does not open the shortcuts modal before closing any dialogs by
calling page.keyboard.press('?') then immediately asserting on the
shortcutsModal locator (const shortcutsModal =
page.locator('[data-testid="shortcuts-help-modal"]')) with await
expect(shortcutsModal).not.toBeVisible() and only after that press 'Escape' to
close the settings dialog, so reorder the steps to press '?' → assert
shortcutsModal not visible → press 'Escape' → assert dialog closed.
---
Nitpick comments:
In `@e2e/logged-in/keyboard-shortcuts.spec.ts`:
- Around line 81-83: The test is swallowing failures by appending .catch(() =>
{}) to the assertion on the dialog locator (page.getByRole('dialog')), which
hides real test errors; remove the .catch(() => {}) so the await
expect(page.getByRole('dialog')).not.toBeVisible({ timeout: 5000 }) can fail
loudly, or if the dialog may or may not exist, replace this pattern with an
explicit conditional check using the locator's isVisible()/count() and assert
the expected state instead (e.g., await locator.isVisible() and then expect(...)
or assert count === 0) to avoid suppressing assertion failures.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
e2e/helpers/db-query.tse2e/logged-in/keyboard-shortcuts.spec.tse2e/unauthenticated/public-board-happy-path.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- e2e/unauthenticated/public-board-happy-path.spec.ts
✅ Actions performedComments resolved. Auto-approval is disabled; enable |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #159 +/- ##
==========================================
+ Coverage 69.06% 69.37% +0.31%
==========================================
Files 153 155 +2
Lines 4636 4696 +60
Branches 1239 1259 +20
==========================================
+ Hits 3202 3258 +56
- Misses 1411 1415 +4
Partials 23 23 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
add-repository-pagination.spec.tsby addingresetRepoCards()inbeforeEachfor proper test isolationresetBoardPublicState,resetMaintenanceItems) and updateseed.sqlwith public board + maintenance seed dataNew E2E Files
public-board-happy-path.spec.tsaccount-profile.spec.tscommand-palette.spec.tskeyboard-shortcuts.spec.ts?help,.overflow,Enteropen cardmaintenance-crud.spec.tsmaintenance-view-toggle.spec.tsNew Storybook Stories
AccountClient.stories.tsxPublicBoardClient.stories.tsxExisting Test Fixes
add-repository-pagination.spec.ts: AddedresetRepoCards()inbeforeEachto ensure combobox shows available repos (previously failed due to leftover cards from prior tests)Test plan
pnpm lint— 0 warnings, 0 errorspnpm typecheck— 0 errorspnpm test— 1290 passedpnpm build— successpnpm e2e:parallel— 402 passed, 0 failed (12/12 shards)Summary by CodeRabbit