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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions CONTRIBUTING.md
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
@@ -1,3 +1,14 @@
exports['errors .errors.formErrorText calls solution if a function 1'] = `
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added ability to have error solution being a function, not just static text

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'] = [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the names of the errors are now sorted before snapshotting

"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
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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

filled missing solution for this error, pointing at the GH issue


----------

Failed to access /invalid/cache/dir:
Expand Down
41 changes: 38 additions & 3 deletions cli/__snapshots__/verify_spec.js
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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

when verifying Cypress enable logging so we can tell if this is display error


----------

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
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 = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a complex error that has both the display error and the follow-up error

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))}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

at least give the relevant issue url

`,
}

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)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if error solution property is a function, let it form the full error text


`)
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