-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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: Add @cypress/puppeteer plugin #28370
Conversation
This reverts commit d8b5f35.
…nto issue-28023-cy-puppeteer
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.
Very clean and great documentation. Just a few comments.
npm/puppeteer/README.md
Outdated
on, | ||
onMessage: { | ||
async switchToTabAndGetContent (browser) { | ||
// ... |
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.
@chrisbreiding I would probably put an explicit comment here that says you need to now write Puppeteer code to execute what you want. I think someone might copy/paste this and assume it should do what it says. 🙂
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.
Good idea. Addressed in 1d8356a
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.
Was this addressed? I don't see an explicit comment?
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.
Ah good catch, I had put the comment here but it's a good idea to add it in this spot as well.
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.
Actually addressed this time here
npm/puppeteer/README.md
Outdated
## yarn | ||
|
||
```sh | ||
yarn add @cypress/puppeteer |
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.
yarn add @cypress/puppeteer | |
yarn add @cypress/puppeteer --dev |
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.
Addressed here
npm/puppeteer/README.md
Outdated
- `onMessage` _required_: An object with string keys and function values (see more details [below](#onmessage)) | ||
- `puppeteer` _optional_: The `puppeteer` library imported from `puppeteer-core`, overriding the default version of `puppeteer-core` used by this plugin | ||
|
||
#### onMessage |
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 to explicitly call out that you can't run cypress commands in the message handlers?
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.
Yeah I like it. Addressed here
|
||
## Compatibility | ||
|
||
`@cypress/puppeteer` requires Cypress version 13.6.0 or greater. |
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 to add a peerDependency
in the package.json
for this requirement?
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.
Good idea. Addressed here
Additional details
Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation
? Updates for puppeteer and multi-tab testing cypress-documentation#5565type definitions
?