Fill in E2E tests for client-side media processing#75949
Merged
Conversation
|
Size Change: 0 B Total Size: 6.84 MB ℹ️ View Unchanged
|
|
Size Change: 0 B Total Size: 7.82 MB ℹ️ View Unchanged
|
Adds image files covering format, transparency, EXIF rotation, animation, and oversized dimensions for use in client-side media processing E2E tests. Includes a Node.js generation script for reproducibility.
Plugins for E2E tests: PNG-to-JPEG conversion, JPEG-to-WebP conversion, and progressive/interlaced image saving via WordPress filters.
Covers upload, compression, sub-size generation, batch uploads, server fallback, error handling, browser capabilities, MIME format conversion, EXIF rotation, oversized scaling, and transparency preservation.
Measures single image, large image with processing, and multiple simultaneous upload timings. Adds new metrics to the performance reporter.
- Remove unused `page` destructuring from test functions that access page via fixture - Fix prettier formatting for single-line locator and path.join calls - Add JSDoc type annotations to pushBits/bitsToBytes functions and bits variable - Add eslint-enable directive for playwright/expect-expect in media-upload perf test - Remove unused eslint-disable-next-line for playwright/no-skipped-test in fixture method Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add eslint-disable no-bitwise for binary file generator script - Remove unused channels variable in createPNG - Fix prettier formatting in generate-test-media-assets.mjs - Fix JSDoc alignment, getSelectedBlockImageId formatting, and locator string formatting in client-side-media-processing E2E spec Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Increase compression size tolerance from 3x to 5x for varying server configs - Accept both original and converted MIME types for format conversion tests - Accept both rotated and non-rotated dimensions for EXIF orientation test - Fix error handling test to check both snackbar and notice components Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
a3211e0 to
bdcae66
Compare
The test was waiting for a snackbar/error notice that never appears due to a double-unwrapping bug in the error callback chain between the editor and upload-media packages. Instead, assert that the upload queue drains and the image block remains in placeholder state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test cannot reliably intercept uploads made from within the editor iframe's fetch context during client-side media processing. Additionally, the error callback chain has a double-unwrapping bug that prevents errors from surfacing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
The 3200x2400 responsive lightbox JPEG was committed as a test asset but was not included in the generator script.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Flaky tests detected in 4af525c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25123808965
|
Contributor
andrewserong
left a comment
There was a problem hiding this comment.
Just looking over the additional tests, and I'm wondering if they're a bit verbose / if we're doing too much granular testing for e2es. I.e. if much of the conversion is already covered by unit tests, do we need all the different e2e test configurations? And if some of the tests are skipped if not cross origin isolated, how useful are they to be included at this stage?
There seems to be a bit of duplication, and lots of separate page loads, so I wondered if they can be simplified. In general, e2e tests tend to take a long time to run, so that's the main reason I wondered about it 🙂
Another thing is I'm wondering if the included .webp file is valid? It doesn't appear to load for me:
I do think it's a good idea to add e2es, though!
| * Plugin URI: https://github.com/WordPress/gutenberg | ||
| * Author: Gutenberg Team | ||
| * | ||
| * @package gutenberg-test-image-interlaced-progressive |
Contributor
There was a problem hiding this comment.
Is this plugin used? If not can it be removed?
Comment on lines
+5
to
+10
| /** | ||
| * Generate test media assets for client-side media processing E2E tests. | ||
| * | ||
| * Uses raw binary construction to create valid image files without external dependencies. | ||
| * Run: node bin/generate-test-media-assets.mjs | ||
| */ |
Contributor
There was a problem hiding this comment.
This file is clever, and in principle I like the idea of making it easy to generate or regenerate images as needed. But it seems tangential to the task of adding test image assets to the repo for e2e tests. Is it necessary to include it? How frequently do we imagine needing to add new assets?
Member
Author
There was a problem hiding this comment.
no we can just include the static images
Member
Author
I will simplify what I have here and remove some of what is already covered elsewhere.
My assumption was some CI runs would use an environment that was cross origin isolated, let me double check that is the case.
yes, probably we can combine a bit and still get the same coverage. thanks for the feedback.
oh, nope! will fix.
🎉 |
Address reviewer feedback by consolidating redundant tests: - Merge duplicate JPEG tests (basic + MIME type + below threshold) into one - Merge duplicate PNG tests (basic + MIME type) into one - Merge 3 sub-size tests into one comprehensive test - Merge gallery and batch completion tests - Merge 3 browser capability tests into one - Remove duplicate transparency test from Special handling section - Remove unused image-interlaced-progressive.php plugin - Remove generate-test-media-assets.mjs generator script - Remove unused .webp and .avif test assets Reduces from ~21 active tests to 12, cutting page loads significantly.
Add E2E tests that upload WebP and AVIF images to exercise the wasm-vips decode path for these formats. Previously only JPEG, PNG, and GIF uploads were tested. Include valid static test assets generated from real pixel data.
Contributor
|
Nice improvements, just gave it a skim!
Gotcha, thanks for confirming. I see some of the tests are quite permissive in their assertions, e.g. things like: gutenberg/test/e2e/specs/editor/various/client-side-media-processing.spec.js Lines 653 to 657 in 4c659e8 Since this test file is for client-side media processing, I'm wondering if we should skip these tests if client-side media processing isn't available, and then we could be more specific with the assertion? At the moment some of these read a bit like they're trying to cover when client-side process is enabled and also when it isn't enabled? Because we've already got e2e tests for the image and gallery blocks, etc, I'm thinking we can probably be a bit more specific in these tests and not worry about covering when client-side isn't available. I guess it comes back to why we have these tests. Is it to cover the specific client-side processing, or to cover that plus fallbacks 🤔 What do you think? |
Union the performance-reporter metric types so both the PR's E2E media upload benchmarks and trunk's media-processing benchmarks (#76792) are preserved.
Member
Author
Yes, the goal is to cover primarily client side media features so your point is valid. Plus these loose assertions like your example don't do much. I'll give this a pass to make the tests more focused on CSM. |
…c behavior Address review feedback on #75949: assertions were permissive enough to pass whether CSM ran or the server-side path took over. The image and gallery block e2es already cover the non-CSM path, so this spec should focus on behavior that is only true when the CSM pipeline runs. - Skip every test when CSM is not the active upload path (mirrors the `__clientSideMediaProcessing` + feature-detection gate used by the editor). - Drop the "fall back to server-side" test; that path is covered elsewhere and the assertions duplicated existing image-block e2es. - Drop the standalone "verify browser capabilities" test; the new skip helper enforces those preconditions for every test. - Replace mime-type unions (`[A, B].toContain`) with exact `toBe` assertions: WebP stays WebP, AVIF stays AVIF, and filtered conversions must produce the configured target. - Require thumbnail/medium/large sub-sizes (and assert the scaled main file's exact dimensions) instead of the previous "at least one size" check. - Require the rotated dimensions for the EXIF-rotated asset rather than accepting the unrotated original. - Extract the shared insert/upload/wait/fetch flow into a helper.
Contributor
|
Thanks for the update 👍 Just re-looking at this, and a couple of questions about the performance tests. What value do they have over the existing media processing tests? I.e. if we introduced a performance regression, would these added tests capture anything that wouldn't be captured by the existing tests? I imagine the goal here is to make sure we're covering the full e2e range, which makes sense, but I'm wondering a tiny bit how useful they'll be in practice. That said, I don't want to be a blocker here and I see this PR has been open for a long time now. The one potential issue I noticed is that there's no |
Contributor
Member
Author
These benchmarks belong with the throwaway-detection work being landed in #76707. Keep this PR focused on E2E coverage for the client-side media processing pipeline.
Member
Author
|
Thanks @andrewserong — agreed. I've removed the performance tests ( |
andrewserong
approved these changes
Apr 30, 2026
Contributor
andrewserong
left a comment
There was a problem hiding this comment.
Thanks for all the back and forth on this! 🎉
talldan
pushed a commit
that referenced
this pull request
May 8, 2026
* Add test image assets for media processing E2E Adds image files covering format, transparency, EXIF rotation, animation, and oversized dimensions for use in client-side media processing E2E tests. Includes a Node.js generation script for reproducibility. * Add test plugins for image format conversion Plugins for E2E tests: PNG-to-JPEG conversion, JPEG-to-WebP conversion, and progressive/interlaced image saving via WordPress filters. * Add E2E tests for client-side media processing Covers upload, compression, sub-size generation, batch uploads, server fallback, error handling, browser capabilities, MIME format conversion, EXIF rotation, oversized scaling, and transparency preservation. * Add performance benchmarks for media upload Measures single image, large image with processing, and multiple simultaneous upload timings. Adds new metrics to the performance reporter. * Fix lint and TypeScript errors in E2E and performance tests - Remove unused `page` destructuring from test functions that access page via fixture - Fix prettier formatting for single-line locator and path.join calls - Add JSDoc type annotations to pushBits/bitsToBytes functions and bits variable - Add eslint-enable directive for playwright/expect-expect in media-upload perf test - Remove unused eslint-disable-next-line for playwright/no-skipped-test in fixture method Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix remaining lint and prettier formatting errors - Add eslint-disable no-bitwise for binary file generator script - Remove unused channels variable in createPNG - Fix prettier formatting in generate-test-media-assets.mjs - Fix JSDoc alignment, getSelectedBlockImageId formatting, and locator string formatting in client-side-media-processing E2E spec Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix E2E test assertions for CI environment compatibility - Increase compression size tolerance from 3x to 5x for varying server configs - Accept both original and converted MIME types for format conversion tests - Accept both rotated and non-rotated dimensions for EXIF orientation test - Fix error handling test to check both snackbar and notice components Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix server upload failure test to assert on block state The test was waiting for a snackbar/error notice that never appears due to a double-unwrapping bug in the error callback chain between the editor and upload-media packages. Instead, assert that the upload queue drains and the image block remains in placeholder state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Skip flaky server upload failure test The test cannot reliably intercept uploads made from within the editor iframe's fetch context during client-side media processing. Additionally, the error callback chain has a double-unwrapping bug that prevents errors from surfacing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add missing responsive lightbox asset to generator The 3200x2400 responsive lightbox JPEG was committed as a test asset but was not included in the generator script. * Fix prettier formatting in generate-test-media-assets.mjs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Simplify E2E tests: reduce duplication and page loads Address reviewer feedback by consolidating redundant tests: - Merge duplicate JPEG tests (basic + MIME type + below threshold) into one - Merge duplicate PNG tests (basic + MIME type) into one - Merge 3 sub-size tests into one comprehensive test - Merge gallery and batch completion tests - Merge 3 browser capability tests into one - Remove duplicate transparency test from Special handling section - Remove unused image-interlaced-progressive.php plugin - Remove generate-test-media-assets.mjs generator script - Remove unused .webp and .avif test assets Reduces from ~21 active tests to 12, cutting page loads significantly. * Add WebP and AVIF upload tests for decode coverage Add E2E tests that upload WebP and AVIF images to exercise the wasm-vips decode path for these formats. Previously only JPEG, PNG, and GIF uploads were tested. Include valid static test assets generated from real pixel data. * E2E: Tighten client-side media processing tests to assert CSM-specific behavior Address review feedback on #75949: assertions were permissive enough to pass whether CSM ran or the server-side path took over. The image and gallery block e2es already cover the non-CSM path, so this spec should focus on behavior that is only true when the CSM pipeline runs. - Skip every test when CSM is not the active upload path (mirrors the `__clientSideMediaProcessing` + feature-detection gate used by the editor). - Drop the "fall back to server-side" test; that path is covered elsewhere and the assertions duplicated existing image-block e2es. - Drop the standalone "verify browser capabilities" test; the new skip helper enforces those preconditions for every test. - Replace mime-type unions (`[A, B].toContain`) with exact `toBe` assertions: WebP stays WebP, AVIF stays AVIF, and filtered conversions must produce the configured target. - Require thumbnail/medium/large sub-sizes (and assert the scaled main file's exact dimensions) instead of the previous "at least one size" check. - Require the rotated dimensions for the EXIF-rotated asset rather than accepting the unrotated original. - Extract the shared insert/upload/wait/fetch flow into a helper. * E2E: Remove media upload performance tests These benchmarks belong with the throwaway-detection work being landed in #76707. Keep this PR focused on E2E coverage for the client-side media processing pipeline. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
| const path = require( 'path' ); | ||
| const fs = require( 'fs/promises' ); | ||
| const os = require( 'os' ); | ||
| const { v4: uuid } = require( 'uuid' ); |
Member
There was a problem hiding this comment.
We moved to randomUUID in #77960 to avoid the ESM/CJS issues caused by uuid. If we still want to use uuid her, then we may want to declare it as a dependency in test/e2e/package.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #74367.
Performance benchmarks for media upload have been removed from this PR and will be addressed in #76707, which already includes the related
throwawaydetection implementation.Test plan
client-side-media-processing.spec.jsbig-image-size-threshold.spec.jstests still pass