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

CONTRIBUTING.md

@@ -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.

cli/__snapshots__/errors_spec.js

@@ -31,3 +31,38 @@ exports['errors individual has the following errors 1'] = [
   "CYPRESS_RUN_BINARY",
   "smokeTestFailure"
 ]
+
+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 custom solution returns specific solution if on Linux DISPLAY env is set 1'] = `
+Cypress failed to start.
+
+This is usually caused by a missing library or dependency.
+
+The error below 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.
+
+We have noticed that DISPLAY variable is set to "wrong-display-address"
+This might be a problem if X11 server is not responding.
+
+https://github.com/cypress-io/cypress/issues/4034
+
+Try deleting the DISPLAY variable and running the command again.
+
+----------
+
+Platform: linux (Foo-OsVersion)
+Cypress Version: 1.2.3
+`

cli/__snapshots__/install_spec.js

@@ -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:

cli/lib/errors.js

@@ -2,14 +2,18 @@ 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
+
 // common errors Cypress application can encounter
 const failedDownload = {
   description: 'The Cypress App could not be downloaded.',
@@ -21,7 +25,7 @@ const failedUnzip = {
   solution: stripIndent`
     Search for an existing issue or open a GitHub issue at
 
-      ${chalk.blue(issuesUrl)}
+      ${chalk.blue(util.issuesUrl)}
   `,
 }
 
@@ -102,23 +106,48 @@ const smokeTestFailure = (smokeTestCommand, timedOut) => {
   }
 }
 
+const isDisplayOnLinuxSet = () => {
+  return os.platform() === 'linux' &&
+  Boolean(process.env.DISPLAY)
+}
+
 const missingDependency = {
   description: 'Cypress failed to start.',
   // this message is too Linux specific
-  solution: stripIndent`
-    This is usually caused by a missing library or dependency.
+  solution: () => {
+    let text = stripIndent`
+      This is usually caused by a missing library or dependency.
 
-    The error below should indicate which dependency is missing.
+      The error below should indicate which dependency is missing.
 
-      ${chalk.blue(requiredDependenciesUrl)}
+        ${chalk.blue(requiredDependenciesUrl)}
 
-    If you are using Docker, we provide containers with all required dependencies installed.
-  `,
+      If you are using Docker, we provide containers with all required dependencies installed.
+    `
+
+    if (isDisplayOnLinuxSet()) {
+      const issueUrl = util.getGitHubIssueUrl(4034)
+
+      text += `\n\n${stripIndent`
+        We have noticed that DISPLAY variable is set to "${process.env.DISPLAY}"
+        This might be a problem if X11 server is not responding.
+
+          ${chalk.blue(issueUrl)}
+
+        Try deleting the DISPLAY variable and running the command again.
+      `}`
+    }
+
+    return text
+  },
 }
 
 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 = {
@@ -135,7 +164,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.
   `,
@@ -205,10 +234,14 @@ function formErrorText (info, msg) {
       )
     }
 
+    const solution = is.fn(obj.solution) ? obj.solution() : obj.solution
+
+    la(is.unemptyString(solution), 'expected solution to be text', solution)
+
     add(`
       ${obj.description}
 
-      ${obj.solution}
+      ${solution}
 
     `)
 

cli/lib/exec/spawn.js

@@ -52,7 +52,7 @@ module.exports = {
       executable = path.resolve(util.getEnv('CYPRESS_RUN_BINARY'))
     }
 
-    debug('needs XVFB?', needsXvfb)
+    debug('needs to start own XVFB?', needsXvfb)
 
     // always push cwd into the args
     args = [].concat(args, '--cwd', process.cwd())

cli/lib/exec/xvfb.js

@@ -2,9 +2,11 @@ const os = require('os')
 const Promise = require('bluebird')
 const Xvfb = require('@cypress/xvfb')
 const R = require('ramda')
+const { stripIndent } = require('common-tags')
 const debug = require('debug')('cypress:cli')
 const debugXvfb = require('debug')('cypress:xvfb')
 const { throwFormErrorText, errors } = require('../errors')
