Playwright e2e tests are configured to start wp-env but don't wait for REST API to be advertised

[GraemeF]

Description

Playwright is configured to start wp-env if not already running, but when it does the REST API likely isn't ready in time for the globalSetup function to use it. To get around that, the GitHub workflow starts wp-env ahead of the tests in the hope that it's started by the time the globalSetup function tries to use it.

Step-by-step reproduction instructions

In the gutenberg repo, make sure wp-env is not already running, then run the e2e tests:

npm run test:e2e

Expected behaviour

Playwright should wait for all required services to be available before running tests, or should not be configured to start wp-env at all, to clarify that this needs to be done ahead of time.

Actual behaviour

Playwright starts wp-env, waits for the WordPress port to be open, and then attempts to discover the location of the REST API. The REST API isn't immediately advertised via the link header, so discovery fails and the tests are not run.

> [email protected] test:e2e
> wp-scripts test-playwright --config test/e2e/playwright.config.ts

Error: Failed to discover REST API endpoint.
 Link header: undefined

   at ../../../packages/e2e-test-utils-playwright/src/request-utils/rest.ts:32

  30 |
  31 | 	if ( ! restLink ) {
> 32 | 		throw new Error( `Failed to discover REST API endpoint.
     | 		      ^
  33 |  Link header: ${ links }` );
  34 | 	}
  35 |

    at getAPIRootURL (.../packages/e2e-test-utils-playwright/src/request-utils/rest.ts:32:9)
    at RequestUtils.setupRest (.../packages/e2e-test-utils-playwright/src/request-utils/rest.ts:42:29)
    at globalSetup (.../test/e2e/config/global-setup.ts:26:2)

[Mamaduka]

cc @WunderBart @swissspidy

Side note: I think the test runner shouldn't assume that the consumer will be using wp-env, at least when WP_BASE_URL is provided.

[GraemeF]

cc @WunderBart @swissspidy

Side note: I think the test runner shouldn't assume that the consumer will be using wp-env, at least when WP_BASE_URL is provided.

I think current behaviour would be to start wp-env regardless of WP_BASE_URL.

Edit: that's wrong - sorry 😄

[swissspidy]

cc @WunderBart @swissspidy

Side note: I think the test runner shouldn't assume that the consumer will be using wp-env, at least when WP_BASE_URL is provided.

Are you referring to this line?

https://github.com/WordPress/gutenberg/blob/57741ee5e4d434c9fa6988b9f5b5966ef2768b8b/packages/scripts/config/playwright.config.js#L47

I think current behaviour would be to start wp-env regardless of WP_BASE_URL.

Right now, with the webServer config Playwright first checks if there's already a server running (under WP_BASE_URL or http://localhost:8889) and responding with a reasonable status code. If not, it starts the server itself and waits up to 120 seconds.

Playwright should wait for all required services to be available before running tests [...]

Hmm maybe we could use webServer.url for that and point it to the REST API.

Something like that:

const baseUrl = process.env.WP_BASE_URL || 'http://localhost:8889';

// ...
webServer: {
		url: `${baseUrl}/?rest_route=/wp/v2`
		command: 'npm run wp-env start',
		port: 8889,
		timeout: 120_000, // 120 seconds.
		reuseExistingServer: true,
	},

[...] , or should not be configured to start wp-env at all, to clarify that this needs to be done ahead of time.

That doesn't sound like great DX to me.

[Mamaduka]

@swissspidy, You'll get an error when running e2e for custom WP_BASE_URL. Currently, I have to start the Docker. The test runner will start the environment, and then I can test my custom installations.

Previously, using the Docker or wp-env wasn't a requirement for the e2e test, but it seems it is.

@GraemeF, sorry, I went off-topic. I agree it makes sense to wait for REST API.

Screenshot

[GraemeF]

We were trying to run some e2e tests on our own blocks in a similar way to gutenberg and noticed this. To fix we:

• In the webServer config, remove port and instead use url so Playwright waits for a successful HTTP response before continuing, rather than just the port accepting connections
• In globalSetup, add a retry around the call to requestUtils.setupRest so that the initial responses without the Link header don't make the setup fail

I'm not sure how WP_BASE_URL affects things but would be happy to submit a PR similar to the above.

[swissspidy]

You'll get an error when running e2e for custom WP_BASE_URL. Currently, I have to start the Docker. The test runner will start the environment, and then I can test my custom installations.

I think config.webServer.url will solve this. It's currently not defined.

In the webServer config, remove port and instead use url so Playwright waits for a successful HTTP response before continuing, rather than just the port accepting connections

FWIW port is not actually a valid config option there 🤔 Maybe it got removed at some point or so. That might explain why it's not working. I think config.webServer.url should do the trick, and I don't like doing retries in globalSetup.

[GraemeF]

FWIW port is not actually a valid config option there 🤔 Maybe it got removed at some point or so. That might explain why it's not working.

You're right - port seems to have been removed! Hadn't spotted that.

I think config.webServer.url should do the trick, and I don't like doing retries in globalSetup.

Unfortunately not - we found that WordPress will initially respond without the Link header, and there is no retry mechanism for globalSetup. If this setup were migrated to a project then Playwright could be configured to retry.

[WunderBart]

👋 I've added a simple retry mechanism for the API discovery using expect.poll in https://github.com/WordPress/gutenberg/pull/62331. Looks like it's working as expected! 🤞