-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat: init support for e2e libs with Playwright #131
feat: init support for e2e libs with Playwright #131
Conversation
54bc9af
to
07908f2
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.
Thx, looks very useful and Great Work so far 👍🏽
Left a few suggestions
can you also please update the README.md
and CHANGELOG.md
/* Opt out of parallel tests on CI. */ | ||
workers: process.env.CI ? 1 : undefined, |
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.
Do we want this by default?
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.
I believe this is a sensible default?
07908f2
to
00f86fa
Compare
Abstract Middleware logic from dependency on Cypress Add support for Playwright Signed-off-by: Khaled Emara <[email protected]>
Signed-off-by: Khaled Emara <[email protected]>
Signed-off-by: Khaled Emara <[email protected]>
f4c2c0b
to
3f0afb2
Compare
Signed-off-by: Khaled Emara <[email protected]>
5e82375
to
67502f4
Compare
Signed-off-by: Khaled Emara <[email protected]>
67502f4
to
f48b84a
Compare
@grantspeelman @alexeyr-ci I think the PR is ready. It passes the CI for two envs but not the third for a weird reason. |
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.
Great Work, looking real good 👍🏽
One last suggestion
67b7cf2
to
d33d6bd
Compare
Co-authored-by: Grant Petersen-Speelman <[email protected]> Signed-off-by: Khaled Emara <[email protected]>
d33d6bd
to
506f082
Compare
Signed-off-by: Khaled Emara <[email protected]>
Signed-off-by: Khaled Emara <[email protected]>
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.
Well Done and Thank you 👍🏽
Amazing, nice work @KhaledEmaraDev ! |
const appVcrEjectCassette = async () => { | ||
const context = await contextPromise; | ||
|
||
const response = await context.post("/__cypress__/vcr/eject"); |
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.
Should this be
const response = await context.post("/__e2e__/vcr/eject");
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.
You are right. Don't know how I missed it. Will create a PR for it.
Fixes #116