+const util = require('../util')
 
 const xvfb = Promise.promisifyAll(new Xvfb({
   timeout: 5000, // milliseconds
@@ -41,7 +43,33 @@ module.exports = {
   },
 
   isNeeded () {
-    return os.platform() === 'linux' && !process.env.DISPLAY
+    if (os.platform() !== 'linux') {
+      return false
+    }
+
+    if (process.env.DISPLAY) {
+      const issueUrl = util.getGitHubIssueUrl(4034)
+
+      const message = stripIndent`
+        DISPLAY environment variable is set to ${process.env.DISPLAY} on Linux
+        Assuming this DISPLAY points at working X11 server,
+        Cypress will not spawn own XVFB
+
+        NOTE: if the X11 server is NOT working, Cypress will exit without explanation,
+          see ${issueUrl}
+        Solution: Unset the DISPLAY variable and try again:
+          DISPLAY= npx cypress run ...
+      `
+
+      debug(message)
+
+      return false
+    }
+
+    debug('undefined DISPLAY environment variable')
+    debug('Cypress will spawn its own XVFB')
+
+    return true
   },
 
   // async method, resolved with Boolean

cli/lib/util.js

@@ -1,6 +1,8 @@
 const _ = require('lodash')
 const R = require('ramda')
 const os = require('os')
+const la = require('lazy-ass')
+const is = require('check-more-types')
 const tty = require('tty')
 const path = require('path')
 const isCi = require('is-ci')
@@ -16,6 +18,8 @@ const pkg = require(path.join(__dirname, '..', 'package.json'))
 const logger = require('./logger')
 const debug = require('debug')('cypress:cli')
 
+const issuesUrl = 'https://github.com/cypress-io/cypress/issues'
+
 const getosAsync = Promise.promisify(getos)
 
 const stringify = (val) => {
@@ -243,6 +247,15 @@ const util = {
   exec: execa,
 
   stdoutLineMatches,
+
+  issuesUrl,
+
+  getGitHubIssueUrl (number) {
+    la(is.positive(number), 'github issue should be a positive number', number)
+    la(_.isInteger(number), 'github issue should be an integer', number)
+
+    return `${issuesUrl}/${number}`
+  },
 }
 
 module.exports = util

cli/package.json

@@ -87,11 +87,13 @@
     "babel-preset-es2015": "6.24.1",
     "bin-up": "1.2.0",
     "chai": "3.5.0",
+    "chai-as-promised": "7.1.1",
     "chai-string": "1.4.0",
     "dependency-check": "3.3.0",
     "dtslint": "0.7.1",
     "execa-wrap": "1.4.0",
     "mock-fs": "4.9.0",
+    "mocked-env": "1.2.4",
     "nock": "9.6.1",
     "nyc": "13.3.0",
     "proxyquire": "2.1.0",

cli/test/lib/errors_spec.js

@@ -4,6 +4,7 @@ const os = require('os')
 const snapshot = require('../support/snapshot')
 const { errors, formErrorText } = require(`${lib}/errors`)
 const util = require(`${lib}/util`)
+const mockedEnv = require('mocked-env')
 
 describe('errors', function () {
   const { missingXvfb } = errors
@@ -29,5 +30,51 @@ describe('errors', function () {
         snapshot(text)
       })
     })
+
+    it('calls solution if a function', () => {
+      const solution = sinon.stub().returns('a solution')
+      const error = {
+        description: 'description',
+        solution,
+      }
+
+      return formErrorText(error)
+      .then((text) => {
+        snapshot(text)
+        expect(solution).to.have.been.calledOnce
+      })
+    })
+
+    it('expects solution to be a string', () => {
+      const error = {
+        description: 'description',
+        solution: 42,
+      }
+
+      return expect(formErrorText(error)).to.be.rejected
+    })
+
+    describe('custom solution', () => {
+      let restore
+
+      beforeEach(() => {
+        restore = mockedEnv({
+          DISPLAY: 'wrong-display-address',
+        })
+      })
+
+      afterEach(() => {
+        restore()
+      })
+
+      it('returns specific solution if on Linux DISPLAY env is set', () => {
+        os.platform.returns('linux')
+
+        return formErrorText(errors.missingDependency)
+        .then((text) => {
+          snapshot(text)
+        })
+      })
+    })
   })
 })

cli/test/lib/util_spec.js

@@ -15,6 +15,26 @@ describe('util', () => {
     sinon.stub(logger, 'error')
   })
 
+  context('.getGitHubIssueUrl', () => {
+    it('returls url for issue number', () => {
+      const url = util.getGitHubIssueUrl(4034)
+
+      expect(url).to.equal('https://github.com/cypress-io/cypress/issues/4034')
+    })
+
+    it('throws for anything but a positive integer', () => {
+      expect(() => {
+        return util.getGitHubIssueUrl('4034')
+      }).to.throw
+      expect(() => {
+        return util.getGitHubIssueUrl(-5)
+      }).to.throw
+      expect(() => {
+        return util.getGitHubIssueUrl(5.19)
+      }).to.throw
+    })
+  })
+
   context('.stdoutLineMatches', () => {
     const { stdoutLineMatches } = util
 

cli/test/spec_helper.js

@@ -27,6 +27,7 @@ global.lib = path.join(__dirname, '..', 'lib')
 require('chai')
 .use(require('@cypress/sinon-chai'))
 .use(require('chai-string'))
+.use(require('chai-as-promised'))
 
 sinon.usingPromise(Promise)
 

package.json

@@ -33,6 +33,7 @@
     "clean-deps": "npm run all clean-deps && rm -rf node_modules",
     "docker": "./scripts/run-docker-local.sh",
     "lint-js": "eslint --fix scripts/*.js packages/ts/*.js cli/*.js cli/**/*.js",
+    "lint-changed": "git diff --name-only | grep '\\.js$' | xargs npx eslint --fix",
     "lint-coffee": "coffeelint scripts/**/*.coffee",
     "lint": "npm run lint-js && npm run lint-coffee",
     "pretest": "npm run lint && npm run all lint && npm run test-scripts",

packages/server/lib/cypress.coffee

@@ -59,7 +59,9 @@ module.exports = {
             ## promise is expecting this object structure
             debug("electron finished with", code)
             resolve({totalFailed: code})
-          cypressElectron.open(".", require("./util/args").toArray(options), fn)
+          args = require("./util/args").toArray(options)
+          debug("electron open arguments %o", args)
+          cypressElectron.open(".", args, fn)
 
   openProject: (options) ->
     ## this code actually starts a project