fix(e2e): mobile-viewport suite bounced by 5-min idle gate, not layout - #939
fix(e2e): mobile-viewport suite bounced by 5-min idle gate, not layout#939njrini99-code wants to merge 1 commit into
Conversation
…suite The seeded storageState pins the idle-activity marker at auth-setup time, so any test starting >5min (SESSION_IDLE_TIMEOUT_MS) after setup is bounced to /login by the middleware idle gate before measuring — the blocking suite passed or failed on runtime proximity to that cliff (train run 29623264998 crossed it at coach test 29 / player test 60; the earlier green run simply finished under it). An active-viewport test should look like an active user. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Summary by CodeRabbit
WalkthroughThe mobile viewport test navigation helper now refreshes the ChangesMobile viewport test navigation
Estimated code review effort: 2 (Simple) | ~5 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (10 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)ast-grep could not parse rule config: /ast-grep-rules/../git/.coderabbit/ast-grep/no-explicit-any.yml 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/mobile-viewports.spec.ts`:
- Around line 50-60: Import the exported SESSION_IDLE_COOKIE constant from the
session-idle shared module and use it as the cookie name in the
page.context().addCookies call, replacing the hardcoded 'sb_last_activity' value
while preserving the existing cookie refresh behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e17448b1-d25a-4508-8bcd-95d1b3764daa
📒 Files selected for processing (1)
e2e/mobile-viewports.spec.ts
| // The seeded storageState pins `sb_last_activity` (SESSION_IDLE_COOKIE in | ||
| // src/lib/auth/session-idle-shared.ts) at auth-setup time, so any test that | ||
| // starts more than SESSION_IDLE_TIMEOUT_MS (5 min) after setup is bounced | ||
| // to /login by the middleware idle gate before it can measure anything — | ||
| // the suite passes or fails on how close its runtime sits to that cliff. | ||
| // A viewport test simulates an ACTIVE user, so refresh the marker the same | ||
| // way real interaction does. | ||
| const baseURL = test.info().project.use.baseURL ?? 'http://localhost:3000'; | ||
| await page.context().addCookies([ | ||
| { name: 'sb_last_activity', value: Date.now().toString(), url: baseURL }, | ||
| ]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Use exported SESSION_IDLE_COOKIE constant.
e2e/mobile-viewports.spec.ts:50-60. The inline comment references SESSION_IDLE_COOKIE in src/lib/auth/session-idle-shared.ts. Import and use the exported constant directly instead of hardcoding 'sb_last_activity' to ensure the test suite automatically tracks any upstream contract changes.
♻️ Proposed refactor
Add the import at the top of the file:
import { SESSION_IDLE_COOKIE } from '`@/lib/auth/session-idle-shared`'; // Adjust alias/path as neededAnd update the cookie injection:
await page.context().addCookies([
- { name: 'sb_last_activity', value: Date.now().toString(), url: baseURL },
+ { name: SESSION_IDLE_COOKIE, value: Date.now().toString(), url: baseURL },
]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // The seeded storageState pins `sb_last_activity` (SESSION_IDLE_COOKIE in | |
| // src/lib/auth/session-idle-shared.ts) at auth-setup time, so any test that | |
| // starts more than SESSION_IDLE_TIMEOUT_MS (5 min) after setup is bounced | |
| // to /login by the middleware idle gate before it can measure anything — | |
| // the suite passes or fails on how close its runtime sits to that cliff. | |
| // A viewport test simulates an ACTIVE user, so refresh the marker the same | |
| // way real interaction does. | |
| const baseURL = test.info().project.use.baseURL ?? 'http://localhost:3000'; | |
| await page.context().addCookies([ | |
| { name: 'sb_last_activity', value: Date.now().toString(), url: baseURL }, | |
| ]); | |
| // The seeded storageState pins `sb_last_activity` (SESSION_IDLE_COOKIE in | |
| // src/lib/auth/session-idle-shared.ts) at auth-setup time, so any test that | |
| // starts more than SESSION_IDLE_TIMEOUT_MS (5 min) after setup is bounced | |
| // to /login by the middleware idle gate before it can measure anything — | |
| // the suite passes or fails on how close its runtime sits to that cliff. | |
| // A viewport test simulates an ACTIVE user, so refresh the marker the same | |
| // way real interaction does. | |
| const baseURL = test.info().project.use.baseURL ?? 'http://localhost:3000'; | |
| await page.context().addCookies([ | |
| { name: SESSION_IDLE_COOKIE, value: Date.now().toString(), url: baseURL }, | |
| ]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/mobile-viewports.spec.ts` around lines 50 - 60, Import the exported
SESSION_IDLE_COOKIE constant from the session-idle shared module and use it as
the cookie name in the page.context().addCookies call, replacing the hardcoded
'sb_last_activity' value while preserving the existing cookie refresh behavior.
|
Superseded by the bundled PR from fix/hub-subnav-train-clip (same cookie commit cherry-picked, plus the underlying hub-sub-nav truncation fix the forensics identified). |
1 participant
Run 29623264998 on the merge train 'failed' the blocking baseball mobile suite — but every failure was
Received: /baseball/login(session bounced), not a layout assertion. The seeded storageState pinssb_last_activityat auth-setup time; middleware's idle gate (SESSION_IDLE_TIMEOUT_MS = 5 min) then bounces every test that starts past setup+5min. Coach tests died at exactly setup+5min (test 29), player at its setup+5min (test 60). The earlier green run (29619371852) simply finished under the cliff.Fix:
gotoSettledrefreshes the marker per navigation, the same way real user activity does. Full e2e dispatched on this branch — it also layout-validates #937's calendar-header change at 320px, which the bounced run never got to measure.🤖 Generated with Claude Code
https://claude.ai/code/session_01MMdviLDsAg2YYJ8adsM6fg