From 1efd90d4031173af9a9da6f26e25375ccd532cec Mon Sep 17 00:00:00 2001 From: Blue F Date: Wed, 1 Feb 2023 12:59:56 -0800 Subject: [PATCH] breaking: upgrade cy.readFile() to be a query command (#25595) * Update cli/CHANGELOG.md Co-authored-by: Emily Rohrbough * Update cli/CHANGELOG.md Co-authored-by: Matt Henkes --------- Co-authored-by: Emily Rohrbough Co-authored-by: Matt Henkes --- cli/CHANGELOG.md | 10 +- .../cypress/e2e/commands/assertions.cy.js | 11 +- .../driver/cypress/e2e/commands/files.cy.js | 32 ++- .../e2e/e2e/origin/commands/files.cy.ts | 8 +- packages/driver/src/cy/commands/files.ts | 196 ++++++++++-------- .../src/cy/commands/querying/querying.ts | 3 +- packages/driver/src/cypress/error_messages.ts | 6 + packages/driver/src/cypress/log.ts | 6 +- 8 files changed, 149 insertions(+), 123 deletions(-) diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 15c3204b027f..7540d0430078 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,4 +1,12 @@ - + +## 13.0.0 + +_Released 01/31/2023 (PENDING)_ + +**Breaking Changes:** + +- The [`cy.readFile()`](/api/commands/readfile) command is now retry-able as a [query command](https://on.cypress.io/retry-ability). This should not affect any tests using it; the functionality is unchanged. However, it can no longer be overwritten using [`Cypress.Commands.overwrite()`](/api/cypress-api/custom-commands#Overwrite-Existing-Commands). Addressed in [#25595](https://github.com/cypress-io/cypress/pull/25595). + ## 12.5.1 _Released 02/10/2023 (PENDING)_ diff --git a/packages/driver/cypress/e2e/commands/assertions.cy.js b/packages/driver/cypress/e2e/commands/assertions.cy.js index 03c5db7caf3a..8f5561428269 100644 --- a/packages/driver/cypress/e2e/commands/assertions.cy.js +++ b/packages/driver/cypress/e2e/commands/assertions.cy.js @@ -652,9 +652,14 @@ describe('src/cy/commands/assertions', () => { cy.get('button:first', { timeout: 500 }).should('have.class', 'does-not-have-class') }) - it('has a pending state while retrying for commands with onFail', (done) => { + it('has a pending state while retrying for commands with onFail', function (done) { cy.on('command:retry', () => { - const [readFileLog, shouldLog] = cy.state('current').get('logs') + // Wait for the readFile response to come back from the server + if (this.logs.length < 2) { + return + } + + const [readFileLog, shouldLog] = this.logs expect(readFileLog.get('state')).to.eq('pending') expect(shouldLog.get('state')).to.eq('pending') @@ -662,8 +667,6 @@ describe('src/cy/commands/assertions', () => { done() }) - cy.on('fail', () => {}) - cy.readFile('does-not-exist.json', { timeout: 500 }).should('exist') }) diff --git a/packages/driver/cypress/e2e/commands/files.cy.js b/packages/driver/cypress/e2e/commands/files.cy.js index 8648ee4bae28..a317cc980e23 100644 --- a/packages/driver/cypress/e2e/commands/files.cy.js +++ b/packages/driver/cypress/e2e/commands/files.cy.js @@ -14,6 +14,14 @@ describe('src/cy/commands/files', () => { }) describe('#readFile', () => { + it('really works', () => { + cy.readFile('./cypress/fixtures/fileSpec.json').its('baseUrl').should('eq', 'http://localhost:3500') + }) + + it('works when contents are supposed to be null', () => { + cy.readFile('does-not-exist').should('be.null') + }) + it('triggers \'read:file\' with the right options', () => { Cypress.backend.resolves(okResponse) @@ -80,7 +88,7 @@ describe('src/cy/commands/files', () => { .resolves(okResponse) cy.readFile('foo.json').then(() => { - expect(retries).to.eq(1) + expect(retries).to.eq(2) }) }) @@ -102,18 +110,12 @@ describe('src/cy/commands/files', () => { }) cy.readFile('foo.json').should('eq', 'quux').then(() => { - expect(retries).to.eq(1) + // Two retries: The first one triggers a backend request and throws a 'not ready' error. + // The second gets foobarbaz, triggering another request to the backend. + expect(retries).to.eq(2) }) }) - it('really works', () => { - cy.readFile('./cypress/fixtures/fileSpec.json').its('baseUrl').should('eq', 'http://localhost:3500') - }) - - it('works when contents are supposed to be null', () => { - cy.readFile('does-not-exist').should('be.null') - }) - describe('.log', () => { beforeEach(function () { this.logs = [] @@ -176,10 +178,6 @@ describe('src/cy/commands/files', () => { this.logs = [] - cy.on('fail', () => { - cy.off('log:added', collectLogs) - }) - return null }) @@ -243,7 +241,7 @@ describe('src/cy/commands/files', () => { cy.on('fail', (err) => { const { fileLog } = this - assertLogLength(this.logs, 2) + assertLogLength(this.logs, 3) expect(fileLog.get('error')).to.eq(err) expect(fileLog.get('state')).to.eq('failed') expect(err.message).to.eq(stripIndent`\ @@ -388,7 +386,7 @@ describe('src/cy/commands/files', () => { expect(fileLog.get('error')).to.eq(err) expect(fileLog.get('state')).to.eq('failed') expect(err.message).to.eq(stripIndent`\ - \`cy.readFile("foo")\` timed out after waiting \`10ms\`. + Timed out retrying after 10ms: \`cy.readFile("foo")\` timed out. `) expect(err.docsUrl).to.eq('https://on.cypress.io/readfile') @@ -412,7 +410,7 @@ describe('src/cy/commands/files', () => { expect(fileLog.get('error')).to.eq(err) expect(fileLog.get('state')).to.eq('failed') expect(err.message).to.eq(stripIndent`\ - \`cy.readFile("foo")\` timed out after waiting \`42ms\`. + Timed out retrying after 42ms: \`cy.readFile("foo")\` timed out. `) expect(err.docsUrl).to.eq('https://on.cypress.io/readfile') diff --git a/packages/driver/cypress/e2e/e2e/origin/commands/files.cy.ts b/packages/driver/cypress/e2e/e2e/origin/commands/files.cy.ts index d708b9b9fc94..346e3d99dd54 100644 --- a/packages/driver/cypress/e2e/e2e/origin/commands/files.cy.ts +++ b/packages/driver/cypress/e2e/e2e/origin/commands/files.cy.ts @@ -64,11 +64,11 @@ context('cy.origin files', { browser: '!webkit' }, () => { }) cy.shouldWithTimeout(() => { - const { consoleProps } = findCrossOriginLogs('readFile', logs, 'foobar.com') + const log = findCrossOriginLogs('readFile', logs, 'foobar.com') - expect(consoleProps.Command).to.equal('readFile') - expect(consoleProps['File Path']).to.include('cypress/fixtures/example.json') - expect(consoleProps.Contents).to.deep.equal({ example: true }) + expect(log.consoleProps.Command).to.equal('readFile') + expect(log.consoleProps['File Path']).to.include('cypress/fixtures/example.json') + expect(log.consoleProps.Contents).to.deep.equal({ example: true }) }) }) diff --git a/packages/driver/src/cy/commands/files.ts b/packages/driver/src/cy/commands/files.ts index 02512836a209..e1d63a90707f 100644 --- a/packages/driver/src/cy/commands/files.ts +++ b/packages/driver/src/cy/commands/files.ts @@ -4,9 +4,8 @@ import { basename } from 'path' import $errUtils from '../../cypress/error_utils' import type { Log } from '../../cypress/log' -interface InternalReadFileOptions extends Partial { - _log?: Log - encoding: Cypress.Encodings +interface ReadFileOptions extends Partial { + encoding?: Cypress.Encodings } interface InternalWriteFileOptions extends Partial { @@ -14,108 +13,123 @@ interface InternalWriteFileOptions extends Partial { - Commands.addAll({ - readFile (file, encoding, userOptions: Partial = {}) { - if (_.isObject(encoding)) { - userOptions = encoding - encoding = undefined + Commands.addQuery('readFile', function readFile (file, encoding, options: ReadFileOptions = {}) { + if (_.isObject(encoding)) { + options = encoding + encoding = options.encoding + } + + encoding = encoding === undefined ? 'utf8' : encoding + + const timeout = options.timeout ?? Cypress.config('defaultCommandTimeout') + + this.set('timeout', timeout) + this.set('ensureExistenceFor', 'subject') + + const log = options.log !== false && Cypress.log({ message: file, timeout }) + + if (!file || !_.isString(file)) { + $errUtils.throwErrByPath('files.invalid_argument', { + args: { cmd: 'readFile', file }, + }) + } + + let fileResult: any = null + let filePromise: Promise | null = null + let mostRecentError = $errUtils.cypressErrByPath('files.read_timed_out', { + args: { file }, + }) + + const createFilePromise = () => { + // If we already have a pending request to the backend, we'll wait + // for that one to resolve instead of creating a new one. + if (filePromise) { + return } - const options: InternalReadFileOptions = _.defaults({}, userOptions, { + fileResult = null + filePromise = Cypress.backend('read:file', file, { encoding }) + .timeout(timeout) + .then((result) => { // https://github.com/cypress-io/cypress/issues/1558 - // If no encoding is specified, then Cypress has historically defaulted - // to `utf8`, because of it's focus on text files. This is in contrast to - // NodeJs, which defaults to binary. We allow users to pass in `null` - // to restore the default node behavior. - encoding: encoding === undefined ? 'utf8' : encoding, - log: true, - timeout: Cypress.config('defaultCommandTimeout'), + // https://github.com/cypress-io/cypress/issues/20683 + // We invoke Buffer.from() in order to transform this from an ArrayBuffer - + // which socket.io uses to transfer the file over the websocket - into a `Buffer`. + if (encoding === null && result.contents !== null) { + result.contents = Buffer.from(result.contents) + } + + // Add the filename to the current command, in case we need it later (such as when storing an alias) + state('current').set('fileName', basename(result.filePath)) + + fileResult = result }) + .catch((err) => { + if (err.name === 'TimeoutError') { + $errUtils.throwErrByPath('files.timed_out', { + args: { cmd: 'readFile', file, timeout }, + retry: false, + }) + } - const consoleProps = {} + // Non-ENOENT errors are not retried + if (err.code !== 'ENOENT') { + $errUtils.throwErrByPath('files.unexpected_error', { + args: { cmd: 'readFile', action: 'read', file, filePath: err.filePath, error: err.message }, + errProps: { retry: false }, + }) + } - if (options.log) { - options._log = Cypress.log({ - message: file, - timeout: options.timeout, - consoleProps () { - return consoleProps - }, + // We have a ENOENT error - the file doesn't exist. Whether this is an error or not is deterimened + // by verifyUpcomingAssertions, when the command_queue receives the null file contents. + fileResult = { contents: null, filePath: err.filePath } + }) + .catch((err) => mostRecentError = err) + // Pass or fail, we always clear the filePromise, so future retries know there's no + // live request to the server. + .finally(() => filePromise = null) + } + + // When an assertion attached to this command fails, then we want to throw away the existing result + // and create a new promise to read a new one. + this.set('onFail', (err, timedOut) => { + if (err.type === 'existence') { + // file exists but it shouldn't - or - file doesn't exist but it should + const errPath = fileResult.contents ? 'files.existent' : 'files.nonexistent' + const { message, docsUrl } = $errUtils.cypressErrByPath(errPath, { + args: { cmd: 'readFile', file, filePath: fileResult.filePath }, }) - } - if (!file || !_.isString(file)) { - $errUtils.throwErrByPath('files.invalid_argument', { - onFail: options._log, - args: { cmd: 'readFile', file }, - }) + err.message = message + err.docsUrl = docsUrl } - // We clear the default timeout so we can handle - // the timeout ourselves - cy.clearTimeout() + createFilePromise() + }) - const verifyAssertions = () => { - return Cypress.backend('read:file', file, _.pick(options, 'encoding')).timeout(options.timeout) - .catch((err) => { - if (err.name === 'TimeoutError') { - return $errUtils.throwErrByPath('files.timed_out', { - onFail: options._log, - args: { cmd: 'readFile', file, timeout: options.timeout }, - }) - } - - // Non-ENOENT errors are not retried - if (err.code !== 'ENOENT') { - return $errUtils.throwErrByPath('files.unexpected_error', { - onFail: options._log, - args: { cmd: 'readFile', action: 'read', file, filePath: err.filePath, error: err.message }, - }) - } - - return { - contents: null, - filePath: err.filePath, - } - }).then(({ filePath, contents }) => { - // https://github.com/cypress-io/cypress/issues/1558 - // https://github.com/cypress-io/cypress/issues/20683 - // We invoke Buffer.from() in order to transform this from an ArrayBuffer - - // which socket.io uses to transfer the file over the websocket - into a `Buffer`. - if (options.encoding === null && contents !== null) { - contents = Buffer.from(contents) - } - - // Add the filename as a symbol, in case we need it later (such as when storing an alias) - state('current').set('fileName', basename(filePath)) - - consoleProps['File Path'] = filePath - consoleProps['Contents'] = contents - - return cy.verifyUpcomingAssertions(contents, options, { - ensureExistenceFor: 'subject', - onFail (err) { - if (err.type !== 'existence') { - return - } - - // file exists but it shouldn't - or - file doesn't exist but it should - const errPath = contents ? 'files.existent' : 'files.nonexistent' - const { message, docsUrl } = $errUtils.cypressErrByPath(errPath, { - args: { cmd: 'readFile', file, filePath }, - }) - - err.message = message - err.docsUrl = docsUrl - }, - onRetry: verifyAssertions, - }) - }) + return () => { + // Once we've read a file, that remains the result, unless it's cleared + // because of a failed assertion in `onFail` above. + if (fileResult) { + const props = { + 'Contents': fileResult?.contents, + 'File Path': fileResult?.filePath, + } + + log && state('current') === this && log.set('consoleProps', () => props) + + return fileResult.contents } - return verifyAssertions() - }, + createFilePromise() + // If we don't have a result, then the promise is pending. + // Throw an error and wait for the promise to eventually resolve on a future retry. + throw mostRecentError + } + }) + + Commands.addAll({ writeFile (fileName, contents, encoding, userOptions: Partial = {}) { if (_.isObject(encoding)) { userOptions = encoding diff --git a/packages/driver/src/cy/commands/querying/querying.ts b/packages/driver/src/cy/commands/querying/querying.ts index 53da09ff071b..69df1075d7e9 100644 --- a/packages/driver/src/cy/commands/querying/querying.ts +++ b/packages/driver/src/cy/commands/querying/querying.ts @@ -66,8 +66,7 @@ function getAlias (selector, log, cy) { */ if (command.get('name') === 'intercept') { - // Intercept aliases are fairly similar, but `getAliasedRequests` does *not* handle indexes - // and we have to do it ourselves here. + // `getAliasedRequests` does *not* handle indexes and we have to do it ourselves here. const requests = getAliasedRequests(aliasObj.alias, cy.state) diff --git a/packages/driver/src/cypress/error_messages.ts b/packages/driver/src/cypress/error_messages.ts index f89958dd2d8a..be8346aa1c42 100644 --- a/packages/driver/src/cypress/error_messages.ts +++ b/packages/driver/src/cypress/error_messages.ts @@ -520,6 +520,12 @@ export default { docsUrl: `https://on.cypress.io/${_.toLower(obj.cmd)}`, } }, + read_timed_out (obj) { + return { + message: `${cmd('readFile', '"{{file}}"')} timed out.`, + docsUrl: `https://on.cypress.io/readfile`, + } + }, timed_out (obj) { return { message: `${cmd('{{cmd}}', '"{{file}}"')} timed out after waiting \`{{timeout}}ms\`.`, diff --git a/packages/driver/src/cypress/log.ts b/packages/driver/src/cypress/log.ts index 588d38c03e17..0497809f9176 100644 --- a/packages/driver/src/cypress/log.ts +++ b/packages/driver/src/cypress/log.ts @@ -148,10 +148,8 @@ const defaults = function (state: StateFunc, config, obj) { return {} } - const ret = $dom.isElement(current.get('subject')) ? - $dom.getElements(current.get('subject')) - : - current.get('subject') + const subject = current.get('subject') + const ret = $dom.isElement(subject) ? $dom.getElements(subject) : subject return { Yielded: ret } },