-
Notifications
You must be signed in to change notification settings - Fork 100
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(ci): only install headless shell chromium for playwright #2544
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2544 +/- ##
=========================================
Coverage 43.40% 43.40%
Complexity 882 882
=========================================
Files 77 77
Lines 3359 3359
=========================================
Hits 1458 1458
Misses 1901 1901 |
79414b3
to
6e6633f
Compare
This is all we need for playwright right now. https://playwright.dev/docs/browsers#chromium-headless-shell Signed-off-by: Max <[email protected]>
6e6633f
to
b9d7c35
Compare
I would like to revert this. Speed is no metric here but coverage. We had many regressions in Safari and co meaning browser specific. So we should test all different browsers. |
Ok for me. :) or are there any other improvements on the performance possible that include the use of other browsers? |
@susnux Even before this PR we the test were only run with chromium as that's the only browser defined in https://github.com/nextcloud/forms/blob/main/playwright.config.ts#L40-L46 I don't think Safari was installed before this PR either. My understanding is that it was firefox, webkit, chromium and headless chromium - of which only the latter was actually used. |
@max-nextcloud sorry wrong project 🙈 I was somehow thinking we test Safari but that nc-vue not forms where we test Safari. |
Speeds up the installation of the browsers drastically: