diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index db8114eaff5d..068404e1160e 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -5,6 +5,8 @@ _Released 11/7/2023 (PENDING)_ **Bugfixes:** +- Fixed an issue where clicking a link to download a file could cause a page load timeout when the download attribute was missing. Note: download behaviors in experimental Webkit are still an issue. Fixes [#14857](https://github.com/cypress-io/cypress/issues/14857). +- Fixed an issue to account for canceled and failed downloads to correctly reflect these status in Command log as a download failure where previously it would be pending. Fixed in [#28222](https://github.com/cypress-io/cypress/pull/28222). - Fixed an issue determining visibility when an element is hidden by an ancestor with a shared edge. Fixes [#27514](https://github.com/cypress-io/cypress/issues/27514). - Fixed an issue where network requests made from tabs/windows other than the main Cypress tab would be delayed. Fixes [#28113](https://github.com/cypress-io/cypress/issues/28113). - Stopped processing CDP events at the end of a spec when Test Isolation is off and Test Replay is enabled. Addressed in [#28213](https://github.com/cypress-io/cypress/pull/28213). diff --git a/packages/app/src/runner/event-manager.ts b/packages/app/src/runner/event-manager.ts index eb02090ad72f..13dca2e60f5a 100644 --- a/packages/app/src/runner/event-manager.ts +++ b/packages/app/src/runner/event-manager.ts @@ -148,6 +148,9 @@ export class EventManager { case 'complete:download': Cypress.downloads.end(data) break + case 'canceled:download': + Cypress.downloads.end(data, true) + break default: break } diff --git a/packages/driver/cypress/e2e/cypress/downloads.cy.ts b/packages/driver/cypress/e2e/cypress/downloads.cy.ts index 46e915836a88..078d5199cade 100644 --- a/packages/driver/cypress/e2e/cypress/downloads.cy.ts +++ b/packages/driver/cypress/e2e/cypress/downloads.cy.ts @@ -4,6 +4,7 @@ describe('src/cypress/downloads', () => { let log let snapshot let end + let error let downloads let downloadItem = { id: '1', @@ -11,13 +12,16 @@ describe('src/cypress/downloads', () => { url: 'http://localhost:1234/location.csv', mime: 'text/csv', } + let action beforeEach(() => { end = cy.stub() - snapshot = cy.stub().returns({ end }) + error = cy.stub() + snapshot = cy.stub().returns({ end, error }) log = cy.stub().returns({ snapshot }) + action = cy.stub() - downloads = $Downloads.create({ log }) + downloads = $Downloads.create({ action, log }) }) context('#start', () => { @@ -51,9 +55,21 @@ describe('src/cypress/downloads', () => { downloads.start(downloadItem) downloads.end({ id: '1' }) + expect(action).to.be.calledWith('app:download:received') + expect(snapshot).to.be.called expect(end).to.be.called }) + it('fails with snapshot if matching log exists', () => { + downloads.start(downloadItem) + downloads.end({ id: '1' }, true) + + expect(action).to.be.calledWith('app:download:received') + expect(snapshot).to.be.called + expect(end).not.to.be.called + expect(error).to.be.called + }) + it('is a noop if matching log does not exist', () => { downloads.end({ id: '1' }) @@ -62,3 +78,35 @@ describe('src/cypress/downloads', () => { }) }) }) + +describe('download behavior', () => { + beforeEach(() => { + cy.visit('/fixtures/downloads.html') + }) + + it('downloads from anchor tag with download attribute', () => { + cy.exec(`rm -f ${Cypress.config('downloadsFolder')}/downloads_records.csv`) + cy.readFile(`${Cypress.config('downloadsFolder')}/downloads_records.csv`).should('not.exist') + + // trigger download + cy.get('[data-cy=download-csv]').click() + cy.readFile(`${Cypress.config('downloadsFolder')}/downloads_records.csv`) + .should('contain', '"Joe","Smith"') + }) + + it('downloads from anchor tag without download attribute', { browser: '!webkit' }, () => { + cy.exec(`rm -f ${Cypress.config('downloadsFolder')}/downloads_records.csv`) + cy.readFile(`${Cypress.config('downloadsFolder')}/downloads_records.csv`).should('not.exist') + + // trigger download + cy.get('[data-cy=download-without-download-attr]').click() + cy.readFile(`${Cypress.config('downloadsFolder')}/downloads_records.csv`) + .should('contain', '"Joe","Smith"') + }) + + it('invalid download path from anchor tag with download attribute', () => { + // attempt to download + cy.get('[data-cy=invalid-download]').click() + cy.readFile(`${Cypress.config('downloadsFolder')}/downloads_does_not_exist.csv`).should('not.exist') + }) +}) diff --git a/packages/driver/cypress/fixtures/downloads.html b/packages/driver/cypress/fixtures/downloads.html new file mode 100644 index 000000000000..b5042407e355 --- /dev/null +++ b/packages/driver/cypress/fixtures/downloads.html @@ -0,0 +1,15 @@ +<!DOCTYPE html> +<html> +<head> +</head> +<body> + <h3>Download CSV</h3> + <a data-cy="download-csv" href="downloads_records.csv" download>downloads_records.csv</a> + + <h3>Download CSV</h3> + <a data-cy="download-without-download-attr" href="downloads_records.csv">download csv from anchor tag without download attr</a> + + <h3>Download CSV</h3> + <a data-cy="invalid-download" href="downloads_does_not_exist.csv" download>broken download link</a> +</body> +</html> diff --git a/packages/driver/cypress/fixtures/downloads_records.csv b/packages/driver/cypress/fixtures/downloads_records.csv new file mode 100644 index 000000000000..54b734d103e6 --- /dev/null +++ b/packages/driver/cypress/fixtures/downloads_records.csv @@ -0,0 +1,2 @@ +"First name","Last name","Occupation","Age","City","State" +"Joe","Smith","student",20,"Boston","MA" \ No newline at end of file diff --git a/packages/driver/src/cy/commands/navigation.ts b/packages/driver/src/cy/commands/navigation.ts index 73e0c66f2daf..1d3ec096832c 100644 --- a/packages/driver/src/cy/commands/navigation.ts +++ b/packages/driver/src/cy/commands/navigation.ts @@ -302,6 +302,23 @@ const stabilityChanged = async (Cypress, state, config, stable) => { debug('waiting for window:load') const promise = new Promise((resolve) => { + const handleDownloadUnloadEvent = () => { + cy.state('onPageLoadErr', null) + cy.isStable(true, 'download') + + options._log + .set({ + message: 'download fired beforeUnload event', + consoleProps () { + return { + Note: 'This event fired when the download was initiated.', + } + }, + }).snapshot().end() + + resolve() + } + const onWindowLoad = ({ url }) => { const href = state('autLocation').href const count = getRedirectionCount(href) @@ -351,6 +368,7 @@ const stabilityChanged = async (Cypress, state, config, stable) => { } } + cy.once('download:received', handleDownloadUnloadEvent) cy.once('internal:window:load', onInternalWindowLoad) // If this request is still pending after the test run, resolve it, no commands were waiting on its result. diff --git a/packages/driver/src/cypress.ts b/packages/driver/src/cypress.ts index c45b9462a4ff..34d5351e93de 100644 --- a/packages/driver/src/cypress.ts +++ b/packages/driver/src/cypress.ts @@ -712,6 +712,9 @@ class $Cypress { case 'app:navigation:changed': return this.emit('navigation:changed', ...args) + case 'app:download:received': + return this.emit('download:received') + case 'app:form:submitted': return this.emit('form:submitted', args[0]) diff --git a/packages/driver/src/cypress/downloads.ts b/packages/driver/src/cypress/downloads.ts index d33c353cee8b..25436855890c 100644 --- a/packages/driver/src/cypress/downloads.ts +++ b/packages/driver/src/cypress/downloads.ts @@ -25,11 +25,17 @@ export default { return log.snapshot() } - const end = ({ id }) => { + const end = ({ id }, isCanceled = false) => { + Cypress.action('app:download:received') + const log = logs[id] if (log) { - log.snapshot().end() + if (isCanceled) { + log.snapshot().error(new Error('Download was canceled.')) + } else { + log.snapshot().end() + } // don't need this anymore since the download has ended // and won't change anymore diff --git a/packages/extension/app/v2/background.js b/packages/extension/app/v2/background.js index e70d5729f6e5..6fe74d24dcf0 100644 --- a/packages/extension/app/v2/background.js +++ b/packages/extension/app/v2/background.js @@ -51,11 +51,19 @@ const connect = function (host, path, extraOpts) { }) browser.downloads.onChanged.addListener((downloadDelta) => { - if ((downloadDelta.state || {}).current !== 'complete') return + const state = (downloadDelta.state || {}).current - ws.emit('automation:push:request', 'complete:download', { - id: `${downloadDelta.id}`, - }) + if (state === 'complete') { + ws.emit('automation:push:request', 'complete:download', { + id: `${downloadDelta.id}`, + }) + } + + if (state === 'canceled') { + ws.emit('automation:push:request', 'canceled:download', { + id: `${downloadDelta.id}`, + }) + } }) }) diff --git a/packages/extension/test/integration/background_spec.js b/packages/extension/test/integration/background_spec.js index b0d4617a275b..1dd316c89198 100644 --- a/packages/extension/test/integration/background_spec.js +++ b/packages/extension/test/integration/background_spec.js @@ -231,6 +231,23 @@ describe('app/background', () => { }) }) + it('onChanged emits automation:push:request canceled:download', async function () { + const downloadDelta = { + id: '1', + state: { + current: 'canceled', + }, + } + + sinon.stub(browser.downloads.onChanged, 'addListener').yields(downloadDelta) + + const ws = await this.connect() + + expect(ws.emit).to.be.calledWith('automation:push:request', 'canceled:download', { + id: `${downloadDelta.id}`, + }) + }) + it('onChanged does not emit if state does not exist', async function () { const downloadDelta = { id: '1', diff --git a/packages/server/lib/automation/automation.ts b/packages/server/lib/automation/automation.ts index 53d4bbf0f94f..22b2abfaa15f 100644 --- a/packages/server/lib/automation/automation.ts +++ b/packages/server/lib/automation/automation.ts @@ -127,6 +127,7 @@ export class Automation { case 'change:cookie': return this.cookies.changeCookie(data) case 'create:download': + case 'canceled:download': case 'complete:download': return data default: diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts index ea6b1e20d705..6119341b4b75 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -298,11 +298,17 @@ const _handleDownloads = async function (client, downloadsFolder: string, automa }) client.on('Page.downloadProgress', (data) => { - if (data.state !== 'completed') return + if (data.state === 'completed') { + automation.push('complete:download', { + id: data.guid, + }) + } - automation.push('complete:download', { - id: data.guid, - }) + if (data.state === 'canceled') { + automation.push('canceled:download', { + id: data.guid, + }) + } }) await client.send('Page.setDownloadBehavior', { @@ -521,13 +527,12 @@ export = { await pageCriClient.send('Page.enable') - await utils.handleDownloadLinksViaCDP(pageCriClient, automation) - await options['onInitializeNewBrowserTab']?.() await Promise.all([ options.videoApi && this._recordVideo(cdpAutomation, options.videoApi, Number(options.browser.majorVersion)), this._handleDownloads(pageCriClient, options.downloadsFolder, automation), + utils.handleDownloadLinksViaCDP(pageCriClient, automation), ]) await this._navigateUsingCRI(pageCriClient, url) diff --git a/packages/server/lib/browsers/electron.ts b/packages/server/lib/browsers/electron.ts index 52a7b1e33c73..fd4f819b4c8c 100644 --- a/packages/server/lib/browsers/electron.ts +++ b/packages/server/lib/browsers/electron.ts @@ -337,7 +337,7 @@ export = { }, _handleDownloads (win, dir, automation) { - const onWillDownload = (event, downloadItem) => { + const onWillDownload = (_event, downloadItem) => { const savePath = path.join(dir, downloadItem.getFilename()) automation.push('create:download', { @@ -347,8 +347,14 @@ export = { url: downloadItem.getURL(), }) - downloadItem.once('done', () => { - automation.push('complete:download', { + downloadItem.once('done', (_event, state) => { + if (state === 'completed') { + return automation.push('complete:download', { + id: downloadItem.getETag(), + }) + } + + automation.push('canceled:download', { id: downloadItem.getETag(), }) }) diff --git a/packages/server/test/unit/browsers/chrome_spec.js b/packages/server/test/unit/browsers/chrome_spec.js index 987afc4c8313..20dc11dfe6d7 100644 --- a/packages/server/test/unit/browsers/chrome_spec.js +++ b/packages/server/test/unit/browsers/chrome_spec.js @@ -338,6 +338,21 @@ describe('lib/browsers/chrome', () => { }) }) }) + + it('pushes canceled:download when download is incomplete', function () { + const downloadData = { + guid: '1', + state: 'canceled', + } + const options = { downloadsFolder: 'downloads' } + + return this.onCriEvent('Page.downloadProgress', downloadData, options) + .then(() => { + expect(this.automation.push).to.be.calledWith('canceled:download', { + id: '1', + }) + }) + }) }) describe('adding header to AUT iframe request', function () { diff --git a/packages/server/test/unit/browsers/electron_spec.js b/packages/server/test/unit/browsers/electron_spec.js index 1fe7fc2d5c76..ecb0101d2f16 100644 --- a/packages/server/test/unit/browsers/electron_spec.js +++ b/packages/server/test/unit/browsers/electron_spec.js @@ -322,7 +322,7 @@ describe('lib/browsers/electron', () => { getFilename: () => 'file.csv', getMimeType: () => 'text/csv', getURL: () => 'http://localhost:1234/file.csv', - once: sinon.stub().yields(), + once: sinon.stub().yields({}, 'completed'), } this.win.webContents.session.on.withArgs('will-download').yields({}, downloadItem) @@ -337,6 +337,27 @@ describe('lib/browsers/electron', () => { }) }) + it('pushes canceled:download when download is incomplete', function () { + const downloadItem = { + getETag: () => '1', + getFilename: () => 'file.csv', + getMimeType: () => 'text/csv', + getURL: () => 'http://localhost:1234/file.csv', + once: sinon.stub().yields({}, 'canceled'), + } + + this.win.webContents.session.on.withArgs('will-download').yields({}, downloadItem) + this.options.downloadsFolder = 'downloads' + sinon.stub(this.automation, 'push') + + return electron._launch(this.win, this.url, this.automation, this.options, undefined, undefined, { attachCDPClient: sinon.stub() }) + .then(() => { + expect(this.automation.push).to.be.calledWith('canceled:download', { + id: '1', + }) + }) + }) + it('sets download behavior', function () { this.options.downloadsFolder = 'downloads'