Skip to content

Commit

Permalink
cli: fix the STDIN pipe on Windows (#5045)
Browse files Browse the repository at this point in the history
* cli: pipe stdin

* uggh, here is the actual change

* update cli unit tests

* add unit test
  • Loading branch information
bahmutov authored and flotwig committed Sep 24, 2019
1 parent 2d498bb commit 908c738
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
20 changes: 17 additions & 3 deletions cli/lib/exec/spawn.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,25 @@ module.exports = {
child.on('close', resolve)
child.on('error', reject)

child.stdin && child.stdin.pipe(process.stdin)
child.stdout && child.stdout.pipe(process.stdout)
// if stdio options is set to 'pipe', then
// we should set up pipes:
// process STDIN (read stream) => child STDIN (writeable)
// child STDOUT => process STDOUT
// child STDERR => process STDERR with additional filtering
if (child.stdin) {
debug('piping process STDIN into child STDIN')
process.stdin.pipe(child.stdin)
}

if (child.stdout) {
debug('piping child STDOUT to process STDOUT')
child.stdout.pipe(process.stdout)
}

// if this is defined then we are manually piping for linux
// to filter out the garbage
child.stderr &&
if (child.stderr) {
debug('piping child STDERR to process STDERR')
child.stderr.on('data', (data) => {
const str = data.toString()

Expand All @@ -158,6 +171,7 @@ module.exports = {
// else pass it along!
process.stderr.write(data)
})
}

// https://github.com/cypress-io/cypress/issues/1841
// In some versions of node, it will throw on windows
Expand Down
8 changes: 7 additions & 1 deletion cli/test/lib/exec/spawn_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ describe('lib/exec/spawn', function () {
},
}

sinon.stub(process, 'stdin').value(new EE)
// process.stdin is both an event emitter and a readable stream
this.processStdin = new EE()
this.processStdin.pipe = sinon.stub().returns(undefined)
sinon.stub(process, 'stdin').value(this.processStdin)
sinon.stub(cp, 'spawn').returns(this.spawnedProcess)
sinon.stub(xvfb, 'start').resolves()
sinon.stub(xvfb, 'stop').resolves()
Expand Down Expand Up @@ -292,6 +295,9 @@ describe('lib/exec/spawn', function () {
return spawn.start()
.then(() => {
expect(cp.spawn.firstCall.args[2].stdio).to.deep.eq('pipe')
// parent process STDIN was piped to child process STDIN
expect(this.processStdin.pipe, 'process.stdin').to.have.been.calledOnce
.and.to.have.been.calledWith(this.spawnedProcess.stdin)
})
})

Expand Down

0 comments on commit 908c738

Please sign in to comment.