Skip to content

Commit

Permalink
fix windowsHide prop (#2853)
Browse files Browse the repository at this point in the history
* Revert "Issue 21316 windows node11 (#2699) (fixes #2667)"

This reverts commit 54ce93b.

* add windowsHide prop

* fix test name
  • Loading branch information
kuceb authored and brian-mann committed Dec 3, 2018
1 parent 742e0d3 commit d57ca55
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 52 deletions.
13 changes: 11 additions & 2 deletions cli/lib/exec/spawn.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const _ = require('lodash')
const os = require('os')
const cp = require('child_process')
const path = require('path')
const Promise = require('bluebird')
Expand All @@ -12,12 +13,16 @@ const { throwFormErrorText, errors } = require('../errors')
const isXlibOrLibudevRe = /^(?:Xlib|libudev)/
const isHighSierraWarningRe = /\*\*\* WARNING/

function isPlatform (platform) {
return os.platform() === platform
}

function needsStderrPiped (needsXvfb) {
return util.isPlatform('darwin') || (needsXvfb && util.isPlatform('linux'))
return isPlatform('darwin') || (needsXvfb && isPlatform('linux'))
}

function needsEverythingPipedDirectly () {
return util.isPlatform('win32')
return isPlatform('win32')
}

function getStdio (needsXvfb) {
Expand Down Expand Up @@ -68,6 +73,7 @@ module.exports = {
}

const overrides = util.getEnvOverrides()
const node11WindowsFix = isPlatform('win32')

debug('spawning Cypress with executable: %s', executable)
debug('spawn forcing env overrides %o', overrides)
Expand All @@ -81,6 +87,9 @@ module.exports = {
// also figure out whether we should force stdout and stderr into thinking
// it is a tty as opposed to a pipe.
options.env = _.extend({}, options.env, overrides)
if (node11WindowsFix) {
options = _.extend({}, options, { windowsHide: false })
}

const child = cp.spawn(executable, args, options)

Expand Down
18 changes: 0 additions & 18 deletions cli/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const isInstalledGlobally = require('is-installed-globally')
const pkg = require(path.join(__dirname, '..', 'package.json'))
const logger = require('./logger')
const debug = require('debug')('cypress:cli')
const compareVersions = require('compare-versions')

const getosAsync = Promise.promisify(getos)

Expand Down Expand Up @@ -68,7 +67,6 @@ const util = {
.mapValues((value) => { // stringify to 1 or 0
return value ? '1' : '0'
})
.extend(util.getNode11WindowsFix()) // the value has to be falsy, '0' as a string not good enoughs
.value()
},

Expand All @@ -80,14 +78,6 @@ const util = {
}
},

getNode11WindowsFix () {
if (compareVersions(util.getNodeVersion(), 'v11') >= 0 && util.isPlatform('win32')) {
return {
windowsHide: false,
}
}
},

getEnvColors () {
const sc = util.supportsColor()

Expand All @@ -98,14 +88,6 @@ const util = {
}
},

getNodeVersion () {
return process.version
},

isPlatform (platform) {
return os.platform() === platform
},

isTty (fd) {
return tty.isatty(fd)
},
Expand Down
1 change: 0 additions & 1 deletion cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
"check-more-types": "2.24.0",
"commander": "2.11.0",
"common-tags": "1.4.0",
"compare-versions": "3.4.0",
"debug": "3.1.0",
"execa": "0.10.0",
"executable": "4.1.1",
Expand Down
26 changes: 25 additions & 1 deletion cli/test/lib/exec/spawn_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ describe('lib/exec/spawn', function () {

it('rejects with error from spawn', function () {
const msg = 'the error message'
this.spawnedProcess.on.withArgs('error').yieldsAsync(new Error(msg))

this.spawnedProcess.on.withArgs('error').yieldsAsync(new Error(msg))

return spawn.start('--foo')
.then(() => {
Expand Down Expand Up @@ -181,6 +181,26 @@ describe('lib/exec/spawn', function () {
})
})

it('sets windowsHide:false property in windows', function () {
this.spawnedProcess.on.withArgs('close').yieldsAsync(0)

os.platform.returns('win32')

return spawn.start([], { env: {} })
.then(() => {
expect(cp.spawn.firstCall.args[2].windowsHide).to.be.false
})
})

it('does not set windowsHide property when in darwin', function () {
this.spawnedProcess.on.withArgs('close').yieldsAsync(0)

return spawn.start([], { env: {} })
.then(() => {
expect(cp.spawn.firstCall.args[2].windowsHide).to.be.undefined
})
})

it('does not force colors and streams when not supported', function () {
this.spawnedProcess.on.withArgs('close').yieldsAsync(0)

Expand Down Expand Up @@ -326,7 +346,9 @@ describe('lib/exec/spawn', function () {
const fn = () => {
called = true
const err = new Error()

err.code = 'EPIPE'

return process.stdin.emit('error', err)
}

Expand All @@ -342,7 +364,9 @@ describe('lib/exec/spawn', function () {
.then(() => {
const fn = () => {
const err = new Error('wattttt')

err.code = 'FAILWHALE'

return process.stdin.emit('error', err)
}

Expand Down
30 changes: 0 additions & 30 deletions cli/test/lib/util_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,36 +191,6 @@ describe('util', () => {
DEBUG_COLORS: '0',
})
})

context('.windowsHide', () => {
it('is false on windows with node 11', () => {
os.platform.returns('win32')
sinon.stub(process, 'version').value('v11.0.0')
expect(util.getEnvOverrides().windowsHide).to.be.false
})

it('is false on windows with node > 11', () => {
os.platform.returns('win32')
sinon.stub(process, 'version').value('v12.0.0')
expect(util.getEnvOverrides().windowsHide).to.be.false
})

it('is undefined on windows with node < 11', () => {
os.platform.returns('win32')
sinon.stub(process, 'version').value('v8.0.0')
expect(util.getEnvOverrides().windowsHide).to.be.undefined

os.platform.returns('win32')
sinon.stub(process, 'version').value('v10.0.0')
expect(util.getEnvOverrides().windowsHide).to.be.undefined
})

it('is undefined on non-windows with node 11', () => {
os.platform.returns('darwin')
sinon.stub(process, 'version').value('v11.0.0')
expect(util.getEnvOverrides().windowsHide).to.be.undefined
})
})
})

context('.getForceTty', () => {
Expand Down

0 comments on commit d57ca55

Please sign in to comment.