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

chore: add more browsers to playwright config #1776

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thepassle
Copy link
Contributor

No description provided.

@thepassle
Copy link
Contributor Author

Welp, looks like a bunch of failing tests 😅 That's a lot more than I was expecting though, I was expecting maybe a couple. Im not sure if I'll have time to address all of these today, but ill try to dive in sometime this week

@kettanaito
Copy link
Member

kettanaito commented Oct 18, 2023

@thepassle, we can address those together in our spare time. MSW as it is doesn't promise nor offer any particular browser support. I always saw MSW as a devtool, and if it doesn't work in one browser simply switch to another and work with it there (similar to how every browser has different devtools available).

That being said, I do find great value in cross-browser testing and we should pursue this endeavor. I think we can improve how browser tests run in CI so it's more apparent what browser corresponds to what failed tests, as well as parallelize cross-browser runs since GitHub Actions has 1 thread and will run all tests for all browsers sequentially. Browser tests already take the most time on CI, and if we add more browser targets, we have to likely introduce separate GitHub workflows for those so they can run in parallel and save us some time.

@kettanaito kettanaito marked this pull request as draft October 18, 2023 09:34
@thepassle
Copy link
Contributor Author

Yeah there's no rush to getting this in, but it would definitely be nice in general to improve the browser support for the library.

MSW as it is doesn't promise nor offer any particular browser support. I always saw MSW as a devtool, and if it doesn't work in one browser simply switch to another and work with it there (similar to how every browser has different devtools available).

At ING we encourage our developers to test their code on multiple browsers, developers can do this manually during local dev, but also often teams use automated testing via @web/test-runner to run their tests on multiple browsers. While a lot of JS syntax-compatibility related things can be transpiled away at build time, not everything is statically analyzable, or detectable, or polyfillable. As a bank, browser compat is very important to us; also for local development.

While I agree that MSW shouldn't have to bend over backwards to support IE11, but I don't think its unreasonable to at least make sure the library works on the latest version of the major browsers (chromium/firefox/safari).

That being said, I do find great value in cross-browser testing and we should pursue this endeavor.

Sounds good! Like I mentioned, there's no rush to get this in. Ill try to find some time this week to start chipping away at this :)

@kettanaito
Copy link
Member

While I agree that MSW shouldn't have to bend over backwards to support IE11, but I don't think its unreasonable to at least make sure the library works on the latest version of the major browsers (chromium/firefox/safari).

Absolutely agree.

@kettanaito kettanaito added this to the Post-2.0 milestone Oct 22, 2023
Base automatically changed from feat/standard-api to main October 23, 2023 07:45
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 this pull request may close these issues.

None yet

2 participants