-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: extend W3mFrameProvider timeout from 30s to 2 minutes #4451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Increase timeout from 30_000 to 120_000 milliseconds in appEvent method - Update corresponding test timeout from 31_000 to 121_000 milliseconds - Allow more time for device approval and OTP input operations Fixes APKT-3008 Co-Authored-By: Sven Voskamp <[email protected]>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
10 Skipped Deployments
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Coverage Report
File Coverage
|
Please also update the E2E tests with the correct timings |
…eout - Update WalletPage.ts session button visibility timeout from 30s to 2 minutes - Update ModalValidator.ts signing success toast timeout from 30s to 2 minutes - Ensures E2E tests align with extended frame provider timeout for device approval and OTP operations Co-Authored-By: Sven Voskamp <[email protected]>
@@ -87,7 +87,7 @@ export class WalletPage { | |||
await this.page.waitForLoadState() | |||
const btn = this.page.getByTestId(`session-${variant}-button`) | |||
await expect(btn, `Session ${variant} element should be visible`).toBeVisible({ | |||
timeout: 30000 | |||
timeout: 120000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No this change is not correct, please have a look at the following tests and change them to the new timeout values:
email-after-farcaster.spec.ts: it should abort embedded wallet flow if it takes more than 20 seconds after abort login with farcaster
email.spec.ts it should abort embedded wallet flow if it takes more than 20 seconds
@@ -148,7 +148,7 @@ export class ModalValidator { | |||
async expectAcceptedSign() { | |||
// We use Chakra Toast and it's not quite straightforward to set the `data-testid` attribute on the toast element. | |||
await expect(this.page.getByText(ConstantsUtil.SigningSucceededToastTitle)).toBeVisible({ | |||
timeout: 30 * 1000 | |||
timeout: 120 * 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for the above comment
…o 2 minutes - Revert WalletPage.ts session button timeout back to 30 seconds (incorrect change) - Revert ModalValidator.ts signing toast timeout back to 30 seconds (incorrect change) - Update email-after-farcaster.spec.ts embedded wallet abort test from 20s to 2 minutes - Update email.spec.ts embedded wallet abort test from 20s to 2 minutes - Addresses svenvoskamp's GitHub comments on PR #4451 Co-Authored-By: Sven Voskamp <[email protected]>
The timeout is too long so the browser will close already in the E2E tests, could you simulate it? |
Extend W3mFrameProvider timeout from 30 seconds to 2 minutes
Summary
This PR extends the request timeout in the W3mFrameProvider from 30 seconds to 2 minutes to allow more time for users completing operations like device approval and OTP input.
Changes Made
30_000
to120_000
milliseconds in theappEvent
method31_000
to121_000
milliseconds and test descriptionTesting
Ticket
Fixes APKT-3008
Link to Devin run
https://app.devin.ai/sessions/5ec0086490bb485bba660172e022f89d
Requested by
Sven Voskamp ([email protected])