cypress

cypress copied to clipboard

• For #6422

User facing changelog

n/a - does not expose WebKit in production yet, only in development

Additional details

• This PR adds development-only support for the WebKit browser. It will detect playwright-webkit installed in a project and if it is found, it will be made available in Cypress as --browser webkit
• See https://github.com/cypress-io/cypress/issues/6422 for remaining work - trying to keep PRs in this epic small since there's a lot to parse

Pre-merge tasks:

• [x] Merge with latest develop
• [x] Add driver test CI job
• [x] Double-check that --headless works w/o Xvfb

How has the user experience changed?

In development only, WebKit will be detected in run/open mode if playwright-webkit is installed.

PR Tasks

• [x] Have tests been added/updated?
  • Only tests relating to browser detection have been added/updated. Future test updates will come as remaining functionality is fixed, see #6422 for details.
• [na] Has the original issue or this PR been tagged with a release in ZenHub?
• [na] Has a PR for user-facing changes been opened in cypress-documentation?
• [na] Have API changes been updated in the type definitions?
  • Public type changes will come in the last PR for experimentalWebKitSupport, see #6422

Co-authored-by: Weyert de Boer [email protected]

Thanks for taking the time to open a PR!

• Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
• Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

Any updates on this?

---

Test summary

37828 • 0 • 615 • 0 • 7

---

Run details

Project	cypress
Status	Passed
Commit	ac70fc8ad5
Started	Aug 15, 2022 10:39 PM
Ended	Aug 15, 2022 11:01 PM
Duration	21:41 💡
OS	Linux Debian - 11.3
Browser	Multiple

View run in Cypress Dashboard ➡️

---

Flakiness

	cypress/e2e/commands/net_stubbing.cy.ts		5
1	network stubbing > intercepting request > can delay with deprecated delayMs param
2	network stubbing > intercepting request > can delay with deprecated delayMs param
3	network stubbing > intercepting request > can delay with deprecated delayMs param
4	network stubbing > intercepting request > can delay with deprecated delayMs param
5	network stubbing > intercepting request > can delay with deprecated delayMs param
This comment includes only the first 5 flaky tests. See all 7 flaky tests in the Cypress Dashboard.

---

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

Still going over the code, but I took it for a test drive. It functions like it should, but I am noticing everything in the webkit driver is kind of blurry.

Compare the text from GH (left) and the spec list (right). I am guessing this isn't on our end, but the webkit-playwright code? Either way, users might notice this - we might want to include a note in the experimental release (unless the bug is localised to my machine?)

I am on an M1 running under arm64 if that is relevant.

I will continue the code review tomorrow. I am excited for WebKit support!

Image: text in webkit browser is a bit blurry (compare to address bar or GH text in background).

@lmiller1990 I can confirm I see the same blurry rendering on my Macbook display, but it looks crisp on my external display (with a much lower pixel density).

@lmiller1990 @tbiethman We check and set 2x pixel density on other browsers but not in WebKit. I can add it to the list of things to fix in the parent epic, but in the spirit of keeping this PR minimal let's not worry about it here. It seems like we can set devicePixelRatio, but not in browserType.launch, so it requires some further investigation as to how to do this with WebKit: https://playwright.dev/docs/api/class-browsertype#browser-type-launch-persistent-context

Code looks good in this initial state. Here are a couple functional findings that don't necessarily need to hold up this initial PR, but I'll let you be the judge:

macOS

• Focus button in launchpad does not focus Webkit browser.

• I'm seeing GraphQL errors that are only present within the Webkit browser. Both e2e/component testing projects seem to be effected, but it can take longer for the error to be presented in component tests.

• Using the Tab key only seems to focus certain elements within the Webkit browser. I imagine this is related to the Safari setting to "Press Tab to highlight each item on a webpage", which must be enabled to behave similar to Chrome/Firefox. Not sure if there's a corresponding launch flag (or if this has any impact on our .focus() command)?

Windows

• Seeing various failures in both e2e and component tests in our internal projects. I'm sure there are incompatibilities we'll want to document, not sure if these are known about or not:

@tbiethman Thanks for the detailed feedback!

* `Focus` button in launchpad does not focus Webkit browser.

Marked it as "less critical" here: https://github.com/cypress-io/cypress/issues/6422 I'm using page.bringToFront to focus but it seems to not behave the same as Chrome.

* I'm seeing GraphQL errors that are only present within the Webkit browser. Both e2e/component testing projects seem to be effected, but it can take longer for the error to be presented in component tests.

Added as a must-fix to #6422

* Using the Tab key only seems to focus certain elements within the Webkit browser. I imagine this is related to the Safari setting to "Press Tab to highlight each item on a webpage", which must be enabled to behave similar to Chrome/Firefox. Not sure if there's a corresponding launch flag (or if this has any impact on our `.focus()` command)?

I have noticed some failures in the driver tests related to focus, thanks for the tip on this. I'll keep this in mind when looking at those.

* Seeing various failures in both e2e and component tests in our internal projects. I'm sure there are incompatibilities we'll want to document, not sure if these are known about or not:

Not known to me yet - I've been working on fixing the driver tests first before checking out other projects, since that should resolve most common issues. I plan on also adding certain system-tests. Limitations that come from those will be documented.

I found something strange. I don't think it needs to block this, but if I start run mode, then cancel it mid-run, global mode opens after a few seconds.

It's also kind of slow (I guess this is just how it is, at least for now). Since this is development only, I think it's fine to merge up, I'll keep posting things I notice as I continue to play around with it (which will be easier once it's merged).

https://user-images.githubusercontent.com/19196536/184058318-e7e6cf1a-ff86-415b-b81f-368464cad4ff.mov

@lmiller1990 I've noticed that too. Haven't been able to track it down yet, but it's super annoying. I have it captured as a must-fix here: https://github.com/cypress-io/cypress/issues/6422

Any plans to mention my name when release this feature?

@weyert - Traditionally we'd do something like this: https://docs.cypress.io/api/commands/selectFile#Community-Recognition. @flotwig - I don't see that we have a docs PR for this, just a reasonable reminder to make sure we mention community contributions when it's ready!

Any plans to mention my name when release this feature?

@weyert I was planning on it, since this does use your initial approach of driving PW. I actually meant to have you as a Co-authored-by on this PR but I guess I managed to merge without including the PR message in the commit message - oops. I'll credit you in the release announcement and docs, unless you don't want to be recognized.

I would greatly appreciate that, if you would do that. Thank you 🤗❤️