Skip to content
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.

Sign up for GitHub

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

E2E Utils: Add retry mechanism to the REST API discovery #62331

Merged
merged 11 commits into from
Jun 5, 2024
Merged

E2E Utils: Add retry mechanism to the REST API discovery #62331

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
393c297
add retry mechanism to the REST API discovery
WunderBart Jun 5, 2024
111ae20
Update packages/e2e-test-utils-playwright/src/request-utils/rest.ts
WunderBart Jun 5, 2024
47d01ba
Extend the discovery timeout to 2 minutes
WunderBart Jun 5, 2024
efd2567
disable the webServer option
WunderBart Jun 5, 2024
7a24758
tmp: print errors
WunderBart Jun 5, 2024
702d7c1
Revert "Extend the discovery timeout to 2 minutes"
WunderBart Jun 5, 2024
4bee3ae
Revert "Update packages/e2e-test-utils-playwright/src/request-utils/r…
WunderBart Jun 5, 2024
6d2b1c3
Revert "add retry mechanism to the REST API discovery"
WunderBart Jun 5, 2024
60bf411
retry API discovery along with login
WunderBart Jun 5, 2024
94d0833
add comment
WunderBart Jun 5, 2024
2b68944
Revert "disable the webServer option"
WunderBart Jun 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 26 additions & 12 deletions packages/e2e-test-utils-playwright/src/request-utils/rest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -22,18 +23,31 @@ function splitRequestsToChunks( requests: BatchRequest[], chunkSize: number ) {
}

async function getAPIRootURL( request: APIRequestContext ) {
// Discover the API root url using link header.
// See https://developer.wordpress.org/rest-api/using-the-rest-api/discovery/#link-header
const response = await request.head( WP_BASE_URL );
const links = response.headers().link;
const restLink = links?.match( /<([^>]+)>; rel="https:\/\/api\.w\.org\/"/ );

if ( ! restLink ) {
throw new Error( `Failed to discover REST API endpoint.
Link header: ${ links }` );
}

const [ , rootURL ] = restLink;
let restLink: unknown = null;
WunderBart marked this conversation as resolved.
Show resolved Hide resolved

// Retry until the REST API root URL is discovered.
// See https://github.com/WordPress/gutenberg/issues/61627
await expect
WunderBart marked this conversation as resolved.
Show resolved Hide resolved
.poll(
async () => {
// Discover the API root url using link header.
// See https://developer.wordpress.org/rest-api/using-the-rest-api/discovery/#link-header
const response = await request.head( WP_BASE_URL );
const links = response.headers().link;
restLink = links?.match(
/<([^>]+)>; rel="https:\/\/api\.w\.org\/"/
);

return restLink;
},
{
message: 'Failed to discover REST API endpoint.',
timeout: 60_000, // 1 minute.
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The 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?

}
)
.not.toBeFalsy();
WunderBart marked this conversation as resolved.
Show resolved Hide resolved

const [ , rootURL ] = restLink as RegExpMatchArray;

return rootURL;
}
Expand Down
Loading