chore: unpin playwright (#462)

* chore: unpin playwright and refactor tests to work with its changed behavior

* fix: change test for server restart to avoid color codes interfering

* chore: move to macos-latest again for tests

* fix: use vite connected message as condition for page ready instead of networkidle

Author: dominikg

.github/workflows/ci.yml

@@ -74,7 +74,7 @@ jobs:
       fail-fast: false
       matrix:
         node: [16]
-        os: [ubuntu-latest, macos-11, windows-latest]
+        os: [ubuntu-latest, macos-latest, windows-latest]
         include:
           - node: 14
             os: ubuntu-latest

package.json

@@ -40,7 +40,7 @@
     "lint-staged": "^13.0.3",
     "node-fetch": "^3.2.10",
     "npm-run-all": "^4.1.5",
-    "playwright-core": "1.25.2",
+    "playwright-core": "^1.27.1",
     "prettier": "^2.7.1",
     "prettier-plugin-svelte": "^2.7.1",
     "rimraf": "^3.0.2",

packages/e2e-tests/hmr/__tests__/hmr.spec.ts

@@ -1,6 +1,5 @@
 import {
 	isBuild,
-	isWin,
 	getEl,
 	getText,
 	editFileAndWaitForHmrComplete,
@@ -12,9 +11,8 @@ import {
 	addFile,
 	removeFile,
 	editViteConfig,
-	page,
 	browserLogs,
-	viteTestUrl
+	waitForServerRestartAndReloadPage
 } from '~utils';
 
 test('should render App', async () => {
@@ -167,9 +165,7 @@ if (!isBuild) {
 
 		test('should work with emitCss: false in svelte config', async () => {
 			addFile('svelte.config.cjs', `module.exports={vitePlugin:{emitCss:false}}`);
-			await sleep(isWin ? 1000 : 500); // adding config restarts server, give it some time
-			await page.goto(viteTestUrl, { waitUntil: 'networkidle' });
-			await sleep(50);
+			await waitForServerRestartAndReloadPage();
 			expect(await getColor(`#hmr-test-1 .label`)).toBe('red');
 			removeFile('svelte.config.cjs');
 		});
@@ -190,9 +186,7 @@ if (!isBuild) {
 				`module.exports = {
 			  preprocess:[{markup:${injectPreprocessor.toString()}}]};`
 			);
-			await sleep(isWin ? 1000 : 500); // adding config restarts server, give it some time
-			await page.goto(viteTestUrl, { waitUntil: 'networkidle' });
-			await sleep(50);
+			await waitForServerRestartAndReloadPage();
 			expect(await getText('#preprocess-inject')).toBe('Injected');
 			expect(await getText(`#hmr-test-1 .counter`)).toBe('0');
 			expect(await getColor(`#hmr-test-1 .label`)).toBe('red');
@@ -207,9 +201,7 @@ if (!isBuild) {
 					.replace('preprocess-inject', 'preprocess-inject-2')
 					.replace('Injected', 'Injected 2')
 			);
-			await sleep(isWin ? 1000 : 500); // editing config restarts server, give it some time
-			await page.goto(viteTestUrl, { waitUntil: 'networkidle' });
-			await sleep(50);
+			await waitForServerRestartAndReloadPage();
 			expect(await getText('#preprocess-inject-2')).toBe('Injected 2');
 			expect(await getEl('#preprocess-inject')).toBe(null);
 			expect(await getColor(`#hmr-test-1 .label`)).toBe('green');
@@ -221,9 +213,7 @@ if (!isBuild) {
 			expect(await getColor(`#hmr-test-1 .label`)).toBe('red');
 			expect(await getText(`#hmr-test-1 .counter`)).toBe('1');
 			await removeFile('svelte.config.cjs');
-			await sleep(isWin ? 1000 : 500); // editing config restarts server, give it some time
-			await page.goto(viteTestUrl, { waitUntil: 'networkidle' });
-			await sleep(50);
+			await waitForServerRestartAndReloadPage();
 			expect(await getEl('#preprocess-inject-2')).toBe(null);
 			expect(await getEl('#preprocess-inject')).toBe(null);
 			expect(await getColor(`#hmr-test-1 .label`)).toBe('red');

packages/e2e-tests/kit-node/__tests__/kit.spec.ts

@@ -11,10 +11,10 @@ import {
 	waitForNavigation,
 	page,
 	browserLogs,
-	fetchPageText
+	fetchPageText,
+	reloadPage
 } from '~utils';
 
-import path from 'path';
 import glob from 'tiny-glob';
 
 describe('kit-node', () => {
@@ -172,7 +172,7 @@ describe('kit-node', () => {
 				it('should serve changes even after page reload', async () => {
 					expect(await getColor(`h1`)).toBe('green');
 					expect(await getText(`#hmr-test2`)).toBe('bar');
-					await page.reload({ waitUntil: 'networkidle' });
+					await reloadPage();
 					expect(await getColor(`h1`)).toBe('green');
 					expect(await getText(`#hmr-test2`)).toBe('bar');
 				});
@@ -189,6 +189,9 @@ describe('kit-node', () => {
 						await updateChild((content) =>
 							content.replace('<!-- HMR-TEMPLATE-INJECT -->', '-foo<!-- HMR-TEMPLATE-INJECT -->')
 						);
+						// for some reason the update takes longer to materialize, so wait for it to avoid subsequent errors
+						await page.getByText('test-child-foo').waitFor({ state: 'attached' });
+
 						expect(await getText('#before-child')).toBe('before-child');
 						expect(await getText('#test-child')).toBe('test-child-foo');
 						expect(await getText('#after-child')).toBe('after-child');

packages/e2e-tests/testUtils.ts

@@ -7,7 +7,16 @@ import colors from 'css-color-names';
 import { ElementHandle } from 'playwright-core';
 import fetch from 'node-fetch';
 
-import { isBuild, isWin, isCI, page, testDir, viteTestUrl, browserLogs } from './vitestSetup';
+import {
+	isBuild,
+	isWin,
+	isCI,
+	page,
+	testDir,
+	browserLogs,
+	e2eServer,
+	waitForViteConnect
+} from './vitestSetup';
 
 export * from './vitestSetup';
 
@@ -42,20 +51,23 @@ const timeout = (n: number) => new Promise((r) => setTimeout(r, n));
 
 async function toEl(el: string | ElementHandle): Promise<ElementHandle> {
 	if (typeof el === 'string') {
-		return await page.$(el);
+		return await page.$(el, { strict: true });
 	}
 	return el;
 }
 
 export async function getColor(el: string | ElementHandle) {
 	el = await toEl(el);
+	if (el == null) {
+		return null;
+	}
 	const rgb = await el.evaluate((el) => getComputedStyle(el as Element).color);
 	return hexToNameMap[rgbToHex(rgb)] || rgb;
 }
 
 export async function getBg(el: string | ElementHandle) {
 	el = await toEl(el);
-	return el.evaluate((el) => getComputedStyle(el as Element).backgroundImage);
+	return el == null ? null : el.evaluate((el) => getComputedStyle(el as Element).backgroundImage);
 }
 
 export function readFileContent(filename: string) {
@@ -124,7 +136,7 @@ export async function getEl(selector: string) {
 
 export async function getText(el: string | ElementHandle) {
 	el = await toEl(el);
-	return el ? await el.evaluate((el) => el.textContent) : null;
+	return el ? el.textContent() : null;
 }
 
 export async function hmrUpdateComplete(file, timeout) {
@@ -204,9 +216,33 @@ export async function saveScreenshot(name: string) {
 
 export async function editViteConfig(replacer: (str: string) => string) {
 	editFile('vite.config.js', replacer);
-	await sleep(isWin ? 1000 : 500); // editing vite config restarts server, give it some time
-	await page.goto(viteTestUrl, { waitUntil: 'networkidle' });
-	await sleep(50);
+	if (!isBuild) {
+		await waitForServerRestartAndReloadPage();
+	}
+}
+
+export async function waitForServerRestartAndReloadPage(timeout = 10000) {
+	const logs = e2eServer.logs.server.out;
+	const startIdx = logs.length;
+	let timeleft = timeout;
+	const pollInterval = 50;
+	let restarted = false;
+	while (timeleft > 0) {
+		await sleep(pollInterval);
+		if (logs.some((text, i) => i > startIdx && text.endsWith('server restarted.'))) {
+			restarted = true;
+			break;
+		}
+		timeleft -= pollInterval;
+	}
+	if (!restarted) {
+		throw new Error(`server did not restart after ${timeout}ms`);
+	}
+	await reloadPage();
+}
+
+export async function reloadPage() {
+	await Promise.all([page.reload(), waitForViteConnect(page)]);
 }
 
 export async function waitForNavigation(opts: Parameters<typeof page.waitForNavigation>[0]) {

packages/e2e-tests/vitestSetup.ts

@@ -212,14 +212,20 @@ beforeAll(
 );
 
 async function goToUrlAndWaitForViteWSConnect(page: Page, url: string) {
+	return Promise.all([page.goto(url), waitForViteConnect(page, 15000)]);
+}
+
+export async function waitForViteConnect(page: Page, timeoutMS = 5000) {
+	if (isBuild) {
+		return Promise.resolve(); // no vite websocket on build
+	}
 	let timerId;
 	let pageConsoleListener;
-	const timeoutMS = 15000;
 	const timeoutPromise = new Promise(
 		// eslint-disable-next-line no-unused-vars
 		(_, reject) =>
 			(timerId = setTimeout(() => {
-				reject(`page under test not ready after ${timeoutMS}ms. url: ${url}`);
+				reject(`vite client not connected after ${timeoutMS}ms. url: ${page.url()}`);
 			}, timeoutMS))
 	);
 	const connectedPromise = new Promise<void>((resolve) => {
@@ -232,9 +238,8 @@ async function goToUrlAndWaitForViteWSConnect(page: Page, url: string) {
 		page.on('console', pageConsoleListener);
 	});
 
-	const connectedOrTimeout = Promise.race([connectedPromise, timeoutPromise]).finally(() => {
+	return Promise.race([connectedPromise, timeoutPromise]).finally(() => {
 		page.off('console', pageConsoleListener);
 		clearTimeout(timerId);
 	});
-	return page.goto(url).then(() => connectedOrTimeout);
 }