diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 5272767f89..1d2b998a51 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). - We now pass a flag to Chromium browsers to disable Chrome translation, both the manual option and the popup prompt, when a page with a differing language is detected. Fixes [#28225](https://github.com/cypress-io/cypress/issues/28225). - Fixed an issue where in chromium based browsers, global style updates can trigger flooding of font face requests in DevTools and Test Replay. This can affect performance due to the flooding of messages in CDP. Fixes [#28150](https://github.com/cypress-io/cypress/issues/28150) and [#28215](https://github.com/cypress-io/cypress/issues/28215). diff --git a/packages/app/src/runner/event-manager.ts b/packages/app/src/runner/event-manager.ts index eb02090ad7..13dca2e60f 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 46e915836a..078d5199ca 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 0000000000..b5042407e3 --- /dev/null +++ b/packages/driver/cypress/fixtures/downloads.html @@ -0,0 +1,15 @@ + + + + + +

Download CSV

+ downloads_records.csv + +

Download CSV

+ download csv from anchor tag without download attr + +

Download CSV

+ broken download link + + diff --git a/packages/driver/cypress/fixtures/downloads_records.csv b/packages/driver/cypress/fixtures/downloads_records.csv new file mode 100644 index 0000000000..54b734d103 --- /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 73e0c66f2d..1d3ec09683 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 c45b9462a4..34d5351e93 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 d33c353cee..2543685589 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 e70d5729f6..6fe74d24dc 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 b0d4617a27..1dd316c891 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 53d4bbf0f9..22b2abfaa1 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 a4764bdac5..8ff81eab95 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -302,11 +302,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', { @@ -528,13 +534,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 52a7b1e33c..fd4f819b4c 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 987afc4c83..20dc11dfe6 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 1fe7fc2d5c..ecb0101d2f 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'