test: improve ci e2e setup to include multiple browsers, optimize trace capturing

.changeset/breezy-hairs-greet.md

@@ -0,0 +1,5 @@
+---
+"@sveltejs/kit": patch
+---
+
+reset selection in setTimeout after navigating, to ensure correct behaviour in Firefox

.github/workflows/ci.yml

@@ -10,9 +10,17 @@ env:
   TURBO_TOKEN: ${{ secrets.TURBO_TOKEN }}
   TURBO_TEAM: svelte
 
+# cancel in-progress runs on new commits to same PR (gitub.event.number)
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.number || github.sha }}
+  cancel-in-progress: true
+
 jobs:
   Lint:
     runs-on: ubuntu-latest
+    env:
+      # not needed for linting
+      PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: "1"
     steps:
       - uses: actions/checkout@v3
       - uses: pnpm/action-setup@v2.2.2
@@ -30,12 +38,23 @@ jobs:
       fail-fast: false
       matrix:
         node-version: [16]
-        os: [ubuntu-latest, macOS-latest, windows-2019]
+        os: [ubuntu-latest , windows-2019]
+        e2e-browser: ['chromium']
         include:
+          - node-version: 16
+            os: ubuntu-latest
+            e2e-browser: 'firefox'
+          - node-version: 16
+            os: macOS-latest
+            e2e-browser: 'safari'
           - node-version: 18
             os: ubuntu-latest
+            e2e-browser: 'chromium'
     env:
       TURBO_CACHE_KEY: ${{ matrix.os }}-${{ matrix.node-version }}
+      # Install playwright's binray under node_modules so it will be cached together
+      PLAYWRIGHT_BROWSERS_PATH: "0"
+      KIT_E2E_BROWSER: ${{matrix.e2e-browser}}
     steps:
       - run: git config --global core.autocrlf false
       - uses: actions/checkout@v3
@@ -45,15 +64,16 @@ jobs:
           node-version: ${{ matrix.node-version }}
           cache: pnpm
       - run: pnpm install --frozen-lockfile
+      - run: pnpm playwright install
       - run: pnpm test
-      - name: archive test results
+      - name: Archive test results
         if: failure()
         shell: bash
-        run: find packages -type d -name test-results -not -empty | tar -czf test-results.tar.gz --exclude='*-retry*' --files-from=-
-      - name: Upload failed tests screenshots
+        run: find packages -type d -name test-results -not -empty | tar -czf test-results.tar.gz --files-from=-
+      - name: Upload test results
         if: failure()
         uses: actions/upload-artifact@v3
         with:
           retention-days: 3
-          name: test-failure-${{ github.run_id }}-${{ matrix.os }}-${{ matrix.node-version }}
+          name: test-failure-${{ github.run_id }}-${{ matrix.os }}-${{ matrix.node-version }}-${{ matrix.e2e-browser }}
           path: test-results.tar.gz

package.json

@@ -35,6 +35,7 @@
 		"eslint-plugin-import": "^2.26.0",
 		"eslint-plugin-node": "^11.1.0",
 		"eslint-plugin-svelte3": "^4.0.0",
+		"playwright": "^1.22.2",
 		"prettier": "^2.6.2",
 		"rollup": "^2.75.3",
 		"svelte": "^3.48.0",

packages/adapter-static/test/utils.js

@@ -50,11 +50,7 @@ export function run(app, callback) {
 			context.server = await create_server(context.port, handler);
 
 			context.base = `http://localhost:${context.port}`;
-			context.browser = await chromium.launch({
-				// use stable chrome from host OS instead of downloading one
-				// see https://playwright.dev/docs/browsers#google-chrome--microsoft-edge
-				channel: 'chrome'
-			});
+			context.browser = await chromium.launch();
 			context.page = await context.browser.newPage();
 		} catch (e) {
 			// TODO remove unnecessary try-catch https://github.com/lukeed/uvu/pull/61

packages/kit/src/runtime/client/client.js

@@ -304,10 +304,13 @@ export function create_client({ target, session, base, trailing_slash }) {
 				const root = document.body;
 				const tabindex = root.getAttribute('tabindex');
 
-				getSelection()?.removeAllRanges();
 				root.tabIndex = -1;
 				root.focus({ preventScroll: true });
 
+				setTimeout(() => {
+					getSelection()?.removeAllRanges();
+				});
+
 				// restore `tabindex` as to prevent `root` from stealing input from elements
 				if (tabindex !== null) {
 					root.setAttribute('tabindex', tabindex);

packages/kit/test/apps/basics/test/test.js

@@ -74,7 +74,7 @@ test.describe('a11y', () => {
 					selection.addRange(range);
 					return selection.rangeCount;
 				}
-				return 0;
+				return -1;
 			})
 		).toBe(1);
 
@@ -85,7 +85,7 @@ test.describe('a11y', () => {
 				if (selection) {
 					return selection.rangeCount;
 				}
-				return 1;
+				return -1;
 			})
 		).toBe(0);
 	});
