From ff7302068e098b31f9fd8198799dacb6b81686a6 Mon Sep 17 00:00:00 2001 From: Utkarsh Patel Date: Tue, 20 Oct 2026 17:08:04 +0530 Subject: [PATCH] test(e2e): Run live delivery specs in parallel Five specs were tagged @serial only because they need the real Cloudinary connection, which the analytics specs overwrite with fake credentials. They do not break each other, so run them in their own multi-worker pass, tagged @live, between the analytics pass and the serial pass. They share one site, so the steps that act on site-wide state take turns through a file lock in tests/e2e/utils/live-site.js: `wp cloudinary sync`, whose analyze step clears and re-adds `_cld_unsynced` on every attachment, the gallery's image_data sync, post cleanup, and settings writes to the shared media_display option. --- package.json | 5 +- tests/e2e/cloudinary-asset-edit.spec.js | 25 +-- tests/e2e/cloudinary-gallery-delivery.spec.js | 36 ++-- tests/e2e/cloudinary-image-delivery.spec.js | 31 +-- tests/e2e/cloudinary-svg-delivery.spec.js | 35 ++-- tests/e2e/cloudinary-video-delivery.spec.js | 43 ++-- tests/e2e/playwright.config.js | 10 +- tests/e2e/utils/live-site.js | 183 ++++++++++++++++++ 8 files changed, 267 insertions(+), 101 deletions(-) create mode 100644 tests/e2e/utils/live-site.js diff --git a/package.json b/package.json index 1d809f29f..29aec92d8 100644 --- a/package.json +++ b/package.json @@ -40,8 +40,9 @@ "postinstall": "patch-package && composer install", "readme": "composer readme", "prepare": "husky", - "test:e2e": "npm-run-all --silent test:e2e:parallel test:e2e:serial", - "test:e2e:parallel": "./.wp-env/scripts/run-e2e.sh --grep-invert @serial", + "test:e2e": "npm-run-all --silent test:e2e:parallel test:e2e:live test:e2e:serial", + "test:e2e:parallel": "./.wp-env/scripts/run-e2e.sh --grep-invert \"@serial|@live\"", + "test:e2e:live": "./.wp-env/scripts/run-e2e.sh --grep @live", "test:e2e:serial": "./.wp-env/scripts/run-e2e.sh --grep @serial --workers=1", "test:e2e:debug": "./.wp-env/scripts/run-e2e.sh --ui", "test:unit": "wp-env run tests-cli --env-cwd=\"wp-content/plugins/$(basename \"$PWD\")\" vendor/bin/phpunit" diff --git a/tests/e2e/cloudinary-asset-edit.spec.js b/tests/e2e/cloudinary-asset-edit.spec.js index ca4eb3d94..9426a6c8d 100644 --- a/tests/e2e/cloudinary-asset-edit.spec.js +++ b/tests/e2e/cloudinary-asset-edit.spec.js @@ -8,9 +8,12 @@ const { test, expect } = require( './fixtures' ); /** * Internal dependencies */ -const { ensureCloudinaryConnected } = require( './utils/connection' ); const { expectCloudinaryUrl } = require( './utils/delivery' ); -const { wpCli } = require( './utils/wizard' ); +const { + connectLive, + deletePostsLive, + syncLive, +} = require( './utils/live-site' ); const FIXTURE_PATH = path.join( __dirname, 'fixtures', 'test-image.jpg' ); @@ -124,13 +127,13 @@ async function readDelivery( page ) { }; } -// @serial: needs real credentials in `cloudinary_connect` to sync assets, +// @live: needs real credentials in `cloudinary_connect` to sync assets, // while every analytics spec overwrites that option with fake ones. -test.describe( 'Cloudinary per-asset editing', { tag: '@serial' }, () => { +test.describe( 'Cloudinary per-asset editing', { tag: '@live' }, () => { test.describe.configure( { mode: 'serial' } ); test.beforeAll( async ( { requestUtils } ) => { - ( { cloudName } = ensureCloudinaryConnected() ); + ( { cloudName } = connectLive() ); const media = await requestUtils.rest( { method: 'POST', @@ -165,7 +168,7 @@ test.describe( 'Cloudinary per-asset editing', { tag: '@serial' }, () => { // The Edit Asset page and the front-end rewrite both need the // asset to be synced to Cloudinary. - wpCli( [ 'cloudinary', 'sync' ] ); + syncLive(); } ); test.afterAll( () => { @@ -175,15 +178,7 @@ test.describe( 'Cloudinary per-asset editing', { tag: '@serial' }, () => { const { postId, attachmentId } = created; created = null; - // WP-CLI rather than REST for the same reason as the image delivery - // spec: `force=true` on a `?rest_route=` URL is fragile. - for ( const id of [ postId, attachmentId ] ) { - try { - wpCli( [ 'post', 'delete', String( id ), '--force' ] ); - } catch ( e ) { - console.warn( `Cleanup of post ${ id } failed:`, e.message ); - } - } + deletePostsLive( [ postId, attachmentId ] ); } ); test( 'text overlay updates the delivery URL and reverts when removed', async ( { diff --git a/tests/e2e/cloudinary-gallery-delivery.spec.js b/tests/e2e/cloudinary-gallery-delivery.spec.js index a301c2be0..37d8be226 100644 --- a/tests/e2e/cloudinary-gallery-delivery.spec.js +++ b/tests/e2e/cloudinary-gallery-delivery.spec.js @@ -8,9 +8,12 @@ const { test, expect } = require( './fixtures' ); /** * Internal dependencies */ -const { ensureCloudinaryConnected } = require( './utils/connection' ); const { expectCloudinaryUrl, srcsetUrls } = require( './utils/delivery' ); -const { wpCli } = require( './utils/wizard' ); +const { + connectLive, + deletePostsLive, + withSiteLock, +} = require( './utils/live-site' ); const FIXTURE_PATH = path.join( __dirname, 'fixtures', 'test-image.jpg' ); @@ -100,13 +103,13 @@ async function collectImageUrls( images ) { ); } -// @serial: needs real credentials in `cloudinary_connect` to sync assets, +// @live: needs real credentials in `cloudinary_connect` to sync assets, // while every analytics spec overwrites that option with fake ones. -test.describe( 'Cloudinary Gallery block delivery', { tag: '@serial' }, () => { +test.describe( 'Cloudinary Gallery block delivery', { tag: '@live' }, () => { test.describe.configure( { mode: 'serial' } ); test.beforeAll( async ( { requestUtils } ) => { - ( { cloudName } = ensureCloudinaryConnected() ); + ( { cloudName } = connectLive() ); const file = fs.readFileSync( FIXTURE_PATH ); const attachmentIds = []; @@ -126,11 +129,14 @@ test.describe( 'Cloudinary Gallery block delivery', { tag: '@serial' }, () => { // Same endpoint the block editor calls when images are picked in // the media modal. It uploads each attachment to Cloudinary if it // is not synced yet and returns the `selectedImages` attribute. - const selectedImages = await requestUtils.rest( { - method: 'POST', - path: '/cloudinary/v1/image_data', - data: { images: attachmentIds.map( ( id ) => ( { id } ) ) }, - } ); + // Under the site lock so it cannot race another worker's sync. + const selectedImages = await withSiteLock( () => + requestUtils.rest( { + method: 'POST', + path: '/cloudinary/v1/image_data', + data: { images: attachmentIds.map( ( id ) => ( { id } ) ) }, + } ) + ); expect( selectedImages, @@ -165,15 +171,7 @@ test.describe( 'Cloudinary Gallery block delivery', { tag: '@serial' }, () => { const { postId, attachmentIds } = created; created = null; - // WP-CLI rather than REST for the same reason as the image delivery - // spec: `force=true` on a `?rest_route=` URL is fragile. - for ( const id of [ postId, ...attachmentIds ] ) { - try { - wpCli( [ 'post', 'delete', String( id ), '--force' ] ); - } catch ( e ) { - console.warn( `Cleanup of post ${ id } failed:`, e.message ); - } - } + deletePostsLive( [ postId, ...attachmentIds ] ); } ); test( 'renders exactly N images, all served from Cloudinary', async ( { diff --git a/tests/e2e/cloudinary-image-delivery.spec.js b/tests/e2e/cloudinary-image-delivery.spec.js index 4aa93159e..a0016eb7c 100644 --- a/tests/e2e/cloudinary-image-delivery.spec.js +++ b/tests/e2e/cloudinary-image-delivery.spec.js @@ -8,9 +8,12 @@ const { test, expect } = require( './fixtures' ); /** * Internal dependencies */ -const { ensureCloudinaryConnected } = require( './utils/connection' ); const { expectCloudinaryUrl } = require( './utils/delivery' ); -const { wpCli } = require( './utils/wizard' ); +const { + connectLive, + deletePostsLive, + syncLive, +} = require( './utils/live-site' ); const FIXTURE_PATH = path.join( __dirname, 'fixtures', 'test-image.jpg' ); @@ -23,11 +26,11 @@ let cloudName; */ let created = null; -// @serial: needs real credentials in `cloudinary_connect` for `wp cloudinary +// @live: needs real credentials in `cloudinary_connect` for `wp cloudinary // sync`, while every analytics spec overwrites that option with fake ones. -test.describe( 'Cloudinary image delivery', { tag: '@serial' }, () => { +test.describe( 'Cloudinary image delivery', { tag: '@live' }, () => { test.beforeAll( () => { - ( { cloudName } = ensureCloudinaryConnected() ); + ( { cloudName } = connectLive() ); } ); test.beforeEach( async ( { requestUtils } ) => { @@ -75,7 +78,7 @@ test.describe( 'Cloudinary image delivery', { tag: '@serial' }, () => { // first front-end render queues the sync but renders local // URLs. Driving `wp cloudinary sync` here makes the test // deterministic without relying on the cron-driven queue. - wpCli( [ 'cloudinary', 'sync' ] ); + syncLive(); } ); test.afterEach( async () => { @@ -85,21 +88,7 @@ test.describe( 'Cloudinary image delivery', { tag: '@serial' }, () => { const { postId, attachmentId } = created; created = null; - // Best-effort cleanup via WP-CLI. We do not use the REST API - // here because the test wp-env runs without pretty permalinks, - // which makes appending `force=true` to a `?rest_route=`-style - // URL fragile. WP-CLI is unambiguous and we already use it in - // other helpers (see utils/wizard.js). - try { - wpCli( [ 'post', 'delete', String( postId ), '--force' ] ); - } catch ( e ) { - console.warn( 'Post cleanup failed:', e.message ); - } - try { - wpCli( [ 'post', 'delete', String( attachmentId ), '--force' ] ); - } catch ( e ) { - console.warn( 'Media cleanup failed:', e.message ); - } + deletePostsLive( [ postId, attachmentId ] ); } ); test( 'serves featured image and inline image via Cloudinary', async ( { diff --git a/tests/e2e/cloudinary-svg-delivery.spec.js b/tests/e2e/cloudinary-svg-delivery.spec.js index 7d86990ef..96fac0831 100644 --- a/tests/e2e/cloudinary-svg-delivery.spec.js +++ b/tests/e2e/cloudinary-svg-delivery.spec.js @@ -8,9 +8,14 @@ const { test, expect } = require( './fixtures' ); /** * Internal dependencies */ -const { ensureCloudinaryConnected } = require( './utils/connection' ); const { expectCloudinaryUrl } = require( './utils/delivery' ); -const { wpCli, wpEvalFile } = require( './utils/wizard' ); +const { wpEvalFile } = require( './utils/wizard' ); +const { + connectLive, + deletePostsLive, + syncLive, + withSiteLock, +} = require( './utils/live-site' ); const VALID_FIXTURE = path.join( __dirname, 'fixtures', 'svg-valid.svg' ); const UNSAFE_FIXTURE = path.join( __dirname, 'fixtures', 'svg-unsafe.svg' ); @@ -54,8 +59,11 @@ let createdIds = []; * @return {Record} Setting slug to the value before the write. */ function saveSettings( values ) { - return JSON.parse( - wpEvalFile( ` + // Under the site lock: the settings share one option with the video + // spec's `video_player`, and each write rewrites the whole array. + return withSiteLock( () => + JSON.parse( + wpEvalFile( ` $settings = get_plugin_instance()->settings; $previous = array(); foreach ( json_decode( '${ JSON.stringify( @@ -67,6 +75,7 @@ function saveSettings( values ) { } echo wp_json_encode( $previous ); ` ) + ) ); } @@ -117,7 +126,7 @@ async function uploadSvgPost( requestUtils, fixturePath, name ) { // Same reason as the image delivery spec: sync synchronously instead // of waiting for the auto-sync queue. - wpCli( [ 'cloudinary', 'sync' ] ); + syncLive(); return { attachmentId: media.id, postLink: post.link, media }; } @@ -188,11 +197,11 @@ async function fetchAsset( request, url ) { }; } -// @serial: needs real credentials in `cloudinary_connect` for `wp cloudinary +// @live: needs real credentials in `cloudinary_connect` for `wp cloudinary // sync`, while every analytics spec overwrites that option with fake ones. -test.describe( 'Cloudinary SVG upload and delivery', { tag: '@serial' }, () => { +test.describe( 'Cloudinary SVG upload and delivery', { tag: '@live' }, () => { test.beforeAll( () => { - ( { cloudName } = ensureCloudinaryConnected() ); + ( { cloudName } = connectLive() ); // SVG support is off by default. Without it WordPress rejects the // upload and the plugin does not sync or sanitize SVGs. @@ -210,15 +219,7 @@ test.describe( 'Cloudinary SVG upload and delivery', { tag: '@serial' }, () => { const ids = createdIds; createdIds = []; - // WP-CLI rather than REST for the same reason as the image delivery - // spec: `force=true` on a `?rest_route=` URL is fragile. - for ( const id of ids ) { - try { - wpCli( [ 'post', 'delete', String( id ), '--force' ] ); - } catch ( e ) { - console.warn( `Cleanup of post ${ id } failed:`, e.message ); - } - } + deletePostsLive( ids ); } ); test( 'a valid SVG uploads, syncs and is delivered as an SVG', async ( { diff --git a/tests/e2e/cloudinary-video-delivery.spec.js b/tests/e2e/cloudinary-video-delivery.spec.js index 45efe4504..ff2d05b4f 100644 --- a/tests/e2e/cloudinary-video-delivery.spec.js +++ b/tests/e2e/cloudinary-video-delivery.spec.js @@ -9,8 +9,13 @@ const { test, expect } = require( './fixtures' ); /** * Internal dependencies */ -const { ensureCloudinaryConnected } = require( './utils/connection' ); -const { wpCli, getCliContainer } = require( './utils/wizard' ); +const { getCliContainer } = require( './utils/wizard' ); +const { + connectLive, + deletePostsLive, + syncLive, + withSiteLock, +} = require( './utils/live-site' ); const FIXTURE_PATH = path.join( __dirname, 'fixtures', 'test-video.mp4' ); @@ -78,18 +83,20 @@ function setVideoPlayer( value ) { // `timeout` matters: execSync blocks the worker's event loop, so // Playwright's own test timeout cannot interrupt a hung docker/wp // call. Without it a wedged container hangs the whole suite. - execSync( cmd, { - encoding: 'utf8', - stdio: [ 'ignore', 'pipe', 'pipe' ], - timeout: 30_000, - } ); + withSiteLock( () => + execSync( cmd, { + encoding: 'utf8', + stdio: [ 'ignore', 'pipe', 'pipe' ], + timeout: 30_000, + } ) + ); } -// @serial: needs real credentials in `cloudinary_connect` for `wp cloudinary +// @live: needs real credentials in `cloudinary_connect` for `wp cloudinary // sync`, while every analytics spec overwrites that option with fake ones. -test.describe( 'Cloudinary video delivery', { tag: '@serial' }, () => { +test.describe( 'Cloudinary video delivery', { tag: '@live' }, () => { test.beforeAll( () => { - ( { cloudName } = ensureCloudinaryConnected() ); + ( { cloudName } = connectLive() ); } ); test.beforeEach( async ( { requestUtils } ) => { @@ -134,7 +141,7 @@ test.describe( 'Cloudinary video delivery', { tag: '@serial' }, () => { // The plugin's URL rewriting depends on the asset being synced // to Cloudinary. Drive the sync synchronously here so the // front-end visit sees rewritten URLs. - wpCli( [ 'cloudinary', 'sync' ] ); + syncLive(); } ); test.afterEach( async () => { @@ -144,19 +151,7 @@ test.describe( 'Cloudinary video delivery', { tag: '@serial' }, () => { const { postId, attachmentId } = created; created = null; - // Best-effort cleanup via WP-CLI. Matches the image spec; we - // do not use the REST API because this wp-env runs without - // pretty permalinks, which makes `?force=true` fragile. - try { - wpCli( [ 'post', 'delete', String( postId ), '--force' ] ); - } catch ( e ) { - console.warn( 'Post cleanup failed:', e.message ); - } - try { - wpCli( [ 'post', 'delete', String( attachmentId ), '--force' ] ); - } catch ( e ) { - console.warn( 'Media cleanup failed:', e.message ); - } + deletePostsLive( [ postId, attachmentId ] ); } ); test.describe( 'with default WP player', () => { diff --git a/tests/e2e/playwright.config.js b/tests/e2e/playwright.config.js index 8fd788edf..a87b29dc6 100644 --- a/tests/e2e/playwright.config.js +++ b/tests/e2e/playwright.config.js @@ -60,9 +60,13 @@ module.exports = defineConfig( { retries: process.env.CI ? 2 : 0, // Spec files are spread across workers; tests within one file still run // in order (fullyParallel is off), which the delivery specs' shared - // beforeAll/afterAll state relies on. Specs tagged @serial mutate - // site-wide state (connection, plugin activation) and are run in a - // second, single-worker pass by `npm run test:e2e`; see package.json. + // beforeAll/afterAll state relies on. `npm run test:e2e` runs three + // passes; see package.json. Specs tagged @live need the real Cloudinary + // connection, which the analytics specs overwrite with fake credentials, + // so they get a pass of their own; they take turns on site-wide steps + // through tests/e2e/utils/live-site.js. Specs tagged @serial mutate + // site-wide state (connection, plugin activation) and run last, in a + // single-worker pass. // Analytics specs are safe to run concurrently because tests/e2e/fixtures.js // gives each worker its own analytics capture log. workers: 3, diff --git a/tests/e2e/utils/live-site.js b/tests/e2e/utils/live-site.js new file mode 100644 index 000000000..2276416ed --- /dev/null +++ b/tests/e2e/utils/live-site.js @@ -0,0 +1,183 @@ +/** + * Helpers for the specs tagged @live, which need a real Cloudinary + * connection and run in parallel workers against one WordPress site. + * + * The steps that act on site-wide state have to take turns, so they run + * under a cross-worker lock: + * + * - `wp cloudinary sync` is not scoped to one attachment. Its analyze step + * clears and re-adds the `_cld_unsynced` flag on every attachment, so a + * second sync that queries in that window finds nothing to sync. + * - The gallery spec syncs through `/cloudinary/v1/image_data`, which would + * otherwise race a concurrent CLI sync for the same attachments. + * - Deleting an attachment while another worker's sync iterates over it. + * - Settings writes, which read and rewrite a whole option array. + * + * All Playwright workers run on the same host, so an atomic `mkdir` in the + * project's artifacts directory is enough. The lock records the owner PID so + * a worker that dies while holding it (for example on a test timeout) does + * not block the rest of the run. + */ + +const fs = require( 'fs' ); +const path = require( 'path' ); + +const { ensureCloudinaryConnected } = require( './connection' ); +const { wpCli } = require( './wizard' ); + +const LOCK_DIR = path.join( process.cwd(), 'artifacts', '.site-lock' ); +const OWNER_FILE = path.join( LOCK_DIR, 'owner' ); +const WAIT_TIMEOUT_MS = 120_000; +const POLL_MS = 100; + +const sleeper = new Int32Array( new SharedArrayBuffer( 4 ) ); + +/** + * Block the current worker for a short time. + * + * The callers already block the event loop on `execSync`, so a synchronous + * wait keeps the lock usable from both sync and async code. + * + * @param {number} ms Milliseconds. + */ +function sleep( ms ) { + Atomics.wait( sleeper, 0, 0, ms ); +} + +/** + * Whether the process that holds the lock is gone. + * + * @return {boolean} True if the lock can be taken over. + */ +function isStale() { + let pid; + try { + pid = Number( fs.readFileSync( OWNER_FILE, 'utf8' ) ); + } catch ( e ) { + // The owner file is written right after mkdir; give the owner a + // moment instead of treating the gap as a dead process. + return false; + } + if ( ! pid ) { + return false; + } + try { + process.kill( pid, 0 ); + return false; + } catch ( e ) { + return 'ESRCH' === e.code; + } +} + +/** + * Take the lock, waiting for another worker to release it if needed. + * + * @throws If the lock is not free within WAIT_TIMEOUT_MS. + */ +function acquire() { + fs.mkdirSync( path.dirname( LOCK_DIR ), { recursive: true } ); + const deadline = Date.now() + WAIT_TIMEOUT_MS; + + for (;;) { + try { + fs.mkdirSync( LOCK_DIR ); + fs.writeFileSync( OWNER_FILE, String( process.pid ) ); + return; + } catch ( e ) { + if ( 'EEXIST' !== e.code ) { + throw e; + } + } + + if ( isStale() ) { + release(); + continue; + } + if ( Date.now() > deadline ) { + throw new Error( + `Timed out after ${ WAIT_TIMEOUT_MS }ms waiting for ${ LOCK_DIR }.` + ); + } + sleep( POLL_MS ); + } +} + +/** + * Release the lock. + */ +function release() { + fs.rmSync( LOCK_DIR, { recursive: true, force: true } ); +} + +/** + * Run a callback while holding the lock. + * + * Accepts sync and async callbacks; for an async callback the lock is held + * until its promise settles. + * + * @template T + * @param {() => T} fn Callback. + * @return {T} The callback's return value. + */ +function withSiteLock( fn ) { + acquire(); + let result; + try { + result = fn(); + } catch ( e ) { + release(); + throw e; + } + if ( result && 'function' === typeof result.then ) { + return result.finally( release ); + } + release(); + return result; +} + +/** + * Connect the plugin with the real credentials, under the lock. + * + * Every @live spec writes the same URL, so after the first write the + * plugin's `verify_connection()` sees no change and skips the API call. + * + * @return {{ cloudName: string }} The cloud name from CLOUDINARY_E2E_URL. + */ +function connectLive() { + return withSiteLock( ensureCloudinaryConnected ); +} + +/** + * Run `wp cloudinary sync` under the lock. + */ +function syncLive() { + withSiteLock( () => wpCli( [ 'cloudinary', 'sync' ] ) ); +} + +/** + * Force-delete posts and attachments under the lock. Failures are logged, + * not thrown, because cleanup is best effort. + * + * WP-CLI rather than REST: this wp-env runs without pretty permalinks, + * which makes `force=true` on a `?rest_route=` URL fragile. + * + * @param {number[]} ids Post or attachment IDs. + */ +function deletePostsLive( ids ) { + withSiteLock( () => { + for ( const id of ids ) { + try { + wpCli( [ 'post', 'delete', String( id ), '--force' ] ); + } catch ( e ) { + console.warn( `Cleanup of post ${ id } failed:`, e.message ); + } + } + } ); +} + +module.exports = { + withSiteLock, + connectLive, + syncLive, + deletePostsLive, +};