chore: add after:browser:launch node event #28180
Conversation
5 flaky tests on run #52169 ↗︎
Details:
|
|||||||||||||||||||||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| src/cy/commands/storage > #getAllLocalStorage > gets local storage from all origins |
Test Replay
Output
|
|
e2e/origin/commands/storage.cy.ts • 1 flaky test • 5x-driver-chrome
| Test | Artifacts | |
|---|---|---|
| cy.origin storage > .getAllLocalStorage |
Test Replay
Output
|
|
cypress/cypress.cy.js • 3 flaky tests • 5x-driver-chrome
| Test | Artifacts | |
|---|---|---|
| ... > correctly returns currentRetry |
Test Replay
Output
|
|
| ... > correctly returns currentRetry |
Test Replay
Output
|
|
| ... > correctly returns currentRetry |
Test Replay
Output
|
|
Review all test suite changes for PR #28180 ↗︎
…press into crb/cy-puppeteer-refactors
| @@ -126,7 +126,7 @@ describe('lib/browsers/firefox', () => { | |||
|
|
|||
| context('#open', () => { | |||
| beforeEach(function () { | |||
| this.browser = { name: 'firefox', channel: 'stable' } | |||
| this.browser = { name: 'firefox', channel: 'stable', majorVersion: 100 } | |||
There was a problem hiding this comment.
Why is majorVersion needed now?
There was a problem hiding this comment.
The tests I added need hasCdp to be true since the after:browser:launch event needs the remote debugging port.
There was a problem hiding this comment.
can we add an in-line comment about this version adding hasCDP? feels like something we'll ask about in the future and forget why it's here
or maybe there is a better place to add this info?
There was a problem hiding this comment.
Added a comment in 2d862af
| } | ||
|
|
||
| async function executeAfterBrowserLaunch (browser: Browser, options: AfterBrowserLaunchDetails) { | ||
| if (plugins.has('after:browser:launch')) { |
There was a problem hiding this comment.
iirc, with chrome if we crash, we will re-launch the browser. does this re-trigger if that happens?
There was a problem hiding this comment.
Do we re-launch the browser? From what I can tell, we detect the crash and report it, which seems like the appropriate behavior.
| @@ -147,5 +147,11 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc | |||
| } | |||
| } | |||
|
|
|||
| await utils.executeAfterBrowserLaunch(browser, { | |||
| get webSocketDebuggerUrl (): never { | |||
| throw new Error('The `webSocketDebuggerUrl` property is not currently supported in the `after:browser:launch` event handler details argument') | |||
There was a problem hiding this comment.
Why would this throw in Webkit vs logging an error and continuing? Wouldn't this fail for webkit testing?
There was a problem hiding this comment.
Because any code relying on getting the websocket url would fail with a less useful message later. This makes it clear to the user why their test is failing in webkit.
There was a problem hiding this comment.
In writing a test for this, I realized it's pretty trivial to provide the websocket url instead of throwing an error, so it's now supported in webkit.
…press-io/cypress into crb/cy-puppeteer-after-browser-launch
| @@ -8,7 +8,7 @@ const minimist = require('minimist') | |||
| const argsUtil = require(`../../../lib/util/args`) | |||
| const getWindowsProxyUtil = require(`../../../lib/util/get-windows-proxy`) | |||
|
|
|||
| const cwd = process.cwd() | |||
| const getCwd = () => process.cwd() | |||
There was a problem hiding this comment.
Using proxyquire + the dynamic require inside webkit.ts exacerbates some test pollution where process.cwd() returns a different value between all the files being loaded and the tests being executed. Getting process.cwd() at the time of the test ensures we're comparing the same values at test time.
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
Adds the
after:browser:launchnode event. It's a soft release of this "feature" for the purpose of usage within the puppeteer plugin, so we're intentionally not documenting it or mentioning it in the changelog. It's not currently intended for public use and is somewhat experimental.Steps to test
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?