playwright-driver: use upstream chromium build#383876
Conversation
e607b4a to
6eeea0e
Compare
|
|
I don't have a strong opinion on this topic but maybe we should just use chromium from playwright bundle as we do for the rest of the browsers (and now for chromium headless shell as well). We can also provide some utility to build custom set of browsers, eg. |
6eeea0e to
2a9c428
Compare
2a9c428 to
bf27800
Compare
|
There was a problem hiding this comment.
In favor of this! Chromium was the only browser we supported for playwright for quite a while, until #298944 was merged.
@kalekseev's suggestion of playwright-browsers.withBrowsers also makes sense to me, as the current mechanism could use a cleanup after so many iterations, but that doesn't need to happen in this PR IMO.
bf27800 to
d5625d7
Compare
Use the exact chromium version upstream uses for playwright, to prevent breaking from browser updates in nixpkgs.
d5625d7 to
9424a47
Compare
phaer
left a comment
There was a problem hiding this comment.
LGTM!
But such a change is probably "breaking" in a sense, so maybe lets wait until the 25.05 branch off with a merge?
Browser updates also get backported, so this would leave 25.05 users still with a potentially broken playwright + chromium combo. I would prefer to still get this into 25.05 |
|
I am a pretty new committer, so don't have much experience with that. But I think the process to get this into 25.05 is do to a backport right after? |
|
Only if the branch-off already happened, which didn't happen yet: #390768 |
|
Successfully created backport PR for |
|
FYI I believe the hash for x86_64-linux is wrong: {
inputs.nixpkgs.url = "github:nixos/nixpkgs?ref=nixos-unstable";
outputs = { self, nixpkgs }: {
packages.x86_64-linux.default = nixpkgs.legacyPackages.x86_64-linux.playwright-test;
};
}Edit: Whoops already fixed on master: #409116 |
This makes playwright always use the upstream recommended chromium version.
I think this is necessary, because chromium updates get backported, which can (and will) break playwright versions on stable, as well as chromium always moving a bit ahead of playwright and we can't expect to delay a browser update for playwright.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.