Skip to content

Conversation

Smrtnyk
Copy link
Contributor

@Smrtnyk Smrtnyk commented Apr 6, 2025

I have noticed that current playwright version is 20 versions old, and what also means that it is using very old browsers.
Updating playwright always to latest is a good idea to get all the bugfixes and performance improvements + being able to test on bleeding edge browser versions

Copy link

codesandbox bot commented Apr 6, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@@ -60,7 +60,7 @@
"test:vitest:coverage:watch": "vitest --coverage=true",
"coverage:merge": "nyc merge coveragefiles .nyc_output/merged-coverage.json",
"local-server": "http-server ./ -d=false",
"test:e2e": "npx playwright test --headed",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no need for npx if playwright is installed locally as a dev dep

Copy link
Member

Choose a reason for hiding this comment

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

i think the point of this was to avoid installing it during all the git hub actions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah ok, I see now

Copy link
Member

Choose a reason for hiding this comment

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

Dependency have also been cached, so this may not be necessary at all. The time was 2m before is 2m now, is ok to remove npx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed again

@asturur
Copy link
Member

asturur commented Apr 6, 2025

You need to add a line in the changelog for things to pass the CI

@Smrtnyk
Copy link
Contributor Author

Smrtnyk commented Apr 6, 2025

You need to add a line in the changelog for things to pass the CI

I will add it to the changelog, first I wanted to see if you are interested for the PR

@Smrtnyk Smrtnyk force-pushed the ci()-update-playwright branch from 7a0639d to 8eb1e3f Compare April 6, 2025 20:16
@asturur
Copy link
Member

asturur commented Apr 6, 2025

Updates are always ok. if it pass the tests, is ok.

@Smrtnyk Smrtnyk force-pushed the ci()-update-playwright branch from 8eb1e3f to a87e7c8 Compare April 6, 2025 20:22
@Smrtnyk Smrtnyk force-pushed the ci()-update-playwright branch from a87e7c8 to ccf2421 Compare April 6, 2025 20:23
@asturur asturur merged commit 4256748 into fabricjs:master Apr 6, 2025
18 of 22 checks passed
@Smrtnyk Smrtnyk deleted the ci()-update-playwright branch April 11, 2025 12:32
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.

2 participants