Skip to content

chore: fix flaky scroll test #13063

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

chore: fix flaky scroll test #13063

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/kit/test/apps/basics/test/cross-platform/client.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
test.describe.configure({ mode: 'parallel' });

test.describe('a11y', () => {
test('resets focus', async ({ page, clicknav, browserName }) => {

Check warning on line 12 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, windows-2019, chromium, dev)

flaky test: resets focus 

retries: 2
const tab = browserName === 'webkit' ? 'Alt+Tab' : 'Tab';

await page.goto('/accessibility/a');
Expand All @@ -33,7 +33,7 @@
expect(await page.evaluate(() => document.documentElement.getAttribute('tabindex'))).toBe(null);
});

test('applies autofocus after a navigation', async ({ page, clicknav }) => {

Check warning on line 36 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, windows-2019, chromium, dev)

flaky test: applies autofocus after a navigation 

retries: 2
await page.goto('/accessibility/autofocus/a');

await clicknav('[href="/accessibility/autofocus/b"]');
Expand Down Expand Up @@ -119,7 +119,7 @@
});

test.describe('Navigation lifecycle functions', () => {
test('beforeNavigate prevents navigation triggered by link click', async ({ page, baseURL }) => {

Check warning on line 122 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, macOS-latest, webkit, build)

flaky test: beforeNavigate prevents navigation triggered by link click 

retries: 2
await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation');

await page.click('[href="/navigation-lifecycle/before-navigate/a"]');
Expand Down Expand Up @@ -452,6 +452,8 @@

await page.goto('/anchor');
await page.evaluate(() => window.scrollTo(0, 1000));
// wait for the scroll to be processed before jumping to the next page
await page.waitForTimeout(1000);
Copy link
Member

@dominikg dominikg Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need a full second here or should we reduce it to 100ms or sth? Also is the same needed after the second scroll? A nicer way could be to await the fact that scrollY is 1000 🤔

Copy link
Member

@benmccann benmccann Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you could use page.waitForFunction to avoid a timeout and instead wait for the page position to make it to 1000 pixels?

await app.goto('/anchor/anchor');
await page.evaluate(() => window.scrollTo(0, 1000));

Expand Down Expand Up @@ -800,7 +802,7 @@
await expect(page.locator('h1')).toHaveText('target: 1');
});

test('responds to <form method="GET"> submission without reload', async ({ page }) => {

Check warning on line 805 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, ubuntu-latest, firefox, dev)

flaky test: responds to <form method="GET"> submission without reload 

retries: 2
await page.goto('/routing/form-get');

expect(await page.textContent('h1')).toBe('...');
Expand All @@ -820,7 +822,7 @@
expect(await page.textContent('h3')).toBe('bar');
});

test('responds to <form target="_blank"> submission with new tab', async ({ page }) => {

Check warning on line 825 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, ubuntu-latest, firefox, build)

flaky test: responds to <form target="_blank"> submission with new tab 

retries: 2

Check warning on line 825 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, macOS-latest, webkit, build)

flaky test: responds to <form target="_blank"> submission with new tab 

retries: 2
await page.goto('/routing/form-target-blank');

let tabs = page.context().pages();
Expand All @@ -834,7 +836,7 @@
expect(tabs.length > 1);
});

test('responds to <button formtarget="_blank" submission with new tab', async ({ page }) => {

Check warning on line 839 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, ubuntu-latest, firefox, build)

flaky test: responds to <button formtarget="_blank" submission with new tab 

retries: 2
await page.goto('/routing/form-target-blank');

let tabs = page.context().pages();
Expand Down