Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically retry verify and run commands on Linux if suspect DISPLAY problem #4165

Merged
merged 22 commits into from
May 13, 2019
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
retry verify with our XVFB
  • Loading branch information
bahmutov committed May 9, 2019
commit 774715251af3084a3dcba862a6b346d169970bdf
29 changes: 22 additions & 7 deletions cli/lib/errors.js
Original file line number Diff line number Diff line change
@@ -107,8 +107,20 @@ const smokeTestFailure = (smokeTestCommand, timedOut) => {
}

const isDisplayOnLinuxSet = () => {
return os.platform() === 'linux' &&
Boolean(process.env.DISPLAY)
return os.platform() === 'linux' && Boolean(process.env.DISPLAY)
}

const invalidDisplayError = {
description: 'Cypress failed to start with own XVFB.',
solution: stripIndent`
This is usually caused by a missing library or dependency.

The error below should indicate which dependency is missing.

${chalk.blue(requiredDependenciesUrl)}

If you are using Docker, we provide containers with all required dependencies installed.
`,
}

const missingDependency = {
@@ -220,8 +232,8 @@ function addPlatformInformation (info) {
/**
* Forms nice error message with error and platform information,
* and if possible a way to solve it. Resolves with a string.
*/
function formErrorText (info, msg) {
*/
function formErrorText (info, msg, prevMessage) {
const hr = '----------'

return addPlatformInformation(info)
@@ -234,7 +246,9 @@ function formErrorText (info, msg) {
)
}

const solution = is.fn(obj.solution) ? obj.solution() : obj.solution
const solution = is.fn(obj.solution)
? obj.solution(msg, prevMessage)
: obj.solution

la(is.unemptyString(solution), 'expected solution to be text', solution)

@@ -281,8 +295,8 @@ const raise = (text) => {
}

const throwFormErrorText = (info) => {
return (msg) => {
return formErrorText(info, msg)
return (msg, prevMessage) => {
return formErrorText(info, msg, prevMessage)
.then(raise)
}
}
@@ -298,6 +312,7 @@ module.exports = {
missingApp,
notInstalledCI,
missingDependency,
invalidDisplayError,
versionMismatch,
binaryNotExecutable,
unexpected,
47 changes: 41 additions & 6 deletions cli/lib/tasks/verify.js
Original file line number Diff line number Diff line change
@@ -48,7 +48,7 @@ const runSmokeTest = (binaryDir, options) => {
return throwFormErrorText(errors.missingXvfb)(`Caught error trying to run XVFB: "${err.message}"`)
}

const onSmokeTestError = (smokeTestCommand) => {
const onSmokeTestError = (smokeTestCommand, runningWithOurXvfb, prevDisplayError) => {
return (err) => {
debug('Smoke test failed:', err)

@@ -59,6 +59,28 @@ const runSmokeTest = (binaryDir, options) => {
return throwFormErrorText(errors.smokeTestFailure(smokeTestCommand, true))(errMessage)
}

if (!runningWithOurXvfb && !prevDisplayError && util.isDisplayError(errMessage)) {
// running without our XVFB
// for the very first time
// and we hit invalid display error
debug('Smoke test hit Linux display problem %s', errMessage)
const err = new Error(errMessage)

err.displayError = true
err.platform = 'linux'
throw err
}

if (prevDisplayError) {
debug('this was our 2nd attempt at verifying')
debug('first we tried with user-given DISPLAY')
debug('now we have tried spinning our own XVFB')
debug('and yet it still has failed with')
debug(errMessage)

return throwFormErrorText(errors.invalidDisplayError)(errMessage, prevDisplayError.message)
}

return throwFormErrorText(errors.missingDependency)(errMessage)
}
}
@@ -67,9 +89,13 @@ const runSmokeTest = (binaryDir, options) => {

debug('needs XVFB?', needsXvfb)

const spawn = () => {
/**
* Spawn Cypress running smoke test to check if all operating system
* dependencies are good.
*/
const spawn = (runningWithOurXvfb, prevDisplayError) => {
const random = _.random(0, 1000)
const args = ['--smoke-test', `--ping=${random}`]
const args = ['--smoke-test', `--ping=${random}`, '--enable-logging']
const smokeTestCommand = `${cypressExecPath} ${args.join(' ')}`

debug('smoke test command:', smokeTestCommand)
@@ -79,7 +105,7 @@ const runSmokeTest = (binaryDir, options) => {
args,
{ timeout: options.smokeTestTimeout }
))
.catch(onSmokeTestError(smokeTestCommand))
.catch(onSmokeTestError(smokeTestCommand, runningWithOurXvfb, prevDisplayError))
.then((result) => {

// TODO: when execa > 1.1 is released
@@ -96,18 +122,27 @@ const runSmokeTest = (binaryDir, options) => {
})
}

if (needsXvfb) {
const spinXvfbAndVerify = (prevDisplayError) => {
return xvfb.start()
.catch(onXvfbError)
.then(spawn)
.then(spawn.bind(null, true, prevDisplayError))
.finally(() => {
return xvfb.stop()
.catch(onXvfbError)
})
}

if (needsXvfb) {
return spinXvfbAndVerify()
}

return spawn()
.catch({ displayError: true, platform: 'linux' }, (e) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handle display error once

debug('there was a display error')
debug('will try spinning our own XVFB and verify Cypress')

return spinXvfbAndVerify(e)
})
}

function testBinary (version, binaryDir, options) {
23 changes: 22 additions & 1 deletion cli/lib/util.js
Original file line number Diff line number Diff line change
@@ -30,6 +30,14 @@ function normalizeModuleOptions (options = {}) {
return _.mapValues(options, stringify)
}

/**
* Returns true if the platform is Linux. We do a lot of different
* stuff on Linux (like XVFB) and it helps to has readable code
*/
const isLinux = () => {
return os.platform() === 'linux'
}

function stdoutLineMatches (expectedLine, stdout) {
const lines = stdout.split('\n').map(R.trim)
const lineMatches = R.equals(expectedLine)
@@ -180,9 +188,11 @@ const util = {
return Promise.resolve(executable(filePath))
},

isLinux,

getOsVersionAsync () {
return Promise.try(() => {
if (os.platform() === 'linux') {
if (isLinux()) {
return getosAsync()
.then((osInfo) => {
return [osInfo.dist, osInfo.release].join(' - ')
@@ -256,6 +266,17 @@ const util = {

return `${issuesUrl}/${number}`
},

/**
* If the DISPLAY variable is set incorrectly, when trying to spawn
* Cypress executable we get an error like this:
```
[1005:0509/184205.663837:WARNING:browser_main_loop.cc(258)] Gtk: cannot open display: 99
```
*/
isDisplayError (errorMessage) {
return isLinux() && errorMessage.includes('cannot open display:')
},
}

module.exports = util