-
Notifications
You must be signed in to change notification settings - Fork 474
feat: add Playwright for end-to-end testing #76
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
Conversation
cefe4cf
to
6bace64
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI is failing. I think we need to also skip playwright tests in scripts/test.mjs
since it requires different environment.
2b8a7a1
to
935b7b4
Compare
935b7b4
to
feee094
Compare
Applied the review feedback, sorry for the late response! Not sure if we should skip it inside the integration tests or run them - with my new implementation we run them. (Would also be awesome if you could "lax" the GitHub Action permissions a big, so you don't have to approve all the time here) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review.
I've tried the PR locally and it's quite a joy to test it.
Just a few nitpicks that needs to be improved.
@@ -1,6 +1,6 @@ | |||
{ | |||
"scripts": { | |||
"test:unit": "vitest --environment jsdom" | |||
"test:unit": "vitest --environment jsdom --root src/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why changing this line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise vitest tries to process the playwright test files as well, this won't work and lead to errors. Maybe there is a better way of handling that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello! It seems that this addition introduces a problem. When running Vitest it tries to create a results.json
inside its cache directory. This directory is defined as resolve(root, slash(dir || 'node_modules/.vitest'))
. This means that with --root src/
added, Vitest creates the results.json
inside the <project_root>/src/node_modules/.vitest
instead of the actual node_modules directory (<project_root>/node_modules/.vitest
).
72b2491
to
b2a32e5
Compare
No worries, was out of office myself so I did not notice at all. Applied the suggestions. Lets see what results the CI will give us. |
Out of curiosity, what's the current status on this? Playwright definitely is an interesting alternative to Cypress, so having it in there as an easy-to-access option seems lovely. |
@mxschmitt I wonder why this PR is closed?😕 |
b2a32e5
to
0891c8e
Compare
f116e99
to
d0a19ee
Compare
@sodatea sorry for the huge delay, I was partially ooo back then and then I forgot about it. Would be nice if you could enable GitHub Actions for contributors, this makes me easier to ensure its all green instead of having to wait for a maintainer to click approve. I hope I addressed the comments, if not, please let me know! The updates Playground PR are here: vuejs/create-vue-templates#1 @sand4rt I rebased it! |
d0a19ee
to
b2d9cc1
Compare
b2d9cc1
to
211b674
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good to me now.
Thank you for putting so much effort into this PR!
I'm merging the PR today.
But FYI, before cutting the next release, I'd like to make some adjustments to the workflow of this repo, the tsconfigs, and the CLI prompts, which would take some time.
So I expect this feature to be included in the October release.
Hello, 👋
Playwright is getting more and more traction over the last years and we should provide users an easy integration with the Vue ecosystem so users benefit from it.
This Pull Request adds Playwright as an end-to-end testing solution like it's done for Cypress. It might make sense to throw an error if the user selects both variants for e2e testing.
We'll follow-up once the component testing story with Playwright and Vite is ready.
Thank you
cc @sodatea