-
Notifications
You must be signed in to change notification settings - Fork 4.7k
E2E Utils: Add retry mechanism to the REST API discovery #62331
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
Changes from 1 commit
393c297
111ae20
47d01ba
efd2567
7a24758
702d7c1
4bee3ae
6d2b1c3
60bf411
94d0833
2b68944
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| */ | ||
| import * as fs from 'fs/promises'; | ||
| import { dirname } from 'path'; | ||
| import { expect } from '@playwright/test'; | ||
| import type { APIRequestContext } from '@playwright/test'; | ||
|
|
||
| /** | ||
|
|
@@ -39,10 +40,30 @@ async function getAPIRootURL( request: APIRequestContext ) { | |
| } | ||
|
|
||
| async function setupRest( this: RequestUtils ): Promise< StorageState > { | ||
| const [ nonce, rootURL ] = await Promise.all( [ | ||
| this.login(), | ||
| getAPIRootURL( this.request ), | ||
| ] ); | ||
| let nonce = ''; | ||
| let rootURL = ''; | ||
|
|
||
| await expect | ||
| .poll( | ||
| async () => { | ||
| try { | ||
| [ nonce, rootURL ] = await Promise.all( [ | ||
| this.login(), | ||
| getAPIRootURL( this.request ), | ||
| ] ); | ||
| } catch ( error ) { | ||
| // Prints the error if the timeout is reached. | ||
| return error; | ||
| } | ||
|
|
||
| return nonce && rootURL ? true : false; | ||
| }, | ||
| { | ||
| message: 'Failed to setup REST API.', | ||
| timeout: 60_000, // 1 minute. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Do we have a rough number when the REST API will be available? A minute seems like a long wait if it returns nothing. Is 5 seconds or 10 seconds enough?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question, and I have no idea TBH. Testing this thing takes ages, and it only fails in CI. Having said that, since this solution seems to be working across the board, I can lower the timeout value and do a couple more reruns. We can also address it in a follow-up PR since the failing perf tests keep blocking folks. What do you think? |
||
| } | ||
| ) | ||
| .toBe( true ); | ||
|
|
||
| const { cookies } = await this.request.storageState(); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.