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

fix: screenshot() times out when the main Cypress tab is not focused #29038

Merged
merged 18 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
activate main cypress tab before taking a screenshot
  • Loading branch information
cacieprins committed Feb 29, 2024
commit 4c14646408f83f512637c4a6030419efe19dfefd
59 changes: 56 additions & 3 deletions packages/server/lib/browsers/cdp_automation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,8 @@ export class CdpAutomation implements CDPClient {
on: OnFn
off: OffFn
send: SendDebuggerCommand
private frameTree: any
private gettingFrameTree: any
private frameTree: Protocol.Page.FrameTree | undefined
private gettingFrameTree: Promise<void> | undefined | null

private constructor (private sendDebuggerCommandFn: SendDebuggerCommand, private onFn: OnFn, private offFn: OffFn, private sendCloseCommandFn: SendCloseCommand, private automation: Automation) {
onFn('Network.requestWillBeSent', this.onNetworkRequestWillBeSent)
Expand Down Expand Up @@ -205,6 +205,51 @@ export class CdpAutomation implements CDPClient {
return cdpAutomation
}

private async activateMainTab () {
const ActivationTimeoutMessage = 'Unable to communicate with Cypress Extension'

const sendActivationMessage = `
(() => {
if (document.defaultView !== top) { return }
let onMessage
return new Promise((res) => {
onMessage = (ev) => {
if (ev.data.message === 'cypress:extension:main:tab:activated') {
window.removeEventListener('message', onMessage)
res()
}
}

window.addEventListener('message', onMessage)
window.postMessage({ message: 'cypress:extension:activate:main:tab' })
})
})()`

debugVerbose('sending activation message ', sendActivationMessage)

try {
await Promise.race([
this.sendDebuggerCommandFn('Runtime.evaluate', {
expression: sendActivationMessage,
awaitPromise: true,
}),
new Promise((_, reject) => {
setTimeout(() => reject(new Error(ActivationTimeoutMessage)), 2000)
}),
])
} catch (e) {
// If rejected due to timeout, fall back to bringing the main tab to focus -
// this will steal window focus, so it is a last resort. If any other error
// was thrown, re-throw as it was unexpected.
if ((e as Error).message === ActivationTimeoutMessage) {
debugVerbose('Unable to communicate with browser extensioin - stealing focus')
this.sendDebuggerCommandFn('Page.bringToFront')
} else {
throw e
}
}
}

private onNetworkRequestWillBeSent = (params: Protocol.Network.RequestWillBeSentEvent) => {
debugVerbose('received networkRequestWillBeSent %o', params)

Expand Down Expand Up @@ -420,7 +465,7 @@ export class CdpAutomation implements CDPClient {
client.on('Page.frameDetached', this._updateFrameTree(client, 'Page.frameDetached'))
}

onRequest = (message, data) => {
onRequest = async (message, data) => {
let setCookie

switch (message) {
Expand Down Expand Up @@ -494,6 +539,14 @@ export class CdpAutomation implements CDPClient {
case 'remote:debugger:protocol':
return this.sendDebuggerCommandFn(data.command, data.params)
case 'take:screenshot':
debugVerbose('capturing screenshot')
try {
await this.activateMainTab()
} catch (e) {
debugVerbose('Error while attempting to activate main tab: %O', e)
//throw new Error('The extension could not activate the main tab before capturing the screenshot.')
}

return this.sendDebuggerCommandFn('Page.captureScreenshot', { format: 'png' })
.catch((err) => {
throw new Error(`The browser responded with an error when Cypress attempted to take a screenshot.\n\nDetails:\n${err.message}`)
Expand Down
74 changes: 74 additions & 0 deletions system-tests/__snapshots__/issue_5016_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
exports['e2e issue 5016 - screenshot times out after clicking target _blank fails but does not timeout taking screenshot 1'] = `

====================================================================================================

(Run Starting)

┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 1.2.3 │
│ Browser: FooBrowser 88 │
│ Specs: 1 found (issue-5016.cy.js) │
│ Searched: cypress/e2e/**/*.cy.{js,jsx,ts,tsx} │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


────────────────────────────────────────────────────────────────────────────────────────────────────

Running: issue-5016.cy.js (1 of 1)


issue 5016
✓ should take a normal screenshot
1) should fail but not timeout while taking the screenshot
✓ should not timeout taking screenshot when not failing


2 passing
1 failing

1) issue 5016
should fail but not timeout while taking the screenshot:
AssertionError: Timed out retrying after 4050ms: expected '<a>' to have attribute 'foo'
[stack trace lines]




(Results)

┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 3 │
│ Passing: 2 │
│ Failing: 1 │
│ Pending: 0 │
│ Skipped: 0 │
│ Screenshots: 3 │
│ Video: false │
│ Duration: X seconds │
│ Spec Ran: issue-5016.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘


(Screenshots)

- /XXX/XXX/XXX/cypress/screenshots/issue-5016.cy.js/issue 5016 -- should take a no (YxX)
rmal screenshot.png
- /XXX/XXX/XXX/cypress/screenshots/issue-5016.cy.js/issue 5016 -- should fail but (YxX)
not timeout while taking the screenshot (failed).png
- /XXX/XXX/XXX/cypress/screenshots/issue-5016.cy.js/issue 5016 -- should not timeo (YxX)
ut taking screenshot when not failing.png


====================================================================================================

(Run Finished)


Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✖ issue-5016.cy.js XX:XX 3 2 1 - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✖ 1 of 1 failed (100%) XX:XX 3 2 1 - -


`
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
screenshotOnRunFailure: true,
e2e: {
supportFile: false,
},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// https://github.com/cypress-io/cypress/issues/5016
describe('issue 5016', {
screenshotOnRunFailure: true,
}, function () {
it('should take a normal screenshot', function () {
cy.visit('/cypress/fixtures/issue-5016/index.html').screenshot()
})

it('should fail but not timeout while taking the screenshot', function () {
cy.visit('/cypress/fixtures/issue-5016/index.html')
cy.get('a').click().should('have.attr', 'foo')
})

it('should not timeout taking screenshot when not failing', function () {
cy.visit('/cypress/fixtures/issue-5016/index.html')
cy.get('a').click().screenshot()
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<!doctype html>

<body>
<a target="_blank" href="./new.html">New Tab</a>
</body>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<!doctype html>

<html></html>
15 changes: 15 additions & 0 deletions system-tests/test/issue_5016_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const systemTests = require('../lib/system-tests').default

describe('e2e issue 5016 - screenshot times out after clicking target _blank', function () {
systemTests.setup()

it('fails but does not timeout taking screenshot', function () {
return systemTests.exec(this, {
project: 'config-screenshot-on-failure-enabled',
sanitizeScreenshotDimensions: true,
snapshot: true,
browser: 'chrome',
expectedExitCode: 1,
})
})
})