Skip to content

fix(test-runner-chrome): remove focus browser patches #2928

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

Merged
merged 2 commits into from
Apr 10, 2025

Conversation

devversion
Copy link
Contributor

See individual commit descriptions.

The patches were added quite a while ago, and the upstream issues should
be resolved in Chromium. See: https://issues.chromium.org/issues/40272146.

Also see:
puppeteer/puppeteer#10350 (comment).

The patches are currently also resulting some instability of web test
runner. That is because the patch calls an exposed function from inside
the browser, while navigation later on during `stopSession` can happen;
breaking the handle for retrieving function call arguments passed to the
exposed function.

Puppeteer team found this issue and also landed a fix to improve the
failure mode here. See:
puppeteer/puppeteer#13759
Copy link

changeset-bot bot commented Apr 9, 2025

🦋 Changeset detected

Latest commit: 24e3290

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@web/test-runner-chrome Patch
@web/test-runner-core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

This should make debugging easier. It wasn't very easy to figure out
where the errors originated from (because of how the actual uncaught
exception only happened with many concurrent builds inside a sandbox
environment that is hard to debug)
@bashmish
Copy link
Member

Thanks for a clear explainer with all the relevant links in it, great work!

@bashmish bashmish merged commit d8b09d0 into modernweb-dev:master Apr 10, 2025
9 of 14 checks passed
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