From 1c19f63e007a4069e9bf2d7010ab980b69c46a8b Mon Sep 17 00:00:00 2001 From: Adrian Moldovan <3854374+adimoldovan@users.noreply.github.com> Date: Wed, 11 Nov 2026 13:23:26 +0200 Subject: [PATCH 1/3] Build/Test Tools: Run the Add New Media E2E spec in parallel. `media-new-client-side-upload.test.js` deleted all media and all posts after each test, so it had to run in the serial project. The new `attachments` fixture deletes only the attachments that a test uploads or tracks. The spec now reads each attachment by its own id, not the newest one, and creates its post with `postFactory`. See #66392. --- tests/e2e/README.md | 2 +- tests/e2e/fixtures/attachment.js | 41 +++++++ tests/e2e/fixtures/index.js | 2 + tests/e2e/playwright.config.js | 1 - .../media-new-client-side-upload.test.js | 106 ++++++++++++++---- 5 files changed, 129 insertions(+), 23 deletions(-) create mode 100644 tests/e2e/fixtures/attachment.js diff --git a/tests/e2e/README.md b/tests/e2e/README.md index ac859e604adca..b9124d088d727 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -1 +1 @@ -# E2E Tests End-To-End (E2E) tests for WordPress. ## Running the tests The e2e tests require a production-like environment to run. By default, they will assume an environment is available at `http://localhost:8889`, with username `admin` and password `password`. If you don't already have an environment ready, you can set one up by following [these instructions](https://github.com/WordPress/wordpress-develop/blob/master/README.md). Then you can launch the tests by running: ``` npm run test:e2e ``` which will run the test suite using a headless browser. If your environment has a different url, username or password to the default, you can provide the base URL, username and password like this: ``` WP_BASE_URL=http://mycustomurl WP_USERNAME=username WP_PASSWORD=password npm run test:e2e ``` **DO NOT run these tests in an actual production environment, as they will delete all your content.** For debugging purposes, you might want to follow the test visually. You can do so by running the tests in an interactive mode: ``` npm run test:e2e -- --ui ``` [UI Mode](https://playwright.dev/docs/test-ui-mode) let's you explore, run and debug tests with a time travel experience complete with watch mode. All test files are loaded into the testing sidebar where you can expand each file and describe block to individually run, view, watch and debug each test. You can also run a single test file separately: ``` npm run test:e2e tests/e2e/specs/hello.test.js ``` ## Projects The suite has three Playwright projects, defined in `playwright.config.js`: * `all`: every spec, one at a time on one worker. `npm run test:e2e` runs this project. * `parallel`: specs that can share the site with other tests. Spec files run at the same time on several workers, one for every two CPU cores. Tests inside one file still run in order. Run it with `npm run test:e2e -- --project=parallel`. * `serial`: specs that break the site for other tests. They run one at a time on one worker. Run it with `npm run test:e2e -- --project=serial`. CI runs `parallel` and `serial` in separate jobs. New specs go in `parallel` by default. Add a spec to `SERIAL_SPECS` if it makes the site unusable for other tests. If it changes content or settings that other tests use, for example with `requestUtils.deleteAllPosts()`, follow [Parallel safety](#parallel-safety). Before you open a pull request with a new spec, run its project a few times with `--shuffle`. Playwright then runs the spec files in a random order, so a spec that needs another spec to run first, or that breaks a spec that runs after it, fails: ``` npm run test:e2e -- --project=parallel --shuffle ``` Playwright prints the seed of each run, for example `shuffle seed 317753389`. To run the same order again, give the seed: `--shuffle 317753389`. ## Parallel safety Specs in `parallel` share one site. A test that changes shared data can break a test in another file that runs at the same time. To make a spec safe, use the first of these that works: 1. Scope the data. Let the test create its own data, for example a post with a unique title, and read only that data. 2. If the test must change shared data, such as a site setting, give it a lock. Do this only if every spec that writes the same data holds the same lock. 3. If you cannot lock every spec that writes the data, add the spec to `SERIAL_SPECS`. A lock is a name. Playwright never runs two tests that hold the same lock name at the same time. A lock only schedules tests. It does not isolate data: a test without the lock can still change the data while a locked test runs. The lock names are in `utils/locks.js`. Import the constant, so that all specs use the same name. Playwright gives no error for a lock name that no other test uses, or for an import name that is not in `utils/locks.js`: ``` import { GENERAL_SETTINGS } from '../utils/locks'; test.describe( 'Settings -> General', { lock: GENERAL_SETTINGS }, () => {} ); ``` A form save writes every field on the page, not only the field that the test changed. A spec that saves the General Settings form (`options-general.php`) must hold `GENERAL_SETTINGS`. A spec in `parallel` that changes a General Settings option in a different way, for example with `siteSettings` or `siteOptions`, must also hold it. ## Logged-out pages Most specs run logged in as the admin user. There are two ways to run a test logged out: * To make the `page` of a whole `describe` group logged out, add `test.use( { storageState: { cookies: [], origins: [] } } )` to the group. See `specs/dashboard.test.js`. * To use a logged-out page next to the logged-in `page` in one test, import `test` and `expect` from `../fixtures` and use the `loggedOutPage` fixture. Its browser context has no cookies, and it closes when the test ends. Unlike `page`, it resolves a URL that starts with `/` against the host only, not the full `WP_BASE_URL`, so it does not work with a site in a subdirectory. See `specs/cache-control-headers-directives.test.js`. ## Fixtures To create posts, pages or users, or to change a setting, an option, or a must-use plugin, import `test` and `expect` from `../fixtures`, and use one of the fixtures below. Each fixture cleans up when the test ends, also if the test fails. If the cleanup fails, the test fails. * `siteSettings`: reads and changes the settings of `/wp/v2/settings` through REST, with `get()`, `remember( keys )` and `set( patch )`. It restores only the keys passed to `remember` or `set`. See `specs/privacy.test.js`. * `siteOptions`: reads and changes options through WP-CLI, with `get( names )`, `remember( names )` and `set( values )`. It reads and restores the stored `gmt_offset`, not the value calculated for a named timezone. It restores only the keys passed to `remember` or `set`. See `specs/general-settings-invalid-timezone.test.js`. * `muPlugin`: writes a must-use plugin with `write( fileName, php )`, and deletes the file when the test ends. Call `write` before the test visits a page. See `specs/fatal-error-handler.test.js`. * `postFactory`: creates posts with `create( data )` and pages with `createPage( data )`, and force-deletes them when the test ends. `uniqueTitle( prefix )` adds a UUID to the prefix. See `specs/edit-posts.test.js`. * `userFactory`: creates users with `create( { roles, ...data } )`, which returns `{ user, username, password }`. For a user that the test creates through a form, call `adopt( username )` before the form is submitted. When the test ends, it deletes each user and the user's auto-drafts. To log in as the user, import `signIn( page, username, password )` from `../fixtures/user`. See `specs/users.test.js`. If the test changes a setting or an option through a form, first call `remember` with an array of the keys that the form changes. The fixture then reads the current value, without a write, and restores it when the test ends. `remember` returns the current values. ## Documentation * Block Editor Handbook end to end testing overview: https://developer.wordpress.org/block-editor/contributors/code/testing-overview/#end-to-end-testing * Gutenberg e2e-test-utils-playwright package API docs: https://github.com/WordPress/gutenberg/tree/trunk/packages/e2e-test-utils-playwright * Playwright API docs: https://playwright.dev/docs (the version we are using is indicated in the `@wordpress/scripts` package: https://github.com/WordPress/gutenberg/blob/trunk/packages/scripts/package.json) \ No newline at end of file +# E2E Tests End-To-End (E2E) tests for WordPress. ## Running the tests The e2e tests require a production-like environment to run. By default, they will assume an environment is available at `http://localhost:8889`, with username `admin` and password `password`. If you don't already have an environment ready, you can set one up by following [these instructions](https://github.com/WordPress/wordpress-develop/blob/master/README.md). Then you can launch the tests by running: ``` npm run test:e2e ``` which will run the test suite using a headless browser. If your environment has a different url, username or password to the default, you can provide the base URL, username and password like this: ``` WP_BASE_URL=http://mycustomurl WP_USERNAME=username WP_PASSWORD=password npm run test:e2e ``` **DO NOT run these tests in an actual production environment, as they will delete all your content.** For debugging purposes, you might want to follow the test visually. You can do so by running the tests in an interactive mode: ``` npm run test:e2e -- --ui ``` [UI Mode](https://playwright.dev/docs/test-ui-mode) let's you explore, run and debug tests with a time travel experience complete with watch mode. All test files are loaded into the testing sidebar where you can expand each file and describe block to individually run, view, watch and debug each test. You can also run a single test file separately: ``` npm run test:e2e tests/e2e/specs/hello.test.js ``` ## Projects The suite has three Playwright projects, defined in `playwright.config.js`: * `all`: every spec, one at a time on one worker. `npm run test:e2e` runs this project. * `parallel`: specs that can share the site with other tests. Spec files run at the same time on several workers, one for every two CPU cores. Tests inside one file still run in order. Run it with `npm run test:e2e -- --project=parallel`. * `serial`: specs that break the site for other tests. They run one at a time on one worker. Run it with `npm run test:e2e -- --project=serial`. CI runs `parallel` and `serial` in separate jobs. New specs go in `parallel` by default. Add a spec to `SERIAL_SPECS` if it makes the site unusable for other tests. If it changes content or settings that other tests use, for example with `requestUtils.deleteAllPosts()`, follow [Parallel safety](#parallel-safety). Before you open a pull request with a new spec, run its project a few times with `--shuffle`. Playwright then runs the spec files in a random order, so a spec that needs another spec to run first, or that breaks a spec that runs after it, fails: ``` npm run test:e2e -- --project=parallel --shuffle ``` Playwright prints the seed of each run, for example `shuffle seed 317753389`. To run the same order again, give the seed: `--shuffle 317753389`. ## Parallel safety Specs in `parallel` share one site. A test that changes shared data can break a test in another file that runs at the same time. To make a spec safe, use the first of these that works: 1. Scope the data. Let the test create its own data, for example a post with a unique title, and read only that data. 2. If the test must change shared data, such as a site setting, give it a lock. Do this only if every spec that writes the same data holds the same lock. 3. If you cannot lock every spec that writes the data, add the spec to `SERIAL_SPECS`. A lock is a name. Playwright never runs two tests that hold the same lock name at the same time. A lock only schedules tests. It does not isolate data: a test without the lock can still change the data while a locked test runs. The lock names are in `utils/locks.js`. Import the constant, so that all specs use the same name. Playwright gives no error for a lock name that no other test uses, or for an import name that is not in `utils/locks.js`: ``` import { GENERAL_SETTINGS } from '../utils/locks'; test.describe( 'Settings -> General', { lock: GENERAL_SETTINGS }, () => {} ); ``` A form save writes every field on the page, not only the field that the test changed. A spec that saves the General Settings form (`options-general.php`) must hold `GENERAL_SETTINGS`. A spec in `parallel` that changes a General Settings option in a different way, for example with `siteSettings` or `siteOptions`, must also hold it. ## Logged-out pages Most specs run logged in as the admin user. There are two ways to run a test logged out: * To make the `page` of a whole `describe` group logged out, add `test.use( { storageState: { cookies: [], origins: [] } } )` to the group. See `specs/dashboard.test.js`. * To use a logged-out page next to the logged-in `page` in one test, import `test` and `expect` from `../fixtures` and use the `loggedOutPage` fixture. Its browser context has no cookies, and it closes when the test ends. Unlike `page`, it resolves a URL that starts with `/` against the host only, not the full `WP_BASE_URL`, so it does not work with a site in a subdirectory. See `specs/cache-control-headers-directives.test.js`. ## Fixtures To create posts, pages or users, to upload attachments, or to change a setting, an option, or a must-use plugin, import `test` and `expect` from `../fixtures`, and use one of the fixtures below. Each fixture cleans up when the test ends, also if the test fails. If the cleanup fails, the test fails. * `siteSettings`: reads and changes the settings of `/wp/v2/settings` through REST, with `get()`, `remember( keys )` and `set( patch )`. It restores only the keys passed to `remember` or `set`. See `specs/privacy.test.js`. * `siteOptions`: reads and changes options through WP-CLI, with `get( names )`, `remember( names )` and `set( values )`. It reads and restores the stored `gmt_offset`, not the value calculated for a named timezone. It restores only the keys passed to `remember` or `set`. See `specs/general-settings-invalid-timezone.test.js`. * `muPlugin`: writes a must-use plugin with `write( fileName, php )`, and deletes the file when the test ends. Call `write` before the test visits a page. See `specs/fatal-error-handler.test.js`. * `postFactory`: creates posts with `create( data )` and pages with `createPage( data )`, and force-deletes them when the test ends. `uniqueTitle( prefix )` adds a UUID to the prefix. See `specs/edit-posts.test.js`. * `attachments`: uploads a file with `upload( absolutePath )`, records an attachment that the browser uploaded with `track( id )`, and deletes them when the test ends. See `specs/media-new-client-side-upload.test.js`. * `userFactory`: creates users with `create( { roles, ...data } )`, which returns `{ user, username, password }`. For a user that the test creates through a form, call `adopt( username )` before the form is submitted. When the test ends, it deletes each user and the user's auto-drafts. To log in as the user, import `signIn( page, username, password )` from `../fixtures/user`. See `specs/users.test.js`. If the test changes a setting or an option through a form, first call `remember` with an array of the keys that the form changes. The fixture then reads the current value, without a write, and restores it when the test ends. `remember` returns the current values. A fixture goes in `fixtures/` only when 2 or more spec files use it. A fixture that only one spec uses stays in that spec. ## Documentation * Block Editor Handbook end to end testing overview: https://developer.wordpress.org/block-editor/contributors/code/testing-overview/#end-to-end-testing * Gutenberg e2e-test-utils-playwright package API docs: https://github.com/WordPress/gutenberg/tree/trunk/packages/e2e-test-utils-playwright * Playwright API docs: https://playwright.dev/docs (the version we are using is indicated in the `@wordpress/scripts` package: https://github.com/WordPress/gutenberg/blob/trunk/packages/scripts/package.json) \ No newline at end of file diff --git a/tests/e2e/fixtures/attachment.js b/tests/e2e/fixtures/attachment.js new file mode 100644 index 0000000000000..2083d120ee640 --- /dev/null +++ b/tests/e2e/fixtures/attachment.js @@ -0,0 +1,41 @@ +/** + * Uploads attachments, records the ones the browser uploads, then deletes + * them in teardown. + * + * @param {Object} fixtures The fixtures this one depends on. + * @param {import('@wordpress/e2e-test-utils-playwright').RequestUtils} fixtures.requestUtils The request utils fixture. + * @param {Function} use Hands the fixture to the test. + * @return {Promise} + */ +export const attachments = async ( { requestUtils }, use ) => { + const ids = new Set(); + + await use( { + async upload( absolutePath ) { + const attachment = await requestUtils.uploadMedia( absolutePath ); + ids.add( attachment.id ); + return attachment; + }, + track( id ) { + ids.add( id ); + }, + } ); + + const failures = []; + for ( const id of ids ) { + try { + await requestUtils.deleteMedia( id ); + } catch ( error ) { + // A test can delete its own attachment through the UI. + if ( error.data?.status !== 404 ) { + failures.push( `${ id }: ${ error.message }` ); + } + } + } + + if ( failures.length ) { + throw new Error( + `attachments could not delete:\n${ failures.join( '\n' ) }` + ); + } +}; diff --git a/tests/e2e/fixtures/index.js b/tests/e2e/fixtures/index.js index 77368fa0d3160..cd62f41898b2f 100644 --- a/tests/e2e/fixtures/index.js +++ b/tests/e2e/fixtures/index.js @@ -6,6 +6,7 @@ import { test as base, expect } from '@wordpress/e2e-test-utils-playwright'; /** * Internal dependencies */ +import { attachments } from './attachment'; import { postFactory } from './post'; import { muPlugin, siteOptions, siteSettings } from './site'; import { userFactory } from './user'; @@ -18,6 +19,7 @@ export const test = base.extend( { await use( await context.newPage() ); await context.close(); }, + attachments, muPlugin, postFactory, siteOptions, diff --git a/tests/e2e/playwright.config.js b/tests/e2e/playwright.config.js index 8f303d9e8fd27..2fa158224637a 100644 --- a/tests/e2e/playwright.config.js +++ b/tests/e2e/playwright.config.js @@ -30,7 +30,6 @@ const SERIAL_SPECS = [ 'empty-trash-restore-trashed-posts.test.js', 'media-library-client-side-upload.test.js', 'media-modal-client-side-upload.test.js', - 'media-new-client-side-upload.test.js', ]; const config = defineConfig( { diff --git a/tests/e2e/specs/media-new-client-side-upload.test.js b/tests/e2e/specs/media-new-client-side-upload.test.js index c9a3601351444..3fd602ed0405b 100644 --- a/tests/e2e/specs/media-new-client-side-upload.test.js +++ b/tests/e2e/specs/media-new-client-side-upload.test.js @@ -1,11 +1,7 @@ -/** - * WordPress dependencies - */ -import { test, expect } from '@wordpress/e2e-test-utils-playwright'; - /** * Internal dependencies */ +import { test, expect } from '../fixtures'; import { TEST_IMAGE_PATH, TEST_AUDIO_PATH, @@ -32,6 +28,41 @@ function editLinks( page ) { .getByRole( 'link', { name: 'Edit', exact: true } ); } +/** + * Tracks each attachment the page creates, through the REST API or through + * async-upload.php, so that the attachments fixture deletes it. + * + * @param {import('@playwright/test').Page} page + * @param {Object} attachments The attachments fixture. + * @return {number[]} The attachment ids, in the order they were created. + */ +function trackUploads( page, attachments ) { + const ids = []; + page.on( 'response', async ( response ) => { + if ( response.request().method() !== 'POST' || ! response.ok() ) { + return; + } + const url = decodeURIComponent( response.url() ); + let id; + try { + if ( /\/wp\/v2\/media(?:[?&]|$)/.test( url ) ) { + id = ( await response.json() ).id; + } else if ( url.includes( '/async-upload.php' ) ) { + // With short=1 the classic upload answers with the bare id. + id = Number( await response.text() ); + } + } catch { + // The page closed before the body arrived. + return; + } + if ( Number.isInteger( id ) && id > 0 ) { + ids.push( id ); + attachments.track( id ); + } + } ); + return ids; +} + /** * Adjusts the settings plupload-handlers.js builds the uploader from. * @@ -65,11 +96,6 @@ async function overrideUploaderInit( page, adjust ) { } test.describe( 'Add New Media File client-side uploads', () => { - test.afterEach( async ( { requestUtils } ) => { - await requestUtils.deleteAllMedia(); - await requestUtils.deleteAllPosts(); - } ); - test( 'sends the Document-Isolation-Policy header', async ( { page, admin, @@ -92,6 +118,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'uploads an image through the client-side pipeline', async ( { page, admin, + attachments, requestUtils, } ) => { await admin.visitAdminPage( 'media-new.php' ); @@ -139,6 +166,7 @@ test.describe( 'Add New Media File client-side uploads', () => { } } ); + const ids = trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( TEST_IMAGE_PATH ); @@ -159,9 +187,9 @@ test.describe( 'Add New Media File client-side uploads', () => { // The finalized attachment carries the browser-generated sub-sizes // in its metadata (the 640x480 source is larger than thumbnail and // medium), and the sideloaded thumbnail file really exists. - const [ attachment ] = await requestUtils.rest( { - path: '/wp/v2/media', - params: { per_page: 1 }, + await expect.poll( () => ids.length ).toBeGreaterThan( 0 ); + const attachment = await requestUtils.rest( { + path: `/wp/v2/media/${ ids.at( -1 ) }`, } ); const sizes = attachment.media_details.sizes || {}; expect( Object.keys( sizes ) ).toEqual( @@ -177,6 +205,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'warns before leaving while a pipeline upload is in flight', async ( { page, admin, + attachments, } ) => { await admin.visitAdminPage( 'media-new.php' ); @@ -208,6 +237,7 @@ test.describe( 'Add New Media File client-side uploads', () => { { timeout: 60_000 } ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( TEST_IMAGE_PATH ); @@ -246,6 +276,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'shows an error for a disallowed file type', async ( { page, admin, + attachments, } ) => { await admin.visitAdminPage( 'media-new.php' ); @@ -257,6 +288,7 @@ test.describe( 'Add New Media File client-side uploads', () => { 'The client-side pipeline requires a cross-origin isolated context' ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( { @@ -282,11 +314,13 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'attaches the upload to the post named by post_id', async ( { page, admin, + attachments, + postFactory, requestUtils, } ) => { // Published, so the attachment (post_status 'inherit') stays visible // in the media collection. - const post = await requestUtils.createPost( { + const post = await postFactory.create( { title: 'Client-side upload parent', status: 'publish', } ); @@ -301,6 +335,7 @@ test.describe( 'Add New Media File client-side uploads', () => { 'The client-side pipeline requires a cross-origin isolated context' ); + const ids = trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( TEST_IMAGE_PATH ); @@ -310,15 +345,16 @@ test.describe( 'Add New Media File client-side uploads', () => { // The classic flow posts `post_id` to async-upload.php; the pipeline // has to send the REST equivalent or the file lands unattached even // though the screen counts it against the post. - const [ attachment ] = await requestUtils.rest( { - path: '/wp/v2/media', - params: { per_page: 1 }, + await expect.poll( () => ids.length ).toBeGreaterThan( 0 ); + const attachment = await requestUtils.rest( { + path: `/wp/v2/media/${ ids.at( -1 ) }`, } ); expect( attachment.post ).toBe( post.id ); } ); test( 'falls back to the classic uploader when the page is not isolated', async ( { page, admin, + attachments, } ) => { // Blocking the Document-Isolation-Policy header reproduces every // browser that ignores it: the page is not isolated, the script @@ -358,6 +394,7 @@ test.describe( 'Add New Media File client-side uploads', () => { } } ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( TEST_IMAGE_PATH ); @@ -371,6 +408,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'uploads several files at once, including duplicates', async ( { page, admin, + attachments, } ) => { await admin.visitAdminPage( 'media-new.php' ); @@ -394,6 +432,7 @@ test.describe( 'Add New Media File client-side uploads', () => { } } ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( [ TEST_IMAGE_PATH, TEST_IMAGE_PATH ] ); @@ -403,7 +442,11 @@ test.describe( 'Add New Media File client-side uploads', () => { expect( finalizeCount ).toBe( 2 ); } ); - test( 'reports pipeline progress on the item', async ( { page, admin } ) => { + test( 'reports pipeline progress on the item', async ( { + page, + admin, + attachments, + } ) => { await admin.visitAdminPage( 'media-new.php' ); const isolated = await page.evaluate( () => @@ -432,6 +475,7 @@ test.describe( 'Add New Media File client-side uploads', () => { { timeout: 60_000 } ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( TEST_IMAGE_PATH ); @@ -456,6 +500,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'surfaces a pipeline error with the server message', async ( { page, admin, + attachments, } ) => { await admin.visitAdminPage( 'media-new.php' ); @@ -469,6 +514,7 @@ test.describe( 'Add New Media File client-side uploads', () => { await failMediaCreate( page ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( TEST_IMAGE_PATH ); @@ -484,6 +530,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'uploads unattached when post_id is not a valid post', async ( { page, admin, + attachments, requestUtils, } ) => { // media_upload_form() hands plupload the raw post_id, but the screen @@ -500,6 +547,7 @@ test.describe( 'Add New Media File client-side uploads', () => { 'The client-side pipeline requires a cross-origin isolated context' ); + const ids = trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( TEST_IMAGE_PATH ); @@ -509,9 +557,9 @@ test.describe( 'Add New Media File client-side uploads', () => { 0 ); - const [ attachment ] = await requestUtils.rest( { - path: '/wp/v2/media', - params: { per_page: 1 }, + await expect.poll( () => ids.length ).toBeGreaterThan( 0 ); + const attachment = await requestUtils.rest( { + path: `/wp/v2/media/${ ids.at( -1 ) }`, } ); expect( attachment.post ).toBeNull(); } ); @@ -519,6 +567,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'forwards plupload multipart params to the REST upload', async ( { page, admin, + attachments, } ) => { await admin.visitAdminPage( 'media-new.php' ); @@ -559,6 +608,7 @@ test.describe( 'Add New Media File client-side uploads', () => { }; } ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( TEST_IMAGE_PATH ); @@ -578,6 +628,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'leaves audio files to the classic uploader', async ( { page, admin, + attachments, } ) => { await admin.visitAdminPage( 'media-new.php' ); @@ -606,6 +657,7 @@ test.describe( 'Add New Media File client-side uploads', () => { } } ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( TEST_AUDIO_PATH ); @@ -619,6 +671,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'splits a batch of an image and audio between the pipeline and the classic uploader', async ( { page, admin, + attachments, } ) => { await admin.visitAdminPage( 'media-new.php' ); @@ -646,6 +699,7 @@ test.describe( 'Add New Media File client-side uploads', () => { } } ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( [ TEST_IMAGE_PATH, TEST_AUDIO_PATH ] ); @@ -660,6 +714,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'leaves JPEG images to the classic uploader while plupload resizes them', async ( { page, admin, + attachments, } ) => { await admin.visitAdminPage( 'media-new.php' ); @@ -691,6 +746,7 @@ test.describe( 'Add New Media File client-side uploads', () => { } } ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( TEST_IMAGE_PATH ); @@ -704,6 +760,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'renders a failed upload with an accessible Dismiss button', async ( { page, admin, + attachments, } ) => { await admin.visitAdminPage( 'media-new.php' ); @@ -717,6 +774,7 @@ test.describe( 'Add New Media File client-side uploads', () => { await failMediaCreate( page ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( TEST_IMAGE_PATH ); @@ -746,6 +804,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'hands a HEIC the browser cannot decode to the server when it can convert it', async ( { page, admin, + attachments, } ) => { // No heic_upload_error means the server's image editor handles HEIC. await overrideUploaderInit( page, ( settings ) => { @@ -781,6 +840,7 @@ test.describe( 'Add New Media File client-side uploads', () => { } } ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( UNDECODABLE_HEIC ); @@ -810,6 +870,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'keeps a PostInit handler another script placed in wpUploaderInit', async ( { page, admin, + attachments, } ) => { // A plugin that adds its own handler map before the uploader is // created must still see its PostInit run once the pipeline hooks in. @@ -832,6 +893,7 @@ test.describe( 'Add New Media File client-side uploads', () => { 'The client-side pipeline requires a cross-origin isolated context' ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); @@ -860,6 +922,7 @@ test.describe( 'Add New Media File client-side uploads', () => { test( 'fires uploadComplete only once plupload has finished its share of a batch', async ( { page, admin, + attachments, } ) => { await admin.visitAdminPage( 'media-new.php' ); @@ -890,6 +953,7 @@ test.describe( 'Add New Media File client-side uploads', () => { }; } ); + trackUploads( page, attachments ); const fileInput = page.locator( FILE_INPUT_SELECTOR ).first(); await expect( fileInput ).toBeAttached( { timeout: 30_000 } ); await fileInput.setInputFiles( [ TEST_IMAGE_PATH, TEST_AUDIO_PATH ] ); From dc5fa049051b3c26f0fe1fbf5b306ce6429e79ec Mon Sep 17 00:00:00 2001 From: Adrian Moldovan <3854374+adimoldovan@users.noreply.github.com> Date: Wed, 11 Nov 2026 15:32:49 +0200 Subject: [PATCH 2/3] Build/Test Tools: Track uploads by the attachment ID header. Core sends `X-WP-Upload-Attachment-ID` after it saves the attachment and before it makes the sub-sizes. Reading it tracks an upload that fails with a 500 after the save, which the old check skipped. The header arrives before the body, so the id is tracked before teardown can run. See #66392. --- .../media-new-client-side-upload.test.js | 31 +++++-------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/tests/e2e/specs/media-new-client-side-upload.test.js b/tests/e2e/specs/media-new-client-side-upload.test.js index 3fd602ed0405b..e8fb061966fe7 100644 --- a/tests/e2e/specs/media-new-client-side-upload.test.js +++ b/tests/e2e/specs/media-new-client-side-upload.test.js @@ -29,8 +29,11 @@ function editLinks( page ) { } /** - * Tracks each attachment the page creates, through the REST API or through - * async-upload.php, so that the attachments fixture deletes it. + * Tracks each attachment the page creates, so that the attachments fixture + * deletes it. + * + * Core sends the id in a header before it makes the sub-sizes, so an upload + * that fails after the attachment is saved is tracked too. * * @param {import('@playwright/test').Page} page * @param {Object} attachments The attachments fixture. @@ -38,24 +41,9 @@ function editLinks( page ) { */ function trackUploads( page, attachments ) { const ids = []; - page.on( 'response', async ( response ) => { - if ( response.request().method() !== 'POST' || ! response.ok() ) { - return; - } - const url = decodeURIComponent( response.url() ); - let id; - try { - if ( /\/wp\/v2\/media(?:[?&]|$)/.test( url ) ) { - id = ( await response.json() ).id; - } else if ( url.includes( '/async-upload.php' ) ) { - // With short=1 the classic upload answers with the bare id. - id = Number( await response.text() ); - } - } catch { - // The page closed before the body arrived. - return; - } - if ( Number.isInteger( id ) && id > 0 ) { + page.on( 'response', ( response ) => { + const id = Number( response.headers()[ 'x-wp-upload-attachment-id' ] ); + if ( id > 0 ) { ids.push( id ); attachments.track( id ); } @@ -187,7 +175,6 @@ test.describe( 'Add New Media File client-side uploads', () => { // The finalized attachment carries the browser-generated sub-sizes // in its metadata (the 640x480 source is larger than thumbnail and // medium), and the sideloaded thumbnail file really exists. - await expect.poll( () => ids.length ).toBeGreaterThan( 0 ); const attachment = await requestUtils.rest( { path: `/wp/v2/media/${ ids.at( -1 ) }`, } ); @@ -345,7 +332,6 @@ test.describe( 'Add New Media File client-side uploads', () => { // The classic flow posts `post_id` to async-upload.php; the pipeline // has to send the REST equivalent or the file lands unattached even // though the screen counts it against the post. - await expect.poll( () => ids.length ).toBeGreaterThan( 0 ); const attachment = await requestUtils.rest( { path: `/wp/v2/media/${ ids.at( -1 ) }`, } ); @@ -557,7 +543,6 @@ test.describe( 'Add New Media File client-side uploads', () => { 0 ); - await expect.poll( () => ids.length ).toBeGreaterThan( 0 ); const attachment = await requestUtils.rest( { path: `/wp/v2/media/${ ids.at( -1 ) }`, } ); From 327c833edcc7085dafdc60992b3e8781382256c2 Mon Sep 17 00:00:00 2001 From: Adrian Moldovan <3854374+adimoldovan@users.noreply.github.com> Date: Wed, 11 Nov 2026 16:05:59 +0200 Subject: [PATCH 3/3] Build/Test Tools: Track a REST upload that fails after the save. `requestUtils.uploadMedia()` throws away the response on an error. If the server saves the attachment and then fails while it makes the sub-sizes, the fixture never learned the id, and the attachment stayed. `upload()` now posts the file itself and reads the id from `X-WP-Upload-Attachment-ID` before it checks the status. See #66392. --- tests/e2e/fixtures/attachment.js | 35 +++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/tests/e2e/fixtures/attachment.js b/tests/e2e/fixtures/attachment.js index 2083d120ee640..4def8a4876671 100644 --- a/tests/e2e/fixtures/attachment.js +++ b/tests/e2e/fixtures/attachment.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import { createReadStream } from 'node:fs'; + /** * Uploads attachments, records the ones the browser uploads, then deletes * them in teardown. @@ -12,9 +17,33 @@ export const attachments = async ( { requestUtils }, use ) => { await use( { async upload( absolutePath ) { - const attachment = await requestUtils.uploadMedia( absolutePath ); - ids.add( attachment.id ); - return attachment; + // Not requestUtils.uploadMedia(): it throws away the response on an + // error, but the server can save the attachment and then fail while + // it makes the sub-sizes. Core sends the id in a header before that. + if ( ! requestUtils.storageState?.nonce ) { + await requestUtils.setupRest(); + } + const { rootURL, nonce } = requestUtils.storageState; + const response = await requestUtils.request.post( + `${ rootURL }wp/v2/media`, + { + headers: { 'X-WP-Nonce': nonce }, + multipart: { file: createReadStream( absolutePath ) }, + failOnStatusCode: false, + } + ); + const id = Number( + response.headers()[ 'x-wp-upload-attachment-id' ] + ); + if ( id > 0 ) { + ids.add( id ); + } + if ( ! response.ok() ) { + throw new Error( + `Upload failed with HTTP ${ response.status() }: ${ await response.text() }` + ); + } + return response.json(); }, track( id ) { ids.add( id );