fix flaky cookie sharing test#1310
Conversation
Temporarily repeat the cross-subdomain cookie test 10 times to verify CI stability. Increased waitUntil timeout from 10s to 30s to account for network retry exponential backoff.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA test timeout in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
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 docstrings
🧪 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 |
Greptile SummaryThis PR increases the Confidence Score: 5/5Safe to merge — change is isolated to a test timeout and does not affect production code. All findings are P2 style/cleanup suggestions with no correctness or reliability impact. The single changed line is a straightforward timeout increase in a test file. No files require special attention; Important Files Changed
Sequence DiagramsequenceDiagram
participant Test
participant StackClientApp
participant CookieStore
participant Backend
Test->>StackClientApp: signUpWithCredential()
StackClientApp->>Backend: POST /api/v1/auth/signup
Backend-->>StackClientApp: refresh_token
StackClientApp->>CookieStore: set default + custom cookie
Test->>Test: delete customCookie, set only defaultCookie
Test->>StackClientApp: new StackClientApp() (page reload simulation)
StackClientApp->>Backend: exchange default refresh_token (with retries)
Backend-->>StackClientApp: session valid
StackClientApp->>CookieStore: eagerly write customCookie
Test->>CookieStore: waitUntil(has(customCookie), 30s)
CookieStore-->>Test: true
Prompt To Fix All With AIThis is a comment left during a code review.
Path: apps/e2e/tests/js/cookies.test.ts
Line: 423
Comment:
**PR description mentions 10x repetition not implemented**
The PR description says the test is "temporarily repeat[ed] 10 times to verify CI stability," but the diff only increases the timeout — there is no loop or repeat logic anywhere in the file. If repetition was intended to catch intermittent failures, consider wrapping the test body in a loop or using a test-framework retry utility before merging.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: apps/e2e/tests/js/cookies.test.ts
Line: 423
Comment:
**No TODO for temporary change**
The PR description explicitly calls this change "temporary," but the code has no `// TODO:` or tracker reference to signal that the 30 s timeout should be reverted once CI stability is confirmed. Without a marker, this inflated timeout is likely to stay indefinitely.
```suggestion
const customRecreated = await waitUntil(() => cookieStore.has(customCookieName), 30_000); // TODO: revert to 10_000 after CI stability is confirmed
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "Remove test loop, keep increased waitUnt..." | Re-trigger Greptile |
Temporarily repeat the cross-subdomain cookie test 10 times to verify CI stability. Increased waitUntil timeout from 10s to 30s to account for network retry exponential backoff.
Summary by CodeRabbit
Release Notes
This pull request contains internal testing infrastructure updates only. A test timeout value was adjusted to improve test reliability. No user-visible changes or new features are included in this release.