Skip to content

Commit 2e3b203

Browse files
Blue Femilyrohrboughmjhenkes
authored andcommitted
breaking: upgrade cy.readFile() to be a query command (#25595)
Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com> Co-authored-by: Matt Henkes <mjhenkes@gmail.com>
1 parent f941005 commit 2e3b203

File tree

8 files changed

+148
-122
lines changed

8 files changed

+148
-122
lines changed

cli/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
11
<!-- See the ../guides/writing-the-cypress-changelog.md for details on writing the changelog. -->
2+
## 13.0.0
3+
4+
_Released 01/31/2023 (PENDING)_
5+
6+
**Breaking Changes:**
7+
8+
- 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).
9+
210
## 12.6.0
311

412
_Released 02/14/2023 (PENDING)_

packages/driver/cypress/e2e/commands/assertions.cy.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -652,18 +652,21 @@ describe('src/cy/commands/assertions', () => {
652652
cy.get('button:first', { timeout: 500 }).should('have.class', 'does-not-have-class')
653653
})
654654

655-
it('has a pending state while retrying for commands with onFail', (done) => {
655+
it('has a pending state while retrying for commands with onFail', function (done) {
656656
cy.on('command:retry', () => {
657-
const [readFileLog, shouldLog] = cy.state('current').get('logs')
657+
// Wait for the readFile response to come back from the server
658+
if (this.logs.length < 2) {
659+
return
660+
}
661+
662+
const [readFileLog, shouldLog] = this.logs
658663

659664
expect(readFileLog.get('state')).to.eq('pending')
660665
expect(shouldLog.get('state')).to.eq('pending')
661666

662667
done()
663668
})
664669

665-
cy.on('fail', () => {})
666-
667670
cy.readFile('does-not-exist.json', { timeout: 500 }).should('exist')
668671
})
669672

packages/driver/cypress/e2e/commands/files.cy.js

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ describe('src/cy/commands/files', () => {
1414
})
1515

1616
describe('#readFile', () => {
17+
it('really works', () => {
18+
cy.readFile('./cypress/fixtures/fileSpec.json').its('baseUrl').should('eq', 'http://localhost:3500')
19+
})
20+
21+
it('works when contents are supposed to be null', () => {
22+
cy.readFile('does-not-exist').should('be.null')
23+
})
24+
1725
it('triggers \'read:file\' with the right options', () => {
1826
Cypress.backend.resolves(okResponse)
1927

@@ -80,7 +88,7 @@ describe('src/cy/commands/files', () => {
8088
.resolves(okResponse)
8189

8290
cy.readFile('foo.json').then(() => {
83-
expect(retries).to.eq(1)
91+
expect(retries).to.eq(2)
8492
})
8593
})
8694

@@ -102,18 +110,12 @@ describe('src/cy/commands/files', () => {
102110
})
103111

104112
cy.readFile('foo.json').should('eq', 'quux').then(() => {
105-
expect(retries).to.eq(1)
113+
// Two retries: The first one triggers a backend request and throws a 'not ready' error.
114+
// The second gets foobarbaz, triggering another request to the backend.
115+
expect(retries).to.eq(2)
106116
})
107117
})
108118

109-
it('really works', () => {
110-
cy.readFile('./cypress/fixtures/fileSpec.json').its('baseUrl').should('eq', 'http://localhost:3500')
111-
})
112-
113-
it('works when contents are supposed to be null', () => {
114-
cy.readFile('does-not-exist').should('be.null')
115-
})
116-
117119
describe('.log', () => {
118120
beforeEach(function () {
119121
this.logs = []
@@ -176,10 +178,6 @@ describe('src/cy/commands/files', () => {
176178

177179
this.logs = []
178180

179-
cy.on('fail', () => {
180-
cy.off('log:added', collectLogs)
181-
})
182-
183181
return null
184182
})
185183

@@ -243,7 +241,7 @@ describe('src/cy/commands/files', () => {
243241
cy.on('fail', (err) => {
244242
const { fileLog } = this
245243

246-
assertLogLength(this.logs, 2)
244+
assertLogLength(this.logs, 3)
247245
expect(fileLog.get('error')).to.eq(err)
248246
expect(fileLog.get('state')).to.eq('failed')
249247
expect(err.message).to.eq(stripIndent`\
@@ -388,7 +386,7 @@ describe('src/cy/commands/files', () => {
388386
expect(fileLog.get('error')).to.eq(err)
389387
expect(fileLog.get('state')).to.eq('failed')
390388
expect(err.message).to.eq(stripIndent`\
391-
\`cy.readFile("foo")\` timed out after waiting \`10ms\`.
389+
Timed out retrying after 10ms: \`cy.readFile("foo")\` timed out.
392390
`)
393391

394392
expect(err.docsUrl).to.eq('https://on.cypress.io/readfile')
@@ -412,7 +410,7 @@ describe('src/cy/commands/files', () => {
412410
expect(fileLog.get('error')).to.eq(err)
413411
expect(fileLog.get('state')).to.eq('failed')
414412
expect(err.message).to.eq(stripIndent`\
415-
\`cy.readFile("foo")\` timed out after waiting \`42ms\`.
413+
Timed out retrying after 42ms: \`cy.readFile("foo")\` timed out.
416414
`)
417415

418416
expect(err.docsUrl).to.eq('https://on.cypress.io/readfile')

packages/driver/cypress/e2e/e2e/origin/commands/files.cy.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,11 @@ context('cy.origin files', { browser: '!webkit' }, () => {
6464
})
6565

6666
cy.shouldWithTimeout(() => {
67-
const { consoleProps } = findCrossOriginLogs('readFile', logs, 'foobar.com')
67+
const log = findCrossOriginLogs('readFile', logs, 'foobar.com')
6868

69-
expect(consoleProps.Command).to.equal('readFile')
70-
expect(consoleProps['File Path']).to.include('cypress/fixtures/example.json')
71-
expect(consoleProps.Contents).to.deep.equal({ example: true })
69+
expect(log.consoleProps.Command).to.equal('readFile')
70+
expect(log.consoleProps['File Path']).to.include('cypress/fixtures/example.json')
71+
expect(log.consoleProps.Contents).to.deep.equal({ example: true })
7272
})
7373
})
7474

packages/driver/src/cy/commands/files.ts

Lines changed: 105 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -4,118 +4,132 @@ import { basename } from 'path'
44
import $errUtils from '../../cypress/error_utils'
55
import type { Log } from '../../cypress/log'
66

7-
interface InternalReadFileOptions extends Partial<Cypress.Loggable & Cypress.Timeoutable> {
8-
_log?: Log
9-
encoding: Cypress.Encodings
7+
interface ReadFileOptions extends Partial<Cypress.Loggable & Cypress.Timeoutable> {
8+
encoding?: Cypress.Encodings
109
}
1110

1211
interface InternalWriteFileOptions extends Partial<Cypress.WriteFileOptions & Cypress.Timeoutable> {
1312
_log?: Log
1413
}
1514

1615
export default (Commands, Cypress, cy, state) => {
17-
Commands.addAll({
18-
readFile (file, encoding, userOptions: Partial<Cypress.Loggable & Cypress.Timeoutable> = {}) {
19-
if (_.isObject(encoding)) {
20-
userOptions = encoding
21-
encoding = undefined
16+
Commands.addQuery('readFile', function readFile (file, encoding, options: ReadFileOptions = {}) {
17+
if (_.isObject(encoding)) {
18+
options = encoding
19+
encoding = options.encoding
20+
}
21+
22+
encoding = encoding === undefined ? 'utf8' : encoding
23+
24+
const timeout = options.timeout ?? Cypress.config('defaultCommandTimeout')
25+
26+
this.set('timeout', timeout)
27+
this.set('ensureExistenceFor', 'subject')
28+
29+
const log = options.log !== false && Cypress.log({ message: file, timeout })
30+
31+
if (!file || !_.isString(file)) {
32+
$errUtils.throwErrByPath('files.invalid_argument', {
33+
args: { cmd: 'readFile', file },
34+
})
35+
}
36+
37+
let fileResult: any = null
38+
let filePromise: Promise<void> | null = null
39+
let mostRecentError = $errUtils.cypressErrByPath('files.read_timed_out', {
40+
args: { file },
41+
})
42+
43+
const createFilePromise = () => {
44+
// If we already have a pending request to the backend, we'll wait
45+
// for that one to resolve instead of creating a new one.
46+
if (filePromise) {
47+
return
2248
}
2349

24-
const options: InternalReadFileOptions = _.defaults({}, userOptions, {
50+
fileResult = null
51+
filePromise = Cypress.backend('read:file', file, { encoding })
52+
.timeout(timeout)
53+
.then((result) => {
2554
// https://github.com/cypress-io/cypress/issues/1558
26-
// If no encoding is specified, then Cypress has historically defaulted
27-
// to `utf8`, because of it's focus on text files. This is in contrast to
28-
// NodeJs, which defaults to binary. We allow users to pass in `null`
29-
// to restore the default node behavior.
30-
encoding: encoding === undefined ? 'utf8' : encoding,
31-
log: true,
32-
timeout: Cypress.config('defaultCommandTimeout'),
55+
// https://github.com/cypress-io/cypress/issues/20683
56+
// We invoke Buffer.from() in order to transform this from an ArrayBuffer -
57+
// which socket.io uses to transfer the file over the websocket - into a `Buffer`.
58+
if (encoding === null && result.contents !== null) {
59+
result.contents = Buffer.from(result.contents)
60+
}
61+
62+
// Add the filename to the current command, in case we need it later (such as when storing an alias)
63+
state('current').set('fileName', basename(result.filePath))
64+
65+
fileResult = result
3366
})
67+
.catch((err) => {
68+
if (err.name === 'TimeoutError') {
69+
$errUtils.throwErrByPath('files.timed_out', {
70+
args: { cmd: 'readFile', file, timeout },
71+
retry: false,
72+
})
73+
}
3474

35-
const consoleProps = {}
75+
// Non-ENOENT errors are not retried
76+
if (err.code !== 'ENOENT') {
77+
$errUtils.throwErrByPath('files.unexpected_error', {
78+
args: { cmd: 'readFile', action: 'read', file, filePath: err.filePath, error: err.message },
79+
errProps: { retry: false },
80+
})
81+
}
3682

37-
if (options.log) {
38-
options._log = Cypress.log({
39-
message: file,
40-
timeout: options.timeout,
41-
consoleProps () {
42-
return consoleProps
43-
},
83+
// We have a ENOENT error - the file doesn't exist. Whether this is an error or not is deterimened
84+
// by verifyUpcomingAssertions, when the command_queue receives the null file contents.
85+
fileResult = { contents: null, filePath: err.filePath }
86+
})
87+
.catch((err) => mostRecentError = err)
88+
// Pass or fail, we always clear the filePromise, so future retries know there's no
89+
// live request to the server.
90+
.finally(() => filePromise = null)
91+
}
92+
93+
// When an assertion attached to this command fails, then we want to throw away the existing result
94+
// and create a new promise to read a new one.
95+
this.set('onFail', (err, timedOut) => {
96+
if (err.type === 'existence') {
97+
// file exists but it shouldn't - or - file doesn't exist but it should
98+
const errPath = fileResult.contents ? 'files.existent' : 'files.nonexistent'
99+
const { message, docsUrl } = $errUtils.cypressErrByPath(errPath, {
100+
args: { cmd: 'readFile', file, filePath: fileResult.filePath },
44101
})
45-
}
46102

47-
if (!file || !_.isString(file)) {
48-
$errUtils.throwErrByPath('files.invalid_argument', {
49-
onFail: options._log,
50-
args: { cmd: 'readFile', file },
51-
})
103+
err.message = message
104+
err.docsUrl = docsUrl
52105
}
53106

54-
// We clear the default timeout so we can handle
55-
// the timeout ourselves
56-
cy.clearTimeout()
107+
createFilePromise()
108+
})
57109

58-
const verifyAssertions = () => {
59-
return Cypress.backend('read:file', file, _.pick(options, 'encoding')).timeout(options.timeout)
60-
.catch((err) => {
61-
if (err.name === 'TimeoutError') {
62-
return $errUtils.throwErrByPath('files.timed_out', {
63-
onFail: options._log,
64-
args: { cmd: 'readFile', file, timeout: options.timeout },
65-
})
66-
}
67-
68-
// Non-ENOENT errors are not retried
69-
if (err.code !== 'ENOENT') {
70-
return $errUtils.throwErrByPath('files.unexpected_error', {
71-
onFail: options._log,
72-
args: { cmd: 'readFile', action: 'read', file, filePath: err.filePath, error: err.message },
73-
})
74-
}
75-
76-
return {
77-
contents: null,
78-
filePath: err.filePath,
79-
}
80-
}).then(({ filePath, contents }) => {
81-
// https://github.com/cypress-io/cypress/issues/1558
82-
// https://github.com/cypress-io/cypress/issues/20683
83-
// We invoke Buffer.from() in order to transform this from an ArrayBuffer -
84-
// which socket.io uses to transfer the file over the websocket - into a `Buffer`.
85-
if (options.encoding === null && contents !== null) {
86-
contents = Buffer.from(contents)
87-
}
88-
89-
// Add the filename as a symbol, in case we need it later (such as when storing an alias)
90-
state('current').set('fileName', basename(filePath))
91-
92-
consoleProps['File Path'] = filePath
93-
consoleProps['Contents'] = contents
94-
95-
return cy.verifyUpcomingAssertions(contents, options, {
96-
ensureExistenceFor: 'subject',
97-
onFail (err) {
98-
if (err.type !== 'existence') {
99-
return
100-
}
101-
102-
// file exists but it shouldn't - or - file doesn't exist but it should
103-
const errPath = contents ? 'files.existent' : 'files.nonexistent'
104-
const { message, docsUrl } = $errUtils.cypressErrByPath(errPath, {
105-
args: { cmd: 'readFile', file, filePath },
106-
})
107-
108-
err.message = message
109-
err.docsUrl = docsUrl
110-
},
111-
onRetry: verifyAssertions,
112-
})
113-
})
110+
return () => {
111+
// Once we've read a file, that remains the result, unless it's cleared
112+
// because of a failed assertion in `onFail` above.
113+
if (fileResult) {
114+
const props = {
115+
'Contents': fileResult?.contents,
116+
'File Path': fileResult?.filePath,
117+
}
118+
119+
log && state('current') === this && log.set('consoleProps', () => props)
120+
121+
return fileResult.contents
114122
}
115123

116-
return verifyAssertions()
117-
},
124+
createFilePromise()
118125

126+
// If we don't have a result, then the promise is pending.
127+
// Throw an error and wait for the promise to eventually resolve on a future retry.
128+
throw mostRecentError
129+
}
130+
})
131+
132+
Commands.addAll({
119133
writeFile (fileName, contents, encoding, userOptions: Partial<Cypress.WriteFileOptions & Cypress.Timeoutable> = {}) {
120134
if (_.isObject(encoding)) {
121135
userOptions = encoding

packages/driver/src/cy/commands/querying/querying.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,7 @@ function getAlias (selector, log, cy) {
6666
*/
6767

6868
if (command.get('name') === 'intercept') {
69-
// Intercept aliases are fairly similar, but `getAliasedRequests` does *not* handle indexes
70-
// and we have to do it ourselves here.
69+
// `getAliasedRequests` does *not* handle indexes and we have to do it ourselves here.
7170

7271
const requests = getAliasedRequests(aliasObj.alias, cy.state)
7372

packages/driver/src/cypress/error_messages.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,12 @@ export default {
520520
docsUrl: `https://on.cypress.io/${_.toLower(obj.cmd)}`,
521521
}
522522
},
523+
read_timed_out (obj) {
524+
return {
525+
message: `${cmd('readFile', '"{{file}}"')} timed out.`,
526+
docsUrl: `https://on.cypress.io/readfile`,
527+
}
528+
},
523529
timed_out (obj) {
524530
return {
525531
message: `${cmd('{{cmd}}', '"{{file}}"')} timed out after waiting \`{{timeout}}ms\`.`,

packages/driver/src/cypress/log.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,8 @@ const defaults = function (state: StateFunc, config, obj) {
148148
return {}
149149
}
150150

151-
const ret = $dom.isElement(current.get('subject')) ?
152-
$dom.getElements(current.get('subject'))
153-
:
154-
current.get('subject')
151+
const subject = current.get('subject')
152+
const ret = $dom.isElement(subject) ? $dom.getElements(subject) : subject
155153

156154
return { Yielded: ret }
157155
},

0 commit comments

Comments
 (0)