From 499d2e7aef3376cc03041b5c7d7b0c69ef3da1a3 Mon Sep 17 00:00:00 2001 From: dominikg Date: Tue, 24 May 2022 20:09:20 +0200 Subject: [PATCH 01/17] test: improve ci e2e setup to include multiple browsers, optimize trace capturing --- .github/workflows/ci.yml | 24 ++++++++++++++++++++++-- packages/kit/test/utils.js | 24 +++++++++++++++++------- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eddd1f5b620f..bab921afc0cd 100644 --- a/.github/workflows/ci.yml +++ b/.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,11 +64,12 @@ jobs: node-version: ${{ matrix.node-version }} cache: pnpm - run: pnpm install --frozen-lockfile + - run: pnpm playwright install - run: pnpm test - 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=- + run: find packages -type d -name test-results -not -empty | tar -czf test-results.tar.gz --exclude='*-retry2*' --exclude='*-retry3*' --exclude='*-retry4*' --exclude='*-retry5*' --files-from=- - name: Upload failed tests screenshots if: failure() uses: actions/upload-artifact@v3 diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index 02c91fb798b2..1bf266ec380e 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -1,7 +1,7 @@ import fs from 'fs'; import http from 'http'; import * as ports from 'port-authority'; -import { test as base } from '@playwright/test'; +import { test as base, devices } from '@playwright/test'; export const test = base.extend({ // @ts-expect-error @@ -135,6 +135,18 @@ export const test = base.extend({ use(read_errors); } }); +const test_browser = process.env.KIT_E2E_BROWSER ?? 'chromium'; +const known_devices = { + chromium: devices['Desktop Chrome'], + firefox: devices['Desktop Firefox'], + safari: devices['Desktop Safari'] +}; + +const test_browser_device = known_devices[test_browser]; + +if (!test_browser_device) { + throw new Error(`invalid test browser specified: KIT_E2E_BROWSER=${process.env.KIT_E2E_BROWSER}. Allowed values: ${Object.keys(known_devices).join(', ')}`); +} /** @type {import('@playwright/test').PlaywrightTestConfig} */ export const config = { @@ -148,24 +160,22 @@ export const config = { retries: process.env.CI ? 5 : 0, projects: [ { - name: `${process.env.DEV ? 'dev' : 'build'}+js`, + name: `${test_browser}-${process.env.DEV ? 'dev' : 'build'}+js`, use: { javaScriptEnabled: true } }, { - name: `${process.env.DEV ? 'dev' : 'build'}-js`, + name: `${test_browser}-${process.env.DEV ? 'dev' : 'build'}-js`, use: { javaScriptEnabled: false } } ], use: { + ...test_browser_device, screenshot: 'only-on-failure', - trace: 'retain-on-failure', - // use stable chrome from host OS instead of downloading one - // see https://playwright.dev/docs/browsers#google-chrome--microsoft-edge - channel: 'chrome' + trace: process.env.KIT_E2E_TRACE ? 'on' : 'on-first-retry' }, workers: process.env.CI ? 2 : undefined }; From 520bb16b55cfa92184b9c1add9226dbfe210e71e Mon Sep 17 00:00:00 2001 From: dominikg Date: Tue, 24 May 2022 20:18:10 +0200 Subject: [PATCH 02/17] fix: action syntax --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bab921afc0cd..a382e3ce047e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,7 +39,7 @@ jobs: matrix: node-version: [16] os: [ubuntu-latest , windows-2019] - e2e-browser: 'chromium' + e2e-browser: ['chromium'] include: - node-version: 16 os: ubuntu-latest From cd51c5adc39faf34faeb9e548d6ee51255e7f8af Mon Sep 17 00:00:00 2001 From: dominikg Date: Tue, 24 May 2022 23:40:57 +0200 Subject: [PATCH 03/17] chore: fix formatting --- packages/kit/test/utils.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index 1bf266ec380e..7228f2d19076 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -145,7 +145,11 @@ const known_devices = { const test_browser_device = known_devices[test_browser]; if (!test_browser_device) { - throw new Error(`invalid test browser specified: KIT_E2E_BROWSER=${process.env.KIT_E2E_BROWSER}. Allowed values: ${Object.keys(known_devices).join(', ')}`); + throw new Error( + `invalid test browser specified: KIT_E2E_BROWSER=${ + process.env.KIT_E2E_BROWSER + }. Allowed values: ${Object.keys(known_devices).join(', ')}` + ); } /** @type {import('@playwright/test').PlaywrightTestConfig} */ From 36ae83ae02fc0d8aeb81e01fd2cfe948e38c8264 Mon Sep 17 00:00:00 2001 From: dominikg Date: Wed, 25 May 2022 10:07:00 +0200 Subject: [PATCH 04/17] fix: wait for kit started on page.goBack and page.reload, remove custom back util. --- packages/kit/test/apps/basics/test/test.js | 33 +++++++++------------- packages/kit/test/utils.d.ts | 1 - packages/kit/test/utils.js | 32 ++++++++------------- 3 files changed, 25 insertions(+), 41 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index cb159068c791..e39fe92db2fd 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -207,17 +207,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); @@ -228,8 +223,7 @@ 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(); @@ -243,7 +237,7 @@ 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'); @@ -304,7 +298,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'); @@ -313,7 +307,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); }); @@ -1921,7 +1915,7 @@ test.describe.parallel('searchParams', () => { }); test.describe.parallel('Redirects', () => { - test('redirect', async ({ baseURL, page, clicknav, back }) => { + test('redirect', async ({ baseURL, page, clicknav }) => { await page.goto('/redirect'); await clicknav('[href="/redirect/a"]'); @@ -1930,7 +1924,7 @@ test.describe.parallel('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`); }); @@ -2270,17 +2264,16 @@ test.describe.parallel('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'); @@ -2288,7 +2281,7 @@ test.describe.parallel('Routing', () => { await page.click('[href="#hash-target"]'); await clicknav('[href="/routing/hashes/b"]'); - await back(); + await page.goBack(); expect(await page.textContent('h1')).toBe('a'); }); @@ -2424,7 +2417,7 @@ test.describe.parallel('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"]'); @@ -2434,7 +2427,7 @@ test.describe.parallel('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'); diff --git a/packages/kit/test/utils.d.ts b/packages/kit/test/utils.d.ts index aea4c91489ca..be8759b36cf8 100644 --- a/packages/kit/test/utils.d.ts +++ b/packages/kit/test/utils.d.ts @@ -19,7 +19,6 @@ export const test: TestType< prefetch: (url: string) => Promise; prefetchRoutes: (urls: string[]) => Promise; }; - back: () => Promise; clicknav: (selector: string, options?: { timeout?: number }) => Promise; in_view: (selector: string) => Promise; read_errors: (href: string) => string; diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index 7228f2d19076..f96fd69fdcb8 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -54,17 +54,6 @@ export const test = base.extend({ }); }, - // @ts-expect-error - back: async ({ page, javaScriptEnabled }, use) => { - use(async () => { - if (javaScriptEnabled) { - await Promise.all([page.goBack(), page.evaluate(() => window.navigated)]); - } else { - return page.goBack().then(() => void 0); - } - }); - }, - // @ts-expect-error clicknav: async ({ page, javaScriptEnabled }, use) => { /** @@ -105,19 +94,22 @@ export const test = base.extend({ }); } - const goto = page.goto; - page.goto = - /** - * @param {string} url - * @param {object} opts - */ - async function (url, opts) { - const res = await goto.call(page, url, opts); + // automatically wait for kit started event after navigation functions if js is enabled + const page_navigation_functions = ['goto', 'goBack', 'reload']; + page_navigation_functions.forEach((fn) => { + const page_fn = page[fn]; + if (!page_fn) { + throw new Error(`function does not exist on page: ${fn}`); + } + page[fn] = async function (...args) { + const res = await page_fn.call(page, ...args); if (javaScriptEnabled) { await page.waitForSelector('body.started', { timeout: 5000 }); } return res; }; + }); + await use(page); }, @@ -179,7 +171,7 @@ export const config = { use: { ...test_browser_device, screenshot: 'only-on-failure', - trace: process.env.KIT_E2E_TRACE ? 'on' : 'on-first-retry' + trace: process.env.KIT_E2E_TRACE ? 'retain-on-failure' : 'on-first-retry' }, workers: process.env.CI ? 2 : undefined }; From 9f05f2117dc314358ac77f142ea2cbc15a13beda Mon Sep 17 00:00:00 2001 From: dominikg Date: Wed, 25 May 2022 11:25:09 +0200 Subject: [PATCH 05/17] fix: improve firefox test results, add browser to results archive name --- .github/workflows/ci.yml | 2 +- packages/kit/test/apps/basics/test/test.js | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a382e3ce047e..1ee9d0487ca2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,5 +75,5 @@ jobs: 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 diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index e39fe92db2fd..47f13a0e5586 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -76,7 +76,7 @@ test.describe.parallel('a11y', () => { selection.addRange(range); return selection.rangeCount; } - return 0; + return -1; }) ).toBe(1); @@ -87,7 +87,7 @@ test.describe.parallel('a11y', () => { if (selection) { return selection.rangeCount; } - return 1; + return -1; }) ).toBe(0); }); @@ -141,7 +141,7 @@ test.describe.parallel('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()); @@ -1942,10 +1942,10 @@ test.describe.parallel('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`); } } }); @@ -2289,7 +2289,8 @@ test.describe.parallel('Routing', () => { await page.goto('/routing/hashes/target#p2'); await page.keyboard.press(browserName === 'webkit' ? 'Alt+Tab' : 'Tab'); - expect(await page.evaluate(() => (document.activeElement || {}).textContent)).toBe( + await page.waitForTimeout(50); + expect(await page.evaluate(() => (document.activeElement?.textContent || 'ERROR: document.activeElement not set'))).toBe( 'next focus element' ); }); From a01f2311bf90d87386de5e938397e60993ab715c Mon Sep 17 00:00:00 2001 From: dominikg Date: Wed, 25 May 2022 11:30:10 +0200 Subject: [PATCH 06/17] chore: fix format --- packages/kit/test/apps/basics/test/test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 47f13a0e5586..f2947216634b 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -2290,9 +2290,11 @@ test.describe.parallel('Routing', () => { await page.keyboard.press(browserName === 'webkit' ? 'Alt+Tab' : 'Tab'); await page.waitForTimeout(50); - expect(await page.evaluate(() => (document.activeElement?.textContent || 'ERROR: document.activeElement not set'))).toBe( - 'next focus element' - ); + 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 }) => { From 035a4e7b884b4d0f2c5a7e13779bc21c64a16fad Mon Sep 17 00:00:00 2001 From: dominikg Date: Wed, 25 May 2022 11:59:12 +0200 Subject: [PATCH 07/17] fix: use downloaded chromium for adapter static test too --- packages/adapter-static/test/utils.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/adapter-static/test/utils.js b/packages/adapter-static/test/utils.js index eeba60c4362a..53ea9f0fb7ea 100644 --- a/packages/adapter-static/test/utils.js +++ b/packages/adapter-static/test/utils.js @@ -48,11 +48,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 From 1f95bb30cd20b8ca04d244b03c44e2f3c9917893 Mon Sep 17 00:00:00 2001 From: dominikg Date: Wed, 25 May 2022 22:31:10 +0200 Subject: [PATCH 08/17] perf: tell vite watcher to ignore playwright output in test projects --- packages/kit/test/apps/amp/svelte.config.js | 7 +++++-- packages/kit/test/apps/basics/svelte.config.js | 4 ++++ packages/kit/test/apps/options-2/svelte.config.js | 4 ++++ packages/kit/test/apps/options/svelte.config.js | 4 ++++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/kit/test/apps/amp/svelte.config.js b/packages/kit/test/apps/amp/svelte.config.js index 9c3a2801c70a..ee5ba8bb44c4 100644 --- a/packages/kit/test/apps/amp/svelte.config.js +++ b/packages/kit/test/apps/amp/svelte.config.js @@ -15,10 +15,13 @@ const config = { fs: { allow: [path.resolve('../../../src')] }, - // TODO: required to support ipv6, remove on vite 3 // https://github.com/vitejs/vite/issues/7075 - host: 'localhost' + host: 'localhost', + watch: { + // perf, do not watch playwright output dir + ignored: ['**/test-results/**'] + } } } } diff --git a/packages/kit/test/apps/basics/svelte.config.js b/packages/kit/test/apps/basics/svelte.config.js index 68b1411a1fdc..917b60fc008d 100644 --- a/packages/kit/test/apps/basics/svelte.config.js +++ b/packages/kit/test/apps/basics/svelte.config.js @@ -22,6 +22,10 @@ const config = { host: 'localhost', fs: { allow: [path.resolve('../../../src')] + }, + watch: { + // perf, do not watch playwright output dir + ignored: ['**/test-results/**'] } } }, diff --git a/packages/kit/test/apps/options-2/svelte.config.js b/packages/kit/test/apps/options-2/svelte.config.js index 8160cdf0cb8f..14afa4cfbcf4 100644 --- a/packages/kit/test/apps/options-2/svelte.config.js +++ b/packages/kit/test/apps/options-2/svelte.config.js @@ -16,6 +16,10 @@ const config = { host: 'localhost', fs: { allow: [path.resolve('../../../src')] + }, + watch: { + // perf, do not watch playwright output dir + ignored: ['**/test-results/**'] } } } diff --git a/packages/kit/test/apps/options/svelte.config.js b/packages/kit/test/apps/options/svelte.config.js index 569ef37d3af6..82bd599d984a 100644 --- a/packages/kit/test/apps/options/svelte.config.js +++ b/packages/kit/test/apps/options/svelte.config.js @@ -33,6 +33,10 @@ const config = { host: 'localhost', fs: { allow: [path.resolve('../../../src')] + }, + watch: { + // perf, do not watch playwright output dir + ignored: ['**/test-results/**'] } } }, From 76377849536a3ca76bb3196a8812c18a00a1610b Mon Sep 17 00:00:00 2001 From: dominikg Date: Fri, 10 Jun 2022 14:26:26 +0200 Subject: [PATCH 09/17] ci: archive all test results including retries --- .github/workflows/ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1ee9d0487ca2..2ecbbf8e0ae5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,11 +66,11 @@ jobs: - 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='*-retry2*' --exclude='*-retry3*' --exclude='*-retry4*' --exclude='*-retry5*' --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: From 4a82b3eca355060d5f0093be4c7a903dfba95a31 Mon Sep 17 00:00:00 2001 From: dominikg Date: Fri, 10 Jun 2022 14:29:56 +0200 Subject: [PATCH 10/17] chore: add comment for crude wait in test --- packages/kit/test/apps/basics/test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index f2947216634b..cdc09a0f6f40 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -2289,7 +2289,7 @@ test.describe.parallel('Routing', () => { await page.goto('/routing/hashes/target#p2'); await page.keyboard.press(browserName === 'webkit' ? 'Alt+Tab' : 'Tab'); - await page.waitForTimeout(50); + 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' From dc1af208d19f47e1f9e615fdbda1fcdafd953e1b Mon Sep 17 00:00:00 2001 From: dominikg Date: Fri, 10 Jun 2022 14:33:51 +0200 Subject: [PATCH 11/17] fix: add playwright back to root package.json so that `pnpm playwright install` works in ci scripts --- package.json | 1 + pnpm-lock.yaml | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/package.json b/package.json index 43db411518fa..5c71bd218fa8 100644 --- a/package.json +++ b/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", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4f070644e361..29d8b93fd0db 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -16,6 +16,7 @@ importers: 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 @@ -35,6 +36,7 @@ importers: eslint-plugin-import: 2.26.0_xsmuhwqsfrjm7m3kqio7zoeziq eslint-plugin-node: 11.1.0_eslint@8.16.0 eslint-plugin-svelte3: 4.0.0_vypdqzeyqutkgs6qzc7qod4c64 + playwright: 1.22.2 prettier: 2.6.2 rollup: 2.75.3 svelte: 3.48.0 @@ -4015,6 +4017,15 @@ packages: hasBin: true dev: true + /playwright/1.22.2: + resolution: {integrity: sha512-hUTpg7LytIl3/O4t0AQJS1V6hWsaSY5uZ7w1oCC8r3a1AQN5d6otIdCkiB3cbzgQkcMaRxisinjMFMVqZkybdQ==} + engines: {node: '>=14'} + hasBin: true + requiresBuild: true + dependencies: + playwright-core: 1.22.2 + dev: true + /polka/1.0.0-next.22: resolution: {integrity: sha512-a7tsZy5gFbJr0aUltZS97xCkbPglXuD67AMvTyZX7BTDBH384FWf0ZQF6rPvdutSxnO1vUlXM2zSLf5tCKk5RA==} engines: {node: '>=8'} From ea7b60d9badf98962dfa63de2ca5626cdc825ad7 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Jul 2022 12:08:54 -0400 Subject: [PATCH 12/17] fix firefox bug, i think --- packages/kit/src/runtime/client/client.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/client.js b/packages/kit/src/runtime/client/client.js index b844f637f464..5ca0509438d6 100644 --- a/packages/kit/src/runtime/client/client.js +++ b/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); From 2bf69344376e4a41c5e4a7df422dd94805ee8b35 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 6 Jul 2022 14:33:38 -0400 Subject: [PATCH 13/17] try to diagnose failure --- packages/kit/test/apps/basics/test/test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index e33c45315baa..107fa3e6f336 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -228,6 +228,8 @@ test.describe('Scrolling', () => { const y1 = await page.evaluate(() => scrollY); + console.error({ y1 }); + await page.click('[href="/scroll/cross-document/b"]'); expect(await page.textContent('h1')).toBe('b'); await page.waitForSelector('body.started'); From ea2f866ef3de34d60355e88bf46d7847a28b80b3 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 17:02:34 -0400 Subject: [PATCH 14/17] cross fingers, sacrifice goat --- packages/kit/test/apps/basics/test/test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 107fa3e6f336..bd110d5b5923 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -226,6 +226,8 @@ test.describe('Scrolling', () => { await page.goto('/scroll/cross-document/a'); await page.locator('[href="/scroll/cross-document/b"]').scrollIntoViewIfNeeded(); + await page.waitForTimeout(50); + const y1 = await page.evaluate(() => scrollY); console.error({ y1 }); From 61a154e1998530caced50c252d3943b58238a461 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 18:15:56 -0400 Subject: [PATCH 15/17] perchance this pleaseth the webkit gods --- packages/kit/test/apps/basics/test/test.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index bd110d5b5923..222f338114eb 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -224,13 +224,12 @@ test.describe('Scrolling', () => { clicknav }) => { await page.goto('/scroll/cross-document/a'); - await page.locator('[href="/scroll/cross-document/b"]').scrollIntoViewIfNeeded(); - await page.waitForTimeout(50); + const rect = await page.locator('[href="/scroll/cross-document/b"]').boundingBox(); + const height = await page.evaluate(() => innerHeight); - const y1 = await page.evaluate(() => scrollY); - - console.error({ y1 }); + 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'); @@ -246,9 +245,9 @@ test.describe('Scrolling', () => { 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(10); // we need a few pixels wiggle room, because browsers }); test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({ From de831ee24b7df88f31a7b8dc9707bc201e6f26f8 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 7 Jul 2022 18:40:15 -0400 Subject: [PATCH 16/17] appease firefox, which i think scrolls to the focused element or something --- packages/kit/test/apps/basics/test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 222f338114eb..655aee1ec231 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -247,7 +247,7 @@ test.describe('Scrolling', () => { const scroll_y = await page.evaluate(() => scrollY); - expect(Math.abs(scroll_y - target_scroll_y)).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 ({ From 3218239cdaaddc10bbc08b34399155ebd389768c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 8 Jul 2022 10:43:15 -0400 Subject: [PATCH 17/17] Create breezy-hairs-greet.md --- .changeset/breezy-hairs-greet.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/breezy-hairs-greet.md diff --git a/.changeset/breezy-hairs-greet.md b/.changeset/breezy-hairs-greet.md new file mode 100644 index 000000000000..73d7130d7ec0 --- /dev/null +++ b/.changeset/breezy-hairs-greet.md @@ -0,0 +1,5 @@ +--- +"@sveltejs/kit": patch +--- + +reset selection in setTimeout after navigating, to ensure correct behaviour in Firefox