@@ -139,7 +139,7 @@ test.describe('beforeNavigate', () => {
 
 	test('prevents unload', async ({ page }) => {
 		await page.goto('/before-navigate/prevent-navigation');
-
+		await page.click('h1'); // The browsers block attempts to prevent navigation on a frame that's never had a user gesture.
 		const type = new Promise((fulfil) => {
 			page.on('dialog', async (dialog) => {
 				fulfil(dialog.type());
@@ -205,17 +205,12 @@ test.describe('Scrolling', () => {
 		expect(await page.evaluate(() => scrollY === 0)).toBeTruthy();
 	});
 
-	test('scroll is restored after hitting the back button', async ({
-		back,
-		baseURL,
-		clicknav,
-		page
-	}) => {
+	test('scroll is restored after hitting the back button', async ({ baseURL, clicknav, page }) => {
 		await page.goto('/anchor');
 		await page.click('#scroll-anchor');
 		const originalScrollY = /** @type {number} */ (await page.evaluate(() => scrollY));
 		await clicknav('#routing-page');
-		await back();
+		await page.goBack();
 		expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2');
 		expect(await page.evaluate(() => scrollY)).toEqual(originalScrollY);
 
@@ -226,13 +221,15 @@ test.describe('Scrolling', () => {
 
 	test('scroll is restored after hitting the back button for an in-app cross-document navigation', async ({
 		page,
-		clicknav,
-		back
+		clicknav
 	}) => {
 		await page.goto('/scroll/cross-document/a');
-		await page.locator('[href="/scroll/cross-document/b"]').scrollIntoViewIfNeeded();
 
-		const y1 = await page.evaluate(() => scrollY);
+		const rect = await page.locator('[href="/scroll/cross-document/b"]').boundingBox();
+		const height = await page.evaluate(() => innerHeight);
+
+		const target_scroll_y = rect.y + rect.height - height;
+		await page.evaluate((y) => scrollTo(0, y), target_scroll_y);
 
 		await page.click('[href="/scroll/cross-document/b"]');
 		expect(await page.textContent('h1')).toBe('b');
@@ -241,16 +238,16 @@ test.describe('Scrolling', () => {
 		await clicknav('[href="/scroll/cross-document/c"]');
 		expect(await page.textContent('h1')).toBe('c');
 
-		await back(); // client-side back
+		await page.goBack(); // client-side back
 		await page.goBack(); // native back
 		expect(await page.textContent('h1')).toBe('a');
 		await page.waitForSelector('body.started');
 
 		await page.waitForTimeout(250); // needed for the test to fail reliably without the fix
 
-		const y2 = await page.evaluate(() => scrollY);
+		const scroll_y = await page.evaluate(() => scrollY);
 
-		expect(Math.abs(y2 - y1)).toBeLessThan(10); // we need a few pixels wiggle room, because browsers
+		expect(Math.abs(scroll_y - target_scroll_y)).toBeLessThan(50); // we need a few pixels wiggle room, because browsers
 	});
 
 	test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({
@@ -302,7 +299,7 @@ test.describe('Scrolling', () => {
 		await expect(page.locator('input')).toBeFocused();
 	});
 
-	test('scroll positions are recovered on reloading the page', async ({ page, back, app }) => {
+	test('scroll positions are recovered on reloading the page', async ({ page, app }) => {
 		await page.goto('/anchor');
 		await page.evaluate(() => window.scrollTo(0, 1000));
 		await app.goto('/anchor/anchor');
@@ -311,7 +308,7 @@ test.describe('Scrolling', () => {
 		await page.reload();
 		expect(await page.evaluate(() => window.scrollY)).toBe(1000);
 
-		await back();
+		await page.goBack();
 		expect(await page.evaluate(() => window.scrollY)).toBe(1000);
 	});
 
@@ -1910,7 +1907,7 @@ test.describe('searchParams', () => {
 });
 
 test.describe('Redirects', () => {
-	test('redirect', async ({ baseURL, page, clicknav, back }) => {
+	test('redirect', async ({ baseURL, page, clicknav }) => {
 		await page.goto('/redirect');
 
 		await clicknav('[href="/redirect/a"]');
@@ -1919,7 +1916,7 @@ test.describe('Redirects', () => {
 		expect(await page.textContent('h1')).toBe('c');
 		expect(page.url()).toBe(`${baseURL}/redirect/c`);
 
-		await back();
+		await page.goBack();
 		expect(page.url()).toBe(`${baseURL}/redirect`);
 	});
 
@@ -1937,10 +1934,10 @@ test.describe('Redirects', () => {
 			);
 		} else {
 			// there's not a lot we can do to handle server-side redirect loops
-			if (browserName === 'webkit') {
-				expect(page.url()).toBe(`${baseURL}/redirect`);
-			} else {
+			if (browserName === 'chromium') {
 				expect(page.url()).toBe('chrome-error://chromewebdata/');
+			} else {
+				expect(page.url()).toBe(`${baseURL}/redirect`);
 			}
 		}
 	});
@@ -2260,35 +2257,37 @@ test.describe('Routing', () => {
 		expect(await page.textContent('h1')).toBe('y/1');
 	});
 
-	test('back button returns to initial route', async ({ page, clicknav, back }) => {
+	test('back button returns to initial route', async ({ page, clicknav }) => {
 		await page.goto('/routing');
 		await clicknav('[href="/routing/a"]');
 
-		await back();
+		await page.goBack();
 		expect(await page.textContent('h1')).toBe('Great success!');
 	});
 
 	test('back button returns to previous route when previous route has been navigated to via hash anchor', async ({
 		page,
-		back,
 		clicknav
 	}) => {
 		await page.goto('/routing/hashes/a');
 
 		await page.click('[href="#hash-target"]');
 		await clicknav('[href="/routing/hashes/b"]');
 
-		await back();
+		await page.goBack();
 		expect(await page.textContent('h1')).toBe('a');
 	});
 
 	test('focus works if page load has hash', async ({ page, browserName }) => {
 		await page.goto('/routing/hashes/target#p2');
 
 		await page.keyboard.press(browserName === 'webkit' ? 'Alt+Tab' : 'Tab');
-		expect(await page.evaluate(() => (document.activeElement || {}).textContent)).toBe(
-			'next focus element'
-		);
+		await page.waitForTimeout(50); // give browser a bit of time to complete the native behavior of the key press
+		expect(
+			await page.evaluate(
+				() => document.activeElement?.textContent || 'ERROR: document.activeElement not set'
+			)
+		).toBe('next focus element');
 	});
 
 	test('focus works when navigating to a hash on the same page', async ({ page, browserName }) => {
@@ -2414,7 +2413,7 @@ test.describe('Routing', () => {
 		expect(await page.textContent('body')).toBe('xyz/abc/qwe');
 	});
 
-	test('rest parameters do not swallow characters', async ({ page, clicknav, back }) => {
+	test('rest parameters do not swallow characters', async ({ page, clicknav }) => {
 		await page.goto('/routing/rest/non-greedy');
 
 		await clicknav('[href="/routing/rest/non-greedy/foo/one/two"]');
@@ -2424,7 +2423,7 @@ test.describe('Routing', () => {
 		await clicknav('[href="/routing/rest/non-greedy/food/one/two"]');
 		expect(await page.textContent('h1')).not.toBe('non-greedy');
 
-		await back();
+		await page.goBack();
 
 		await clicknav('[href="/routing/rest/non-greedy/one-bar/two/three"]');
 		expect(await page.textContent('h1')).toBe('non-greedy');

packages/kit/test/utils.d.ts

@@ -20,7 +20,6 @@ export const test: TestType<
 				prefetch: (url: string) => Promise<void>;
 				prefetchRoutes: (urls: string[]) => Promise<void>;
 			};
-			back: () => Promise<void>;
 			clicknav: (selector: string, options?: { timeout?: number }) => Promise<void>;
 			in_view: (selector: string) => Promise<boolean>;
 			read_errors: (href: string) => string;