Skip to content

Commit

Permalink
Simplification based on review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
BlueWinds committed Oct 25, 2021
1 parent 2bacf10 commit 14ab9ec
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 30 deletions.
8 changes: 4 additions & 4 deletions packages/driver/cypress/integration/commands/files_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('src/cy/commands/files', () => {
// https://github.com/cypress-io/cypress/issues/1558
it('passes explicit null encoding through to server and decodes response', () => {
Cypress.backend.resolves({
contents: Buffer.from('\n').toString('base64'),
contents: Buffer.from('\n'),
filePath: '/path/to/foo.json',
})

Expand Down Expand Up @@ -357,16 +357,16 @@ describe('src/cy/commands/files', () => {
})

// https://github.com/cypress-io/cypress/issues/1558
it('explicit null encoding is sent to server as base64 string', () => {
it('explicit null encoding is sent to server as Buffer', () => {
Cypress.backend.resolves(okResponse)

cy.writeFile('foo.txt', Buffer.from([0, 0, 54, 255]), null).then(() => {
expect(Cypress.backend).to.be.calledWith(
'write:file',
'foo.txt',
'AAA2/w==',
Buffer.from([0, 0, 54, 255]),
{
encoding: 'base64',
encoding: null,
flag: 'w',
},
)
Expand Down
15 changes: 6 additions & 9 deletions packages/driver/src/cy/commands/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,12 @@ export default (Commands, Cypress, cy) => {
args: { cmd: 'readFile', action: 'read', file, filePath: err.filePath, error: err.message },
})
}).then(({ contents, filePath }) => {
// Binary files (read with explicit `null` encoding by the user) are transmitted over the
// websocket base64 encoded. See packages/server/lib/files.js.
// https://github.com/cypress-io/cypress/issues/1558
// 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`, which webpack polyfills in the browser.
if (options.encoding === null) {
contents = Buffer.from(contents, 'base64')
contents = Buffer.from(contents)
}

consoleProps['File Path'] = filePath
Expand Down Expand Up @@ -136,12 +138,7 @@ export default (Commands, Cypress, cy) => {
})
}

if (Buffer.isBuffer(contents)) {
contents = contents.toString('base64')
options.encoding = 'base64'
}

if (_.isObject(contents)) {
if (_.isObject(contents) && !Buffer.isBuffer(contents)) {
contents = JSON.stringify(contents, null, 2)
}

Expand Down
7 changes: 4 additions & 3 deletions packages/driver/src/cy/commands/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,11 @@ export default (Commands, Cypress, cy, state, config) => {
}

// https://github.com/cypress-io/cypress/issues/1558
// Binary files (read with explicit `null` encoding by the user) are transmitted over the
// websocket base64 encoded. See packages/server/lib/fixture.js.
// 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`, which webpack polyfills in the browser.
if (options.encoding === null) {
response = Buffer.from(response, 'base64')
response = Buffer.from(response)
}

// add the fixture to the cache
Expand Down
6 changes: 2 additions & 4 deletions packages/server/lib/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { fs } = require('./util/fs')
module.exports = {
readFile (projectRoot, file, options = {}) {
const filePath = path.resolve(projectRoot, file)
const readFn = path.extname(filePath) === '.json' ? fs.readJsonAsync : fs.readFileAsync
const readFn = (path.extname(filePath) === '.json' && options.encoding !== null) ? fs.readJsonAsync : fs.readFileAsync

// https://github.com/cypress-io/cypress/issues/1558
// If no encoding is specified, then Cypress has historically defaulted
Expand All @@ -14,9 +14,7 @@ module.exports = {
return readFn(filePath, options.encoding === undefined ? 'utf8' : options.encoding)
.then((contents) => {
return {
// For binary files, we base64 encode them for transmission over the websocket
// There is a matching Buffer.from() in packages/driver/src/cy/commands/files.ts
contents: options.encoding === null ? contents.toString('base64') : contents,
contents,
filePath,
}
})
Expand Down
4 changes: 0 additions & 4 deletions packages/server/lib/socket-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,6 @@ export class SocketBase {
return firefoxUtil.windowFocus()
case 'get:fixture':
return getFixture(args[0], args[1])
// Buffers are base64 encoded for sending over the websocket. This is done
// here rather than in fixture code because net stubbing wants to send the
// unencoded buffer, and it also relies on the same fixture loading code.
.then((resp) => Buffer.isBuffer(resp) ? resp.toString('base64') : resp)
case 'read:file':
return files.readFile(config.projectRoot, args[0], args[1])
case 'write:file':
Expand Down
8 changes: 4 additions & 4 deletions packages/server/test/unit/files_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ describe('lib/files', () => {
})

// https://github.com/cypress-io/cypress/issues/1558
it('explicit null encoding is sent to driver as base64 string', function () {
it('explicit null encoding is sent to driver as a Buffer', function () {
return files.readFile(this.projectRoot, 'tests/_fixtures/ascii.foo', { encoding: null }).then(({ contents }) => {
expect(contents).to.eql(Buffer.from('\n').toString('base64'))
expect(contents).to.eql(Buffer.from('\n'))
})
})

Expand Down Expand Up @@ -83,10 +83,10 @@ describe('lib/files', () => {
})

// https://github.com/cypress-io/cypress/issues/1558
it('explicit null encoding is read from driver as base64 string', function () {
it('explicit null encoding is written exactly as received', function () {
return files.writeFile(this.projectRoot, '.projects/write_file.txt', Buffer.from(''), { encoding: null }).then(() => {
return files.readFile(this.projectRoot, '.projects/write_file.txt', { encoding: null }).then(({ contents }) => {
expect(contents).to.eql(Buffer.from('').toString('base64'))
expect(contents).to.eql(Buffer.from(''))
})
})
})
Expand Down
4 changes: 2 additions & 2 deletions packages/server/test/unit/socket_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,9 @@ describe('lib/socket', () => {
return this.client.emit('backend:request', 'get:fixture', 'does-not-exist.txt', {}, cb)
})

it('converts Buffers to base64 strings', function (done) {
it('passes Buffers through intact', function (done) {
const cb = function (resp) {
expect(resp.response).to.eq('W3sianNvbiI6IHRydWV9XQ==')
expect(resp.response).to.eql(Buffer.from('[{"json": true}]'))

return done()
}
Expand Down

0 comments on commit 14ab9ec

Please sign in to comment.