From f49ac6610e72db605535fce787bfbe1aa3da3e2e Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Tue, 23 Jul 2024 00:28:43 +0800 Subject: [PATCH 1/8] wait for download event --- .../basics/test/cross-platform/client.test.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index 5b3d1fa953f5..3005d2f27eb2 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -223,13 +223,20 @@ test.describe('Navigation lifecycle functions', () => { test('beforeNavigate is triggered after clicking a download link', async ({ page, baseURL }) => { await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation'); - await page.click('a[download]'); - expect(await page.innerHTML('pre')).toBe('0 false undefined'); + const download = page.waitForEvent('download', { timeout: 3000 }); + await page.locator('a[download]').click(); + await (await download).cancel(); + + const current_url = baseURL + '/navigation-lifecycle/before-navigate/prevent-navigation'; + + expect(page.url()).toBe(current_url); + expect(await page.locator('pre').innerText()).toBe('0 false undefined'); await page.click('a[href="/navigation-lifecycle/before-navigate/a"]'); + await page.waitForLoadState('networkidle'); - expect(page.url()).toBe(baseURL + '/navigation-lifecycle/before-navigate/prevent-navigation'); - expect(await page.innerHTML('pre')).toBe('1 false link'); + expect(page.url()).toBe(current_url); + expect(await page.locator('pre').innerText()).toBe('1 false link'); }); test('afterNavigate calls callback', async ({ page, clicknav }) => { From 8367a590f19e448e4eecbf8a11a644b290a0b2e5 Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Tue, 23 Jul 2024 00:40:02 +0800 Subject: [PATCH 2/8] reuse url --- .../kit/test/apps/basics/test/cross-platform/client.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index 3005d2f27eb2..ef26ba0e296d 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -221,13 +221,15 @@ test.describe('Navigation lifecycle functions', () => { }); test('beforeNavigate is triggered after clicking a download link', async ({ page, baseURL }) => { - await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation'); + const pathname = '/navigation-lifecycle/before-navigate/prevent-navigation'; + + await page.goto(pathname); const download = page.waitForEvent('download', { timeout: 3000 }); await page.locator('a[download]').click(); await (await download).cancel(); - const current_url = baseURL + '/navigation-lifecycle/before-navigate/prevent-navigation'; + const current_url = baseURL + pathname; expect(page.url()).toBe(current_url); expect(await page.locator('pre').innerText()).toBe('0 false undefined'); From 73ada90a43531ed9564383005367c6a3a460079d Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Tue, 23 Jul 2024 23:04:32 +0800 Subject: [PATCH 3/8] remove timeout override --- .../kit/test/apps/basics/test/cross-platform/client.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index ef26ba0e296d..f2f7dd2ded6d 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -225,7 +225,7 @@ test.describe('Navigation lifecycle functions', () => { await page.goto(pathname); - const download = page.waitForEvent('download', { timeout: 3000 }); + const download = page.waitForEvent('download'); await page.locator('a[download]').click(); await (await download).cancel(); From 13ec20734892fa4770323f33a19eefe998642e49 Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Tue, 23 Jul 2024 23:05:45 +0800 Subject: [PATCH 4/8] use clicknav util --- .../apps/basics/test/cross-platform/client.test.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index f2f7dd2ded6d..3b428e19f2af 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -220,13 +220,17 @@ test.describe('Navigation lifecycle functions', () => { expect(await page.innerHTML('pre')).toBe('2 false goto'); }); - test('beforeNavigate is triggered after clicking a download link', async ({ page, baseURL }) => { + test('beforeNavigate is triggered after clicking a download link', async ({ + page, + baseURL, + clicknav + }) => { const pathname = '/navigation-lifecycle/before-navigate/prevent-navigation'; await page.goto(pathname); const download = page.waitForEvent('download'); - await page.locator('a[download]').click(); + await clicknav('a[download]'); await (await download).cancel(); const current_url = baseURL + pathname; @@ -234,8 +238,7 @@ test.describe('Navigation lifecycle functions', () => { expect(page.url()).toBe(current_url); expect(await page.locator('pre').innerText()).toBe('0 false undefined'); - await page.click('a[href="/navigation-lifecycle/before-navigate/a"]'); - await page.waitForLoadState('networkidle'); + await clicknav('a[href="/navigation-lifecycle/before-navigate/a"]'); expect(page.url()).toBe(current_url); expect(await page.locator('pre').innerText()).toBe('1 false link'); From 3f38a450562093f085c1a0d32920afcd1989401a Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Tue, 23 Jul 2024 23:16:54 +0800 Subject: [PATCH 5/8] oops we don't want to wait for nav when clicking download --- .../kit/test/apps/basics/test/cross-platform/client.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index 3b428e19f2af..8a4add1d72d1 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -230,7 +230,7 @@ test.describe('Navigation lifecycle functions', () => { await page.goto(pathname); const download = page.waitForEvent('download'); - await clicknav('a[download]'); + await page.locator('a[download]').click(); await (await download).cancel(); const current_url = baseURL + pathname; From 896c37c00e54ec83165addb429aba4c51fb7ebd0 Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Wed, 24 Jul 2024 01:15:23 +0800 Subject: [PATCH 6/8] beforeNavigate prevents navigation so don't use clicknav --- .../kit/test/apps/basics/test/cross-platform/client.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index 8a4add1d72d1..bc1507a1a4c4 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -223,7 +223,6 @@ test.describe('Navigation lifecycle functions', () => { test('beforeNavigate is triggered after clicking a download link', async ({ page, baseURL, - clicknav }) => { const pathname = '/navigation-lifecycle/before-navigate/prevent-navigation'; @@ -238,7 +237,7 @@ test.describe('Navigation lifecycle functions', () => { expect(page.url()).toBe(current_url); expect(await page.locator('pre').innerText()).toBe('0 false undefined'); - await clicknav('a[href="/navigation-lifecycle/before-navigate/a"]'); + await page.locator('a[href="/navigation-lifecycle/before-navigate/a"]').click(); expect(page.url()).toBe(current_url); expect(await page.locator('pre').innerText()).toBe('1 false link'); From 139a3186c936fedbb57132b7b0d8d7f02c4d2c97 Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Wed, 24 Jul 2024 01:26:43 +0800 Subject: [PATCH 7/8] try page.waitForUrl --- .../apps/basics/test/cross-platform/client.test.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index bc1507a1a4c4..aeefb2082dd8 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -220,26 +220,22 @@ test.describe('Navigation lifecycle functions', () => { expect(await page.innerHTML('pre')).toBe('2 false goto'); }); - test('beforeNavigate is triggered after clicking a download link', async ({ - page, - baseURL, - }) => { + test('beforeNavigate is triggered after clicking a download link', async ({ page, baseURL }) => { const pathname = '/navigation-lifecycle/before-navigate/prevent-navigation'; + const current_url = baseURL + pathname; await page.goto(pathname); const download = page.waitForEvent('download'); await page.locator('a[download]').click(); + await page.waitForURL(current_url); await (await download).cancel(); - const current_url = baseURL + pathname; - - expect(page.url()).toBe(current_url); expect(await page.locator('pre').innerText()).toBe('0 false undefined'); await page.locator('a[href="/navigation-lifecycle/before-navigate/a"]').click(); + await page.waitForURL(current_url); - expect(page.url()).toBe(current_url); expect(await page.locator('pre').innerText()).toBe('1 false link'); }); From b2bdc84087c70b2e097d084ed9e76878d332d146 Mon Sep 17 00:00:00 2001 From: Tee Ming Date: Wed, 24 Jul 2024 03:09:32 +0800 Subject: [PATCH 8/8] apply Ben's suggestions --- .../apps/basics/test/cross-platform/client.test.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/kit/test/apps/basics/test/cross-platform/client.test.js b/packages/kit/test/apps/basics/test/cross-platform/client.test.js index aeefb2082dd8..313cdfec80ae 100644 --- a/packages/kit/test/apps/basics/test/cross-platform/client.test.js +++ b/packages/kit/test/apps/basics/test/cross-platform/client.test.js @@ -220,23 +220,18 @@ test.describe('Navigation lifecycle functions', () => { expect(await page.innerHTML('pre')).toBe('2 false goto'); }); - test('beforeNavigate is triggered after clicking a download link', async ({ page, baseURL }) => { - const pathname = '/navigation-lifecycle/before-navigate/prevent-navigation'; - const current_url = baseURL + pathname; - - await page.goto(pathname); + test('beforeNavigate is triggered after clicking a download link', async ({ page }) => { + await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation'); const download = page.waitForEvent('download'); await page.locator('a[download]').click(); - await page.waitForURL(current_url); await (await download).cancel(); - expect(await page.locator('pre').innerText()).toBe('0 false undefined'); + await expect(page.locator('pre')).toHaveText('0 false undefined'); await page.locator('a[href="/navigation-lifecycle/before-navigate/a"]').click(); - await page.waitForURL(current_url); - expect(await page.locator('pre').innerText()).toBe('1 false link'); + await expect(page.locator('pre')).toHaveText('1 false link'); }); test('afterNavigate calls callback', async ({ page, clicknav }) => {