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

[test] Failing BrowserStack tests #27991

Closed
michaldudak opened this issue Aug 27, 2021 · 4 comments · Fixed by #28041
Closed

[test] Failing BrowserStack tests #27991

michaldudak opened this issue Aug 27, 2021 · 4 comments · Fixed by #28041

Comments

@michaldudak
Copy link
Member

michaldudak commented Aug 27, 2021

This is a follow-up to #27179 (comment)
The browser test timeouts occur increasingly more often because of the limited processing power of BrowserStack.

Some of the solutions available:

  1. Removing some browsers from the test matrix (Firefox looks like the good candidate as it's slow compared to the others)
  2. Removing the BrowserStack tests during PR and doing them periodically on the main branch (and before releases)
  3. Removing the BrowserStack tests completely
  4. Moving the run of the karma tests from BrowserStack to Selenium in CircleCI
  5. Only run BrowserStack on the baseline branches (HEAD, next, master)
  6. more?
@eps1lon
Copy link
Member

eps1lon commented Aug 27, 2021

I understand the frustration here.

Removing firefox or safari is probably the worst solution since those are the browsers who are most likely to deviate from headless chrome.

Running them periodically only on the upstream branches makes the most sense in my opinion (failing would result in a blocked release).

Though personally I'm all for removing them completely. What they specifically test should really only be either visual regression or e2e tests. Both of these jobs weren't as performant or viable when test_browsers was introduce so this is a good opportunity to re-evaluate what the point of test_browser is.

@flaviendelangle
Copy link
Member

Removing firefox or safari is probably the worst solution since those are the browsers who are most likely to deviate from headless chrome.

I agree that letting the test_browser only for Chrome-based browser would make this step quite useless.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 30, 2021

@michaldudak I have update the options listed to introduce more diversity. It's also to be noted that #16263 is related to this pain.

I personally resonate with options 4. or 5. For instance, we could start with 5, if we see that we introduce regressions too often, then we could invest in 4 to move these tests back on the PRs (maybe it will never get to this).

It seems that it's what #28041 goes after (I didn't see it before starting to comment). @flaviendelangle does it work for you? Do you want to cherry-pick this approach in X?

@flaviendelangle
Copy link
Member

Fine for me 👍

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

Successfully merging a pull request may close this issue.

4 participants