Skip to content
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.

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

Reduce the use of Webkit in browser tests #8578

Closed
benmccann opened this issue Jan 17, 2023 · 3 comments · Fixed by #8585
Closed

Reduce the use of Webkit in browser tests #8578

benmccann opened this issue Jan 17, 2023 · 3 comments · Fixed by #8585

Comments

@benmccann
Copy link
Member

Describe the problem

The WebKit tests are flaky. My best guess is that the WebKit playwright integration is poorly maintained as it is likely the least used. fetch just sometimes doesn't run in both the form action tests (#8570) and in the caching tests (#8225). Playwright for WebKit also sometimes returns content that clearly isn't in the page (microsoft/playwright#17557)

We added WebKit tests after it was discovered that we had a scrolling bug in Safari (#4065).
This is the only Safari-specific issue I recall though it's possible there have been others. We had to tweak the tests a bit to get them working in WebKit as it handled things like focus, browser history, fetch headers, and displaying client errors slightly differently (#4847), but these things were all working in Safari and it was only the test code that had to be updated.

Describe the proposed solution

I think we should be more explicit about which things need to tested cross-browser. There are some things like our 'Routing' and 'Scrolling' tests that probably make sense to run on MacOS WebKit, but I'm not sure the rest do. I'd probably let everything run on Windows still as we do catch lots of errors that are Windows-specific and the test runner there has not been causing us the same kinds of headaches

Alternatives considered

Importance

i cannot use SvelteKit without it

Additional Information

No response

@dominikg
Copy link
Member

#8151 was just meant as an investigative measure to see if the problem is caused by the macos runner somehow or actually by the webkit tests. I don't think it is an alternative to outright skip safari tests altogether.

Limiting non-chromium tests to a browser-feature relevant suite may be a worthwhile endeavor, as both macos+safari and linux+firefox are the slowest to finish in the matrix.

Problem is how do you determine tests that do not rely on browser engine behavior and are safe to be executed once in chrome only?

@Rich-Harris
Copy link
Member

Problem is how do you determine tests that do not rely on browser engine behavior and are safe to be executed once in chrome only?

One approach would be innocent-until-proven-guilty — assume all browsers behave identically until we get bug reports that something is broken in Safari. Faster test suite, but our users (and theirs) pay for it.

@benmccann
Copy link
Member Author

There's quite a lot that pretty obviously fall in one camp or the other. The scroll tests should run cross-browser while the tests in server.test.js really don't need to be run in WebKit. There might be some that subtly rely on browser engine behavior that aren't obvious, but I'm not sure there's a lot and we're really gaining a lot of value there. I feel like losing a few of those in favor of a greener CI would give me more confidence overall.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants