misc: Close extra tabs/windows between tests #28204
Conversation
Passing run #52334 ↗︎
Details:
Review all test suite changes for PR #28204 ↗︎ |
|||||||||||||||
chrisbreiding
force-pushed
the
crb/cy-puppeteer-filter-extra-target-traffic
branch
from
681023a
to
e9a94ff
Compare
chrisbreiding
force-pushed
the
crb/cy-puppeteer-close-extra-targets
branch
from
c911035
to
4bfd3c0
Compare
| @@ -573,6 +573,18 @@ export class BrowserCriClient { | |||
| this.extraTargetClients.delete(targetId) | |||
| } | |||
|
|
|||
| async closeExtraTargets () { | |||
| for (const [targetId] of this.extraTargetClients) { | |||
There was a problem hiding this comment.
I think this for loop with the awaited promise will be blocking -- would it be better to map these and use Promise.all to ensure each completes if we do need to wait on these?
There was a problem hiding this comment.
It's intended to be blocking and will await all of them as-is. Using Promise.all would run them in parallel, but I'm not sure that's advisable with CDP. I think it's a safer bet to run them serially.
There was a problem hiding this comment.
Why wouldn't that be advisable with CDP? I think using Promise.all makes sense here as well.
There was a problem hiding this comment.
I thought there was a case where CDP had trouble with too many commands being run at once, but I can't find that instance. I updated it to run the target closing in parallel.
| * Closes extra targets that are not the Cypress tab | ||
| */ | ||
| async closeExtraTargets () { | ||
| if (!instance || !instance.browser) return |
There was a problem hiding this comment.
| if (!instance || !instance.browser) return | |
| if (!instance?.browser) return |
There was a problem hiding this comment.
Addressed in 62e08a3
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
Closes any extra targets (tabs/windows) between tests so that new tests always start with only the main Cypress tab, in order to maintain test isolation and a clean slate before each test. This is in anticipation of support for testing multiple tabs/windows with the future
@cypress/puppeteerplugin.Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?