fix(e2e): make the runtime-error tests able to fail, drop a save race

.github/workflows/tests.yml

@@ -165,9 +165,9 @@ jobs:
       - name: Setup Node.js and install dependencies
         uses: ./.github/setup-node
 
-      - name: Npm build
-        run: npm run build
-
+      # No `npm run build` here. `lint:js` is `biome check`, and the shared
+      # Biome config excludes `**/build`; `lint:css` globs the `assets` sources
+      # directly. Neither reads a single file the build produces.
       - name: Running the lint
         run: npm run lint
 

package.json

@@ -45,7 +45,7 @@
 		"@babel/preset-env": "^7.23.9",
 		"@babel/register": "^7.23.7",
 		"@biomejs/biome": "2.5.7",
-		"@nk-crew/plugin-toolkit": "^0.5.1",
+		"@nk-crew/plugin-toolkit": "^0.6.0",
 		"@playwright/test": "^1.45.3",
 		"@wordpress/e2e-test-utils-playwright": "^1.4.0",
 		"@wordpress/env": "^10.4.0",

tests/e2e/specs/gist-block.spec.js

@@ -73,7 +73,22 @@ async function mockGistResponses(target, stylesheetUrl) {
 	return stats;
 }
 
-async function publishAndGetFrontendPage(page, editor) {
+/**
+ * Publishes the post and opens its frontend.
+ *
+ * `onPageReady` runs against the new page *before* it navigates. That ordering
+ * is the whole point: Playwright does not buffer `console` or `pageerror` for
+ * listeners attached later, and `goto` resolves on `load`, which already waited
+ * for the async plugin scripts to execute. A listener attached after this
+ * function returned could never see an error thrown during script evaluation --
+ * which is exactly the error these specs exist to catch.
+ *
+ * @param {Object}   page        Playwright page holding the editor.
+ * @param {Object}   editor      Block editor utils.
+ * @param {Function} onPageReady Called with the frontend page before it loads.
+ * @return {Object} The frontend page.
+ */
+async function publishAndGetFrontendPage(page, editor, onPageReady) {
 	await editor.publishPost();
 
 	const viewPageButton = page
@@ -86,14 +101,17 @@ async function publishAndGetFrontendPage(page, editor) {
 
 	if (href) {
 		const frontendPage = await page.context().newPage();
+		onPageReady?.(frontendPage);
+		// No `waitForLoadState('domcontentloaded')` after this: `goto` resolves
+		// on `load`, which is strictly later, so the wait was a no-op.
 		await frontendPage.goto(href);
-		await frontendPage.waitForLoadState('domcontentloaded');
 		return frontendPage;
 	}
 
 	const popupPromise = page.waitForEvent('popup', { timeout: 3000 });
 	await viewPageButton.click();
 	const popupPage = await popupPromise;
+	onPageReady?.(popupPage);
 	await popupPage.waitForLoadState('domcontentloaded');
 
 	return popupPage;
@@ -143,13 +161,15 @@ test.describe('gist block', () => {
 			},
 		});
 
-		await page.waitForTimeout(1000);
-
 		await expect(
 			editor.canvas.locator('.ghostkit-gist').first()
 		).toBeVisible();
-		await expect(mockStats.jsonRequests).toBeGreaterThan(0);
-		await expect(mockStats.cssRequests).toBeGreaterThan(0);
+		// `expect.poll`, not `expect`: `mockStats` is a plain object the route
+		// handlers mutate, so a bare `expect` reads it once with no retry. That
+		// is why a flat 1s sleep used to sit above this -- polling ends as soon
+		// as the mock has fired instead.
+		await expect.poll(() => mockStats.jsonRequests).toBeGreaterThan(0);
+		await expect.poll(() => mockStats.cssRequests).toBeGreaterThan(0);
 
 		const relevantErrors = runtimeErrors.filter(
 			(message) =>
@@ -191,16 +211,28 @@ test.describe('gist block', () => {
 			},
 		});
 
-		const frontendPage = await publishAndGetFrontendPage(page, editor);
-		const runtimeErrors = trackRuntimeErrors(frontendPage);
-
-		await frontendPage.waitForTimeout(1000);
+		// Attached through the callback so the listeners exist before the page
+		// navigates. Attaching them to the returned page, as this did, meant the
+		// script had already run and any error was long gone -- `runtimeErrors`
+		// was always empty and the assertion below could not fail.
+		let runtimeErrors = [];
+		const frontendPage = await publishAndGetFrontendPage(
+			page,
+			editor,
+			(frontend) => {
+				runtimeErrors = trackRuntimeErrors(frontend);
+			}
+		);
 
 		await expect(
 			frontendPage.locator('.ghostkit-gist').first()
 		).toBeVisible();
-		await expect(mockStats.jsonRequests).toBeGreaterThan(0);
-		await expect(mockStats.cssRequests).toBeGreaterThan(0);
+		// `expect.poll`, not `expect`: `mockStats` is a plain object the route
+		// handlers mutate, so a bare `expect` reads it once with no retry. That
+		// is why a flat 1s sleep used to sit above this -- polling ends as soon
+		// as the mock has fired instead.
+		await expect.poll(() => mockStats.jsonRequests).toBeGreaterThan(0);
+		await expect.poll(() => mockStats.cssRequests).toBeGreaterThan(0);
 
 		const relevantErrors = runtimeErrors.filter(
 			(message) =>

tests/e2e/specs/lottie-block.spec.js

@@ -61,7 +61,22 @@ async function mockLottieResponse(target) {
 	});
 }
 
-async function publishAndGetFrontendPage(page, editor) {
+/**
+ * Publishes the post and opens its frontend.
+ *
+ * `onPageReady` runs against the new page *before* it navigates. That ordering
+ * is the whole point: Playwright does not buffer `console` or `pageerror` for
+ * listeners attached later, and `goto` resolves on `load`, which already waited
+ * for the async plugin scripts to execute. A listener attached after this
+ * function returned could never see an error thrown during script evaluation --
+ * which is exactly the error these specs exist to catch.
+ *
+ * @param {Object}   page        Playwright page holding the editor.
+ * @param {Object}   editor      Block editor utils.
+ * @param {Function} onPageReady Called with the frontend page before it loads.
+ * @return {Object} The frontend page.
+ */
+async function publishAndGetFrontendPage(page, editor, onPageReady) {
 	await editor.publishPost();
 
 	const viewPageButton = page
@@ -74,14 +89,17 @@ async function publishAndGetFrontendPage(page, editor) {
 
 	if (href) {
 		const frontendPage = await page.context().newPage();
+		onPageReady?.(frontendPage);
+		// No `waitForLoadState('domcontentloaded')` after this: `goto` resolves
+		// on `load`, which is strictly later, so the wait was a no-op.
 		await frontendPage.goto(href);
-		await frontendPage.waitForLoadState('domcontentloaded');
 		return frontendPage;
 	}
 
 	const popupPromise = page.waitForEvent('popup', { timeout: 3000 });
 	await viewPageButton.click();
 	const popupPage = await popupPromise;
+	onPageReady?.(popupPage);
 	await popupPage.waitForLoadState('domcontentloaded');
 
 	return popupPage;
@@ -164,8 +182,18 @@ test.describe('lottie block', () => {
 			},
 		});
 
-		const frontendPage = await publishAndGetFrontendPage(page, editor);
-		const runtimeErrors = trackRuntimeErrors(frontendPage);
+		// Attached through the callback so the listeners exist before the page
+		// navigates. Attaching them to the returned page, as this did, meant the
+		// script had already run and any error was long gone -- `runtimeErrors`
+		// was always empty and the assertion below could not fail.
+		let runtimeErrors = [];
+		const frontendPage = await publishAndGetFrontendPage(
+			page,
+			editor,
+			(frontend) => {
+				runtimeErrors = trackRuntimeErrors(frontend);
+			}
+		);
 
 		await expect(
 			frontendPage.locator('lottie-player').first()

tests/e2e/specs/typography.spec.js

@@ -56,10 +56,24 @@ test.describe('typography', () => {
 
 		await fontInput.fill(font);
 
+		// Wait for the save, not for a guess at how long it takes. The settings
+		// page debounces by exactly 1000ms and then fires the POST without
+		// awaiting it, so `waitForTimeout(1000)` expired as the request was
+		// being issued -- and the `page` fixture closes the page at teardown,
+		// discarding anything still in flight. When it lost that race the
+		// symptom appeared in a later test, as a font mismatch.
+		//
+		// Registered before the keypress: the response is a second away, but the
+		// listener still has to exist before the thing that triggers it.
+		const saved = page.waitForResponse(
+			(response) =>
+				response.url().includes('update_custom_typography') &&
+				response.request().method() === 'POST'
+		);
+
 		await fontInput.press('Enter');
 
-		// Just wait 1 second for the selected font to be saved in the database after selection.
-		await page.waitForTimeout(1000);
+		await saved;
 
 		return wrapper;
 	}