Handle --project "" command line argument (#7744)

Co-authored-by: Jennifer Shehane <jennifer@cypress.io>

cli/__snapshots__/errors_spec.js

@@ -35,6 +35,7 @@ exports['errors individual has the following errors 1'] = [
   "incompatibleHeadlessFlags",
   "invalidCacheDirectory",
   "invalidCypressEnv",
+  "invalidRunProjectPath",
   "invalidSmokeTestDisplayError",
   "missingApp",
   "missingDependency",
@@ -44,6 +45,7 @@ exports['errors individual has the following errors 1'] = [
   "removed",
   "smokeTestFailure",
   "unexpected",
+  "unknownError",
   "versionMismatch"
 ]
 

cli/lib/cypress.js

@@ -9,13 +9,25 @@ const run = require('./exec/run')
 const util = require('./util')
 
 const cypressModuleApi = {
+  /**
+   * Opens Cypress GUI
+   * @see https://on.cypress.io/module-api#cypress-open
+   */
   open (options = {}) {
     options = util.normalizeModuleOptions(options)
 
     return open.start(options)
   },
 
+  /**
+   * Runs Cypress tests in the current project
+   * @see https://on.cypress.io/module-api#cypress-run
+   */
   run (options = {}) {
+    if (!run.isValidProject(options.project)) {
+      return Promise.reject(new Error(`Invalid project path parameter: ${options.project}`))
+    }
+
     options = util.normalizeModuleOptions(options)
 
     return tmp.fileAsync()

cli/lib/errors.js

@@ -9,13 +9,36 @@ const state = require('./tasks/state')
 
 const docsUrl = 'https://on.cypress.io'
 const requiredDependenciesUrl = `${docsUrl}/required-dependencies`
+const runDocumentationUrl = `${docsUrl}/cypress-run`
 
 // TODO it would be nice if all error objects could be enforced via types
 // to only have description + solution properties
 
 const hr = '----------'
 
+const genericErrorSolution = stripIndent`
+  Search for an existing issue or open a GitHub issue at
+
+    ${chalk.blue(util.issuesUrl)}
+`
+
 // common errors Cypress application can encounter
+const unknownError = {
+  description: 'Unknown Cypress CLI error',
+  solution: genericErrorSolution,
+}
+
+const invalidRunProjectPath = {
+  description: 'Invalid --project path',
+  solution: stripIndent`
+    Please provide a valid project path.
+
+    Learn more about ${chalk.cyan('cypress run')} at:
+
+      ${chalk.blue(runDocumentationUrl)}
+  `,
+}
+
 const failedDownload = {
   description: 'The Cypress App could not be downloaded.',
   solution: stripIndent`
@@ -26,11 +49,7 @@ const failedDownload = {
 
 const failedUnzip = {
   description: 'The Cypress App could not be unzipped.',
-  solution: stripIndent`
-    Search for an existing issue or open a GitHub issue at
-
-      ${chalk.blue(util.issuesUrl)}
-  `,
+  solution: genericErrorSolution,
 }
 
 const missingApp = (binaryDir) => {
@@ -390,6 +409,7 @@ module.exports = {
   getError,
   hr,
   errors: {
+    unknownError,
     nonZeroExitCodeXvfb,
     missingXvfb,
     missingApp,
@@ -408,5 +428,6 @@ module.exports = {
     smokeTestFailure,
     childProcessKilled,
     incompatibleHeadlessFlags,
+    invalidRunProjectPath,
   },
 }

cli/lib/exec/run.js

@@ -6,11 +6,60 @@ const spawn = require('./spawn')
 const verify = require('../tasks/verify')
 const { exitWithError, errors } = require('../errors')
 
-// maps options collected by the CLI
-// and forms list of CLI arguments to the server
+/**
+ * Throws an error with "details" property from
+ * "errors" object.
+ * @param {Object} details - Error details
+ */
+const throwInvalidOptionError = (details) => {
+  if (!details) {
+    details = errors.unknownError
+  }
+
+  // throw this error synchronously, it will be caught later on and
+  // the details will be propagated to the promise chain
+  const err = new Error()
+
+  err.details = details
+  throw err
+}
+
+/**
+ * Typically a user passes a string path to the project.
+ * But "cypress open" allows using `false` to open in global mode,
+ * and the user can accidentally execute `cypress run --project false`
+ * which should be invalid.
+ */
+const isValidProject = (v) => {
+  if (typeof v === 'boolean') {
+    return false
+  }
+
+  if (v === '' || v === 'false' || v === 'true') {
+    return false
+  }
+
+  return true
+}
+
+/**
+ * Maps options collected by the CLI
+ * and forms list of CLI arguments to the server.
+ *
+ * Note: there is lightweight validation, with errors
+ * thrown synchronously.
+ *
+ * @returns {string[]} list of CLI arguments
+ */
 const processRunOptions = (options = {}) => {
   debug('processing run options %o', options)
 
+  if (!isValidProject(options.project)) {
+    debug('invalid project option %o', { project: options.project })
+
+    return throwInvalidOptionError(errors.invalidRunProjectPath)
+  }
+
   const args = ['--run-project', options.project]
 
   if (options.browser) {
@@ -55,12 +104,7 @@ const processRunOptions = (options = {}) => {
 
   if (options.headless) {
     if (options.headed) {
-      // throw this error synchronously, it will be caught later on and
-      // the details will be propagated to the promise chain
-      const err = new Error()
-
-      err.details = errors.incompatibleHeadlessFlags
-      throw err
+      return throwInvalidOptionError(errors.incompatibleHeadlessFlags)
     }
 
     args.push('--headed', !options.headless)
@@ -123,6 +167,7 @@ const processRunOptions = (options = {}) => {
 
 module.exports = {
   processRunOptions,
+  isValidProject,
   // resolves with the number of failed tests
   start (options = {}) {
     _.defaults(options, {

cli/test/lib/cypress_spec.js

@@ -148,6 +148,18 @@ describe('cypress', function () {
       })
     })
 
+    it('rejects if project is an empty string', () => {
+      return expect(cypress.run({ project: '' })).to.be.rejected
+    })
+
+    it('rejects if project is true', () => {
+      return expect(cypress.run({ project: true })).to.be.rejected
+    })
+
+    it('rejects if project is false', () => {
+      return expect(cypress.run({ project: false })).to.be.rejected
+    })
+
     it('passes quiet: true', () => {
       const opts = {
         quiet: true,

cli/test/lib/exec/run_spec.js

@@ -15,6 +15,28 @@ describe('exec run', function () {
   })
 
   context('.processRunOptions', function () {
+    it('allows string --project option', () => {
+      const args = run.processRunOptions({
+        project: '/path/to/project',
+      })
+
+      expect(args).to.deep.equal(['--run-project', '/path/to/project'])
+    })
+
+    it('throws an error for empty string --project', () => {
+      expect(() => run.processRunOptions({ project: '' })).to.throw()
+    })
+
+    it('throws an error for boolean --project', () => {
+      expect(() => run.processRunOptions({ project: false })).to.throw()
+      expect(() => run.processRunOptions({ project: true })).to.throw()
+    })
+
+    it('throws an error for --project "false" or "true"', () => {
+      expect(() => run.processRunOptions({ project: 'false' })).to.throw()
+      expect(() => run.processRunOptions({ project: 'true' })).to.throw()
+    })
+
     it('passes --browser option', () => {
       const args = run.processRunOptions({
         browser: 'test browser',

packages/server/lib/util/args.js

@@ -34,17 +34,27 @@ const normalizeBackslash = (s) => {
   return s
 }
 
+/**
+ * remove stray double quote from runProject and other path properties
+ * due to bug in NPM passing arguments with backslash at the end
+ * @see https://github.com/cypress-io/cypress/issues/535
+ *
+ */
 const normalizeBackslashes = (options) => {
-  // remove stray double quote from runProject and other path properties
-  // due to bug in NPM passing arguments with
-  // backslash at the end
-  // https://github.com/cypress-io/cypress/issues/535
   // these properties are paths and likely to have backslash on Windows
   const pathProperties = ['runProject', 'project', 'appPath', 'execPath', 'configFile']
 
   pathProperties.forEach((property) => {
-    if (options[property]) {
+    // sometimes a string parameter might get parsed into a boolean
+    // for example "--project ''" will be transformed in "project: true"
+    // which we should treat as undefined
+    if (typeof options[property] === 'string') {
       options[property] = normalizeBackslash(options[property])
+    } else {
+      // configFile is a special case that can be set to false
+      if (property !== 'configFile') {
+        delete options[property]
+      }
     }
   })
 
@@ -148,6 +158,8 @@ const sanitizeAndConvertNestedArgs = (str, argname) => {
 }
 
 module.exports = {
+  normalizeBackslashes,
+
   toObject (argv) {
     debug('argv array: %o', argv)
 
@@ -210,7 +222,12 @@ module.exports = {
 
     let { spec } = options
     const { env, config, reporterOptions, outputPath, tag } = options
-    const project = options.project || options.runProject
+    let project = options.project || options.runProject
+
+    // only accept project if it is a string
+    if (typeof project !== 'string') {
+      project = undefined
+    }
 
     if (spec) {
       const resolvePath = (p) => {

packages/server/test/unit/args_spec.js

@@ -24,13 +24,56 @@ describe('lib/util/args', () => {
     })
   })
 
+  context('normalizeBackslashes', () => {
+    it('sets non-string properties to undefined', () => {
+      const input = {
+        // string properties
+        project: true,
+        appPath: '/foo/bar',
+        // this option can be string or false
+        configFile: false,
+        // unknown properties will be preserved
+        somethingElse: 42,
+      }
+      const output = argsUtil.normalizeBackslashes(input)
+
+      expect(output).to.deep.equal({
+        appPath: '/foo/bar',
+        configFile: false,
+        somethingElse: 42,
+      })
+    })
+
+    it('handles empty project path string', () => {
+      const input = {
+        project: '',
+      }
+      const output = argsUtil.normalizeBackslashes(input)
+
+      // empty project path remains
+      expect(output).to.deep.equal(input)
+    })
+  })
+
   context('--project', () => {
     it('sets projectRoot', function () {
       const projectRoot = path.resolve(cwd, './foo/bar')
       const options = this.setup('--project', './foo/bar')
 
       expect(options.projectRoot).to.eq(projectRoot)
     })
+
+    it('is undefined if not specified', function () {
+      const options = this.setup()
+
+      expect(options.projectRoot).to.eq(undefined)
+    })
+
+    it('handles bool project parameter', function () {
+      const options = this.setup('--project', true)
+
+      expect(options.projectRoot).to.eq(undefined)
+    })
   })
 
   context('--run-project', () => {