Skip to content

Commit

Permalink
Automatically retry verify and run commands on Linux if suspect DISPL…
Browse files Browse the repository at this point in the history
…AY problem (#4165)

* cli: debug explanation for XVFB

* linting

* add chai-as-promised to CLI dev

* show Linux specific error solution if cannot verify

* add todo

* chore: consolidate github issue url logic

* linting

* add npm script lint-changed to quickly eslint fix changes JS files

* retry verify with our XVFB

* update errors and tests

* update CLI tests

* add test for display error message

* fix unit test

* add successful test with retry

* finish verify retry test

* warn users if hit display problem on first verify

* try to detect display problem when running electron and retry with our xvfb

* add warning message to spawn when attempting xvfb re-run

* add test for display retry behavior on spawn

* more comments for clarity

* fix typo
  • Loading branch information
bahmutov authored May 13, 2019
1 parent 1b1c2f2 commit d25cfac
Show file tree
Hide file tree
Showing 19 changed files with 617 additions and 64 deletions.
8 changes: 8 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,14 @@ DEBUG=cypress:launcher
We use [eslint](https://eslint.org/) to lint all JavaScript code and follow rules specified in
[eslint-plugin-cypress-dev](https://github.com/cypress-io/eslint-plugin-cypress-dev) plugin.

When you edit files, you can quickly fix all changed files before committing using

```bash
npm run lint-changed
```

When committing files, we run Git pre-commit hook to fix the staged JS files. See the `precommit-lint` script in [package.json](package.json). This might change JS files and you would need to commit the changes again.

### Tests

For most packages there are typically unit and some integration tests.
Expand Down
65 changes: 56 additions & 9 deletions cli/__snapshots__/errors_spec.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
exports['errors .errors.formErrorText calls solution if a function 1'] = `
description
a solution
----------
Platform: test platform (Foo-OsVersion)
Cypress Version: 1.2.3
`

exports['errors .errors.formErrorText returns fully formed text message 1'] = `
Your system is missing the dependency: XVFB
Expand All @@ -16,18 +27,54 @@ Cypress Version: 1.2.3
`

exports['errors individual has the following errors 1'] = [
"nonZeroExitCodeXvfb",
"missingXvfb",
"missingApp",
"notInstalledCI",
"missingDependency",
"versionMismatch",
"CYPRESS_RUN_BINARY",
"binaryNotExecutable",
"unexpected",
"failedDownload",
"failedUnzip",
"invalidCacheDirectory",
"invalidDisplayError",
"missingApp",
"missingDependency",
"missingXvfb",
"nonZeroExitCodeXvfb",
"notInstalledCI",
"removed",
"CYPRESS_RUN_BINARY",
"smokeTestFailure"
"smokeTestFailure",
"unexpected",
"versionMismatch"
]

exports['invalid display error'] = `
Cypress failed to start.
First, we have tried to start Cypress using your DISPLAY settings
but encountered the following problem:
----------
prev message
----------
Then we started our own XVFB and tried to start Cypress again, but
got the following error:
----------
current message
----------
This is usually caused by a missing library or dependency.
The error above should indicate which dependency is missing.
https://on.cypress.io/required-dependencies
If you are using Docker, we provide containers with all required dependencies installed.
----------
Platform: test platform (Foo-OsVersion)
Cypress Version: 1.2.3
`
3 changes: 3 additions & 0 deletions cli/__snapshots__/install_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ https://on.cypress.io/installing-cypress
exports['invalid cache directory 1'] = `
Error: Cypress cannot write to the cache directory due to file permissions
See discussion and possible solutions at
https://github.com/cypress-io/cypress/issues/1281
----------
Failed to access /invalid/cache/dir:
Expand Down
41 changes: 38 additions & 3 deletions cli/__snapshots__/verify_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ Error: Cypress verification timed out.
This command failed with the following output:
/cache/Cypress/1.2.3/Cypress.app/Contents/MacOS/Cypress --smoke-test --ping=222
/cache/Cypress/1.2.3/Cypress.app/Contents/MacOS/Cypress --smoke-test --ping=222 --enable-logging
----------
Expand All @@ -181,7 +181,7 @@ Error: Cypress verification failed.
This command failed with the following output:
/cache/Cypress/1.2.3/Cypress.app/Contents/MacOS/Cypress --smoke-test --ping=222
/cache/Cypress/1.2.3/Cypress.app/Contents/MacOS/Cypress --smoke-test --ping=222 --enable-logging
----------
Expand All @@ -203,7 +203,7 @@ Error: Cypress verification failed.
This command failed with the following output:
/cache/Cypress/1.2.3/Cypress.app/Contents/MacOS/Cypress --smoke-test --ping=222
/cache/Cypress/1.2.3/Cypress.app/Contents/MacOS/Cypress --smoke-test --ping=222 --enable-logging
----------
Expand Down Expand Up @@ -384,3 +384,38 @@ Platform: darwin (Foo-OsVersion)
Cypress Version: 1.2.3
`

exports['tried to verify twice, on the first try got the DISPLAY error'] = `
Cypress failed to start.
First, we have tried to start Cypress using your DISPLAY settings
but encountered the following problem:
----------
[some noise here] Gtk: cannot open display: 987
----------
Then we started our own XVFB and tried to start Cypress again, but
got the following error:
----------
some other error
----------
This is usually caused by a missing library or dependency.
The error above should indicate which dependency is missing.
https://on.cypress.io/required-dependencies
If you are using Docker, we provide containers with all required dependencies installed.
----------
Platform: linux (Foo-OsVersion)
Cypress Version: 1.2.3
`
98 changes: 80 additions & 18 deletions cli/lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@ const os = require('os')
const chalk = require('chalk')
const { stripIndent, stripIndents } = require('common-tags')
const { merge } = require('ramda')
const la = require('lazy-ass')
const is = require('check-more-types')

const util = require('./util')
const state = require('./tasks/state')

const issuesUrl = 'https://github.com/cypress-io/cypress/issues'
const docsUrl = 'https://on.cypress.io'
const requiredDependenciesUrl = `${docsUrl}/required-dependencies`

// TODO it would be nice if all error objects could be enforced via types
// to only have description + solution properties

const hr = '----------'

// common errors Cypress application can encounter
const failedDownload = {
description: 'The Cypress App could not be downloaded.',
Expand All @@ -21,7 +27,7 @@ const failedUnzip = {
solution: stripIndent`
Search for an existing issue or open a GitHub issue at
${chalk.blue(issuesUrl)}
${chalk.blue(util.issuesUrl)}
`,
}

Expand Down Expand Up @@ -102,6 +108,39 @@ const smokeTestFailure = (smokeTestCommand, timedOut) => {
}
}

const invalidDisplayError = {
description: 'Cypress failed to start.',
solution (msg, prevMessage) {
return stripIndent`
First, we have tried to start Cypress using your DISPLAY settings
but encountered the following problem:
${hr}
${prevMessage}
${hr}
Then we started our own XVFB and tried to start Cypress again, but
got the following error:
${hr}
${msg}
${hr}
This is usually caused by a missing library or dependency.
The error above should indicate which dependency is missing.
${chalk.blue(requiredDependenciesUrl)}
If you are using Docker, we provide containers with all required dependencies installed.
`
},
}

const missingDependency = {
description: 'Cypress failed to start.',
// this message is too Linux specific
Expand All @@ -118,7 +157,10 @@ const missingDependency = {

const invalidCacheDirectory = {
description: 'Cypress cannot write to the cache directory due to file permissions',
solution: '',
solution: stripIndent`
See discussion and possible solutions at
${chalk.blue(util.getGitHubIssueUrl(1281))}
`,
}

const versionMismatch = {
Expand All @@ -135,7 +177,7 @@ const unexpected = {
Check if there is a GitHub issue describing this crash:
${chalk.blue(issuesUrl)}
${chalk.blue(util.issuesUrl)}
Consider opening a new issue.
`,
Expand Down Expand Up @@ -191,10 +233,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) {
const hr = '----------'

*/
function formErrorText (info, msg, prevMessage) {
return addPlatformInformation(info)
.then((obj) => {
const formatted = []
Expand All @@ -205,20 +245,41 @@ function formErrorText (info, msg) {
)
}

add(`
${obj.description}
la(is.unemptyString(obj.description),
'expected error description to be text', obj.description)

${obj.solution}
// assuming that if there the solution is a function it will handle
// error message and (optional previous error message)
if (is.fn(obj.solution)) {
const text = obj.solution(msg, prevMessage)

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

if (msg) {
add(`
${hr}
${obj.description}
${msg}
${text}
`)
} else {
la(is.unemptyString(obj.solution),
'expected error solution to be text', obj.solution)

add(`
${obj.description}
${obj.solution}
`)

if (msg) {
add(`
${hr}
${msg}
`)
}
}

add(`
Expand Down Expand Up @@ -248,23 +309,24 @@ const raise = (text) => {
}

const throwFormErrorText = (info) => {
return (msg) => {
return formErrorText(info, msg)
return (msg, prevMessage) => {
return formErrorText(info, msg, prevMessage)
.then(raise)
}
}

module.exports = {
raise,
// formError,
formErrorText,
throwFormErrorText,
hr,
errors: {
nonZeroExitCodeXvfb,
missingXvfb,
missingApp,
notInstalledCI,
missingDependency,
invalidDisplayError,
versionMismatch,
binaryNotExecutable,
unexpected,
Expand Down
Loading

0 comments on commit d25cfac

Please sign in to comment.