Skip to content
Merged
Changes from all commits
Commits
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
55 changes: 53 additions & 2 deletions test/performance/specs/media-upload.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,49 @@ async function createTempImage( sourceFile, ext ) {
return { tmpFileName, tmpDirectory };
}

/**
* Waits for the client-side upload queue to fully settle.
*
* With client-side media processing, the image `src` flips to the server URL
* as soon as the main file is uploaded, but thumbnail and original-image
* sideloads plus finalization are still in flight. Cleaning up (deleting
* media) while those requests are pending deletes the attachment out from
* under them, causing 404s and intermittently wedging the upload queue so
* the next upload never completes. See
* https://github.com/WordPress/gutenberg/issues/79923.
*
* @param {Object} page Playwright page.
*/
async function waitForUploadQueueEmpty( page ) {
await page.waitForFunction(
() => {
const store = window.wp.data.select( 'core/upload-media' );
return ! store || store.getItems().length === 0;
},
undefined,
{ timeout: 120_000 }
);
}
Comment on lines +57 to +66

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we have at least three similar helpers with various timeouts. I don't mind duplications, but it seems we're just using arbitrary numbers to solve flaky tests.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better way to wait for the queue to empty here?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not, but it would be nice if helpers were the same, rather than having 3-4 almost identical implementations. Plus was wondering why choosing different timeouts.

Not a blocker here, I can follow up with a consolidation PR.


/**
* Deletes all media as a best effort, tolerating already-deleted items.
*
* When an upload errors client-side after the attachment was created, the
* upload queue deletes its own orphaned attachment in a fire-and-forget
* request. That request can race this cleanup, making one of the deletes
* 404. Leftovers, if any, are swept by the next iteration's cleanup.
*
* @param {Object} requestUtils Request utility.
*/
async function deleteAllMediaBestEffort( requestUtils ) {
try {
await requestUtils.deleteAllMedia();
} catch ( error ) {
// eslint-disable-next-line no-console
console.warn( `Media cleanup failed, continuing: ${ error.message }` );
}
}

/**
* Runs upload iterations for a single image variant within one editor lifecycle.
*
Expand Down Expand Up @@ -99,13 +142,17 @@ async function runUploadIterations( {
bucket.push( elapsed );
}

// Let pending sideloads/finalization finish before cleanup so
// deleting media doesn't race in-flight requests.
await waitForUploadQueueEmpty( page );

// Reset state for next iteration. Use resetBlocks rather than
// selecting and pressing Backspace because after upload, DOM focus
// is inside the block and Backspace is consumed by an inner element.
await page.evaluate( () => {
window.wp.data.dispatch( 'core/block-editor' ).resetBlocks( [] );
} );
await requestUtils.deleteAllMedia();
await deleteAllMediaBestEffort( requestUtils );
await fs.rm( tmpDirectory, {
recursive: true,
force: true,
Expand Down Expand Up @@ -246,13 +293,17 @@ test.describe( 'Media Upload Performance', () => {
results.multipleImageUploadProcessing.push( elapsed );
}

// Let pending sideloads/finalization finish before cleanup
// so deleting media doesn't race in-flight requests.
await waitForUploadQueueEmpty( page );

// Reset state for next iteration.
await page.evaluate( () => {
window.wp.data
.dispatch( 'core/block-editor' )
.resetBlocks( [] );
} );
await requestUtils.deleteAllMedia();
await deleteAllMediaBestEffort( requestUtils );
await fs.rm( tmpDirectory, {
recursive: true,
force: true,
Expand Down
Loading