Skip to content
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

Playwright driver #231

Merged
merged 17 commits into from
Jan 2, 2024
Merged

Playwright driver #231

merged 17 commits into from
Jan 2, 2024

Conversation

adiel
Copy link
Member

@adiel adiel commented Dec 9, 2023

Details

  • Adds a driver for Playwright
  • Enables headless mode for PlaywrightDriver and SeleniumWebDriver
  • New dialog handling API to support Playwright, matches Capybara API i.e. AcceptAlert(...) takes a function that should trigger a dialog. Old dialog API deprecated.
  • All Coypu.AcceptanceTests and Coypu.DriverTests passing with Playwright driver in headed & headless mode
  • Headless SeleniumWebDriver also mostly green, has known issues only with cookies and iframes.
  • Unfortunately there was a small breaking change required to the Cookies API as at some point this had become coupled to Selenium. The new API takes and returns System.Net.Cookie rather than the Selenium type.

Speed

The headless Playwright driver + Chromium runs around 5-6 faster than the current default Selenium + Firefox options on my Mac.

Release

I Propose a 4.1.0 release that adds the new driver followed by some testing from anyone who has an existing Coypu test suite and wants to try it out.

Following that I'd propose a 5.0 release that makes Playwright the default driver. Why push anyone to Selenium any more?

New test suites - Coypu vs Playwright

I made some notes in the readme with some thoughts about using this project for new test suites vs just using Playwright.
https://github.com/featurist/coypu/tree/playwright-driver#a-note-on-playwright

@sbowler
Copy link
Collaborator

sbowler commented Dec 10, 2023

I was going to ask, why use Coypu instead of playright since they seem to do a lot of the same stuff, but you've covered that it looks like.
Sounds like a lot of work. I'll have to take a closer look at this later. Interesting that you think it might be 5x faster than using selenium.

Your proposition sounds okay to me. I thought you were wanting to drop support for selenium, which I'm not yet in favor with so that should be good.

@adiel
Copy link
Member Author

adiel commented Dec 10, 2023

Yeah, the only real point of adding this - and it was pretty simple as we were (almost) entirely decoupled from Selenium - is to improve life for existing Coypu test suites. There's likely a fair few legacy projects that might not be under much development but may have years of life left in them that rely on Coypu+Selenium tests. I'd hope switching to this driver will speed them up and make them more reliable and easier to maintain.

I don't have access to any Coypu test suites apart from the internal tests, so if anyone is reading this and wants to pull it and try it then that would be great. Also be good to run the tests on windows if someone wants to do that.

@sbowler
Copy link
Collaborator

sbowler commented Dec 10, 2023

I should be able to do both of those things eventually.

@adiel
Copy link
Member Author

adiel commented Dec 19, 2023

Have a mind to ship this for Christmas, seems pretty low risk as it's just a new driver plus the one Cookie API change that might need a very quick fix in some test suites.

@adiel
Copy link
Member Author

adiel commented Dec 30, 2023

@sbowler you mind releasing this 4.1.0 to nuget if I merge it now? Not sure gonna be much other feedback and I'd rather just fling it out there and see if it helps anyone. Don't really see any risk.

@sbowler
Copy link
Collaborator

sbowler commented Jan 2, 2024

I was hoping to get a bit more testing on this before merging, but between getting sick and Christmas break nothing has really happened. I suppose it shouldn't be too big of a deal to merge. The biggest concern is probably the breaking changes to cookies.

@sbowler sbowler merged commit c18b339 into master Jan 2, 2024
@adiel
Copy link
Member Author

adiel commented Jan 4, 2024

Shall we just make this 5.0.0? That breaking change kinda requires it and the new driver is the most significant change I can imagine even if not to the API.

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