Skip to content

Build/Test Tools: Run the Media Library grid and media modal E2E specs in parallel. - #15903

Closed
adimoldovan wants to merge 4 commits into
WordPress:trunkfrom
adimoldovan:e2e/media-grid-modal-parallel
Closed

adimoldovan wants to merge 4 commits into
WordPress:trunkfrom
adimoldovan:e2e/media-grid-modal-parallel

Conversation

@adimoldovan

@adimoldovan adimoldovan commented Nov 11, 2026 •

Copy link
Copy Markdown

Trac ticket: https://core.trac.wordpress.org/ticket/66392

media-library-client-side-upload.test.js and media-modal-client-side-upload.test.js deleted all media after each test, so they had to run in the serial project. All their uploads also had the same file name, so a test found its tile by a name that other specs' uploads share. In parallel, the cleanup would delete other workers' files, and the tile lookups and the "newest attachment" reads could match another spec's upload.

Each test now:

  • gives each upload that can create an attachment a unique file name, so its tile name is unique;
  • reads its attachments by the ids that the page's upload responses return in the X-WP-Upload-Attachment-ID header;
  • lets the attachments fixture delete only what it uploaded.

The modal spec creates its posts with postFactory and opens them on post.php, so its tests no longer run on post-new.php. Both specs move to parallel.

trackUploads() moves to tests/e2e/utils/media-pipeline.js, so the three client-side upload specs share it. A new uniqueFile() helper gives a test file a unique name:

const file = uniqueFile( TEST_IMAGE_PATH );
await fileInput.setInputFiles( file );
await expect( tile( page, titleOf( file ) ) ).toBeVisible();

Testing Instructions

  1. Run npm run env:cli -- post list --post_type=attachment --format=count and note the number.
  2. Run npm run test:e2e -- media-library-client-side-upload media-modal-client-side-upload. All 24 tests pass.
  3. Repeat step 1. The number is the same.
  4. Run npm run test:e2e -- --project=parallel --shuffle --repeat-each=2. Every run of the two specs passes.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Code change, test runs and this description.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

`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.
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.
…s in parallel.

`media-library-client-side-upload.test.js` and `media-modal-client-side-upload.test.js` deleted all media after each test, so they had to run in the serial project. Each test now gives its uploads a unique name, finds its tiles by that name, and reads its attachments by id. The `attachments` fixture deletes only what the test uploaded. The modal spec creates its posts with `postFactory`.

`trackUploads()` moves to `utils/media-pipeline.js`, so all three media specs share it.

See #66392.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues remain for the changes isolating media-upload E2E tests for parallel execution.

No open issues 🎉

What changed in this PR

Builds on #15896 to make the Media Library grid and media modal upload E2E specs safe to run in parallel on a shared WordPress test site.

Changes:

  • Uses unique upload filenames and response-provided attachment IDs to avoid matching other tests’ media.
  • Replaces global cleanup with fixture-managed attachments and posts, and opens modal tests on existing posts.
  • Shares upload-tracking utilities across all three client-side upload specs and moves the grid and modal specs into the parallel project.
File Description
tests/​e2e/​utils/​media-pipeline.js Adds unique-file and title helpers and centralizes upload tracking.
tests/​e2e/​specs/​media-new-client-side-upload.test.js Uses scoped fixtures, attachment IDs, and shared upload tracking.
tests/​e2e/​specs/​media-modal-client-side-upload.test.js Isolates uploads and uses fixture-created posts for modal tests.
tests/​e2e/​specs/​media-library-client-side-upload.test.js Scopes tile lookups, attachment assertions, and cleanup to each test.
tests/​e2e/​README.md Documents attachment cleanup and shared-fixture placement.
tests/​e2e/​playwright.config.js Removes client-side upload specs from the serial list.
tests/​e2e/​fixtures/​index.js Registers the attachments fixture.
tests/​e2e/​fixtures/​attachment.js Adds per-test attachment tracking and cleanup.

Review effort: Balanced

@adimoldovan
adimoldovan marked this pull request as ready for review November 11, 2026 14:29
@adimoldovan
adimoldovan requested a review from a team November 11, 2026 14:29
@github-actions

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs.

Core Committers: Use this line as a base for the props when committing in SVN:

Props adrianmoldovanwp.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.


Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

  • @propsbot add <username> to give props to a GitHub user. Requires write access.
  • @propsbot remove <username> to leave a user out of the props. Requires write access.
  • @propsbot refresh to refresh this list. Adding the props-bot label works too.

…al-parallel

# Conflicts:
#	tests/e2e/fixtures/attachment.js
#	tests/e2e/playwright.config.js

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues remain for the media upload test-isolation changes.

No open issues 🎉

Review effort: Balanced

@lancewillett lancewillett left a comment

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.

The parallel migration is supported by the existing upload tests: with the proposed parallel configuration held fixed, two workers and three repetitions produced 2 failures/4 passes using the base specs, and 6 passes using this head, without retries. This comparison exercises the migration; it does not indicate a failure under the original serial configuration.

The changes preserve the upload assertions while giving each test its own attachment identity and cleanup. All four E2E CI jobs and type checking passed, and the refreshed Copilot review has no open issues.

AI review · gpt-6

@lancewillett lancewillett moved this from In review to Ready in WordPress Project Build Tooling Nov 11, 2026
wporg-sync pushed a commit that referenced this pull request Nov 11, 2026
Use unique upload names and per-test attachment and post cleanup.

Developed in: #15903

Props adrianmoldovanwp.
See #66392.


git-svn-id: https://develop.svn.wordpress.org/trunk@65403 602fd350-edb4-49c9-b593-d223f7449a82
@lancewillett

Copy link
Copy Markdown
Member

wporg-sync pushed a commit to WordPress/WordPress that referenced this pull request Nov 11, 2026
Use unique upload names and per-test attachment and post cleanup.

Developed in: WordPress/wordpress-develop#15903

Props adrianmoldovanwp.
See #66392.

Built from https://develop.svn.wordpress.org/trunk@65403


git-svn-id: http://core.svn.wordpress.org/trunk@64542 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants