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

PATCH: Feature/1525 headless mode deprecation #1535

Merged
merged 6 commits into from
Dec 24, 2023

Conversation

dgrebb
Copy link
Contributor

@dgrebb dgrebb commented Dec 24, 2023

Summary

Puppeteer defaults to "new" headless mode, as per the feature request in #1525, also discussed in #1485.

Playwright, on the other hand, will remain headless: true by default, as "new" mode is not supported without some configuration overhead. And speaking of Playwright...

Important

#1534 is fixed, which I found while exploring the above-mentioned caveats of "new" mode.
This should be considered for an immediate patch

In the end, we support "new" headless mode in Playwright, but only when set by a Backstop configuration file.

I also addressed a fix for GitHub Actions npm run integration-test error: Error: Process completed with exit code 254..

Any and all input welcome! We want to be safe, but at the same time, want to support the cutting-edge and standards for Playwright, Puppeteer, and the various browser vendors and teams.

Details

Puppeteer

  • Puppeteer now defaults to "new" headless mode
    • tested docker
    • tested CLI
    • ran through GH actions

Playwright

From what I've gathered, Playwright is not planning to support "new" mode at this point in time. There are performance considerations for their users, so they make it more challenging to use the new mode.

Once Playwright supports "new" mode, we should discuss enforcing this default for Playwright, but until then, this is a user configuration.

Problem

Playwright expects a boolean for headless mode.

Solution

Override Playwrights default args.

We evaluate whether a user is trying to pass headless as a string, and include ignoreDefaultArgs: ["--headless"], which allows a string to be passed.

We set the property headless: true if "new" is passed, and include --headless=new in the final playwrightArgs sent to the runner.

{
 args: [ '--no-sandbox', '--headless=new' ],
 headless: true,
 ignoreDefaultArgs: [ '--headless' ]
}

Notes

Example Playwright headless: 'new' error

Creating Browser
No Playwright browser specified, assuming Chromium.
COMMAND | Command "test" ended with an error after [0.004s]
COMMAND | browserType.launch: headless: expected boolean, got string

Extra

I started adding inline documentation, which are a nice addition to overall developer experience. The workarounds for Playwright "new" headless mode are commented on for future reference.

Pasted image 20231224000613

@dgrebb
Copy link
Contributor Author

dgrebb commented Dec 24, 2023

@garris this addresses new headless mode. We can default Playwright to use new if that is the goal, but because it's a little hacky, I didn't do so (yet).

Please let me know if you'd like an npm script and GitHub Action test for this as well. I think it's worth it but didn't want to overload the PR more than it already may be.

@garris
Copy link
Owner

garris commented Dec 24, 2023

@dgrebb Thanks Dan for doing the extra research on this. It sounds like you are setting defaults that are in-line with the philosophies of the different frameworks -- which makes sense.

I don't think this new/old/true/false oddness needs to be elevated to a GH test -- I'd prefer to keep GH testing space for high-level critical-path stuff e.g. sanity, smoke, integration, basic UI backstop testing with your cool fixtures idea.

Thank you!

@garris garris merged commit 885a68d into garris:master Dec 24, 2023
4 checks passed
@dgrebb dgrebb deleted the feature/1525-headless-mode-deprecation branch December 24, 2023 18:06
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