Show warning when binary is run directly (outside npm module) (#4701)

* commit previous changes made by @bahmutov for #1573

* Pass '--run-from-cli' flag through ping test in order to prevent warning from printing

* woops, require 'argsUntil'

* 'headed' was changed to 'interactiveMode'

* fix duplicate misspelled require

* tighten up args utils and cleanup decaf garbage

* cleanup errors.log to take a cb and conditionally be async

* remove extraneous --run-from-cli argument, consolidate to use --cli

- update tests

* fixes tests, ensure that record.createRun() always returns a promise

* refactor tests to handle new errors.logException interface

* make logException always return a promise, cleanup interface, add test

* fix linting errors


Co-authored-by: Brian Mann <brian.mann86@gmail.com>
This commit is contained in:
Jennifer Shehane
2019-07-29 18:03:33 +06:30
committed by Brian Mann
parent d73d350cfb
commit a7dfda9865
13 changed files with 260 additions and 152 deletions
+2
View File
@@ -73,6 +73,8 @@ module.exports = {
debug('needs to start own Xvfb?', needsXvfb)
// always push cwd into the args
// which additionally acts as a signal to the
// binary that it was invoked through the NPM module
args = [].concat(args, '--cwd', process.cwd())
_.defaults(options, {
-1
View File
@@ -27,7 +27,6 @@
"cypress:verify": "node ./cli/bin/cypress verify --dev",
"predecaffeinate-bulk": "shx cp .eslintrc.decaffeinate.js .eslintrc.js",
"decaffeinate-bulk": "bulk-decaffeinate",
"postdecaffeinate-bulk": "shx rm .eslintrc.js",
"dev": "node ./scripts/start.js",
"dev-debug": "node ./scripts/debug.js dev",
"docker": "./scripts/run-docker-local.sh",
+9 -1
View File
@@ -16,6 +16,9 @@ path = require("path")
Promise = require("bluebird")
debug = require("debug")("cypress:server:cypress")
warning = (code) ->
require("./errors").warning(code)
exit = (code = 0) ->
## TODO: we shouldn't have to do this
## but cannot figure out how null is
@@ -32,7 +35,7 @@ exitErr = (err) ->
## and exit with 1
debug('exiting with err', err)
require("./errors").log(err)
require("./errors").logException(err)
.then -> exit(1)
module.exports = {
@@ -49,6 +52,11 @@ module.exports = {
## like in production and we shouldn't spawn a new
## process
if @isCurrentlyRunningElectron()
## if we weren't invoked from the CLI
## then display a warning to the user
if not options.invokedFromCli
warning("INVOKED_BINARY_OUTSIDE_NPM_MODULE")
## just run the gui code directly here
## and pass our options directly to main
require("./modes")(mode, options)
+35 -12
View File
@@ -6,6 +6,9 @@ Promise = require("bluebird")
twoOrMoreNewLinesRe = /\n{2,}/
isProduction = ->
process.env["CYPRESS_ENV"] is "production"
listItems = (paths) ->
_
.chain(paths)
@@ -690,6 +693,16 @@ getMsgByType = (type, arg1 = {}, arg2) ->
We looked but did not find a #{chalk.blue('cypress.json')} file in this folder: #{chalk.blue(arg1)}
"""
when "INVOKED_BINARY_OUTSIDE_NPM_MODULE"
"""
It looks like you are running the Cypress binary directly.
This is not the recommended approach, and Cypress may not work correctly.
Please install the 'cypress' NPM package and follow the instructions here:
https://on.cypress.io/installing-cypress
"""
when "DUPLICATE_TASK_KEY"
"""
Warning: Multiple attempts to register the following task(s): #{chalk.blue(arg1)}. Only the last attempt will be registered.
@@ -833,6 +846,8 @@ get = (type, arg1, arg2) ->
warning = (type, arg1, arg2) ->
err = get(type, arg1, arg2)
log(err, "magenta")
return null
throwErr = (type, arg1, arg2) ->
throw get(type, arg1, arg2)
@@ -860,27 +875,35 @@ clone = (err, options = {}) ->
obj
log = (err, color = "red") ->
Promise.try ->
console.log chalk[color](err.message)
if err.details
console.log("\n", chalk["yellow"](err.details))
console.log chalk[color](err.message)
## bail if this error came from known
## list of Cypress errors
return if isCypressErr(err)
if err.details
console.log("\n", chalk["yellow"](err.details))
console.log chalk[color](err.stack)
## bail if this error came from known
## list of Cypress errors
return if isCypressErr(err)
if process.env["CYPRESS_ENV"] is "production"
## log this error to raygun since its not
## a known error
require("./logger").createException(err).catch(->)
console.log chalk[color](err.stack)
return err
logException = Promise.method (err) ->
## TODO: remove context here
if @log(err) and isProduction()
## log this exception since
## its not a known error
return require("./logger")
.createException(err)
.catch(->)
module.exports = {
get,
log,
logException,
clone,
warning,
+1 -1
View File
@@ -247,7 +247,7 @@ billingLink = (orgId) ->
gracePeriodMessage = (gracePeriodEnds) ->
gracePeriodEnds or "the grace period ends"
createRun = (options = {}) ->
createRun = Promise.method (options = {}) ->
_.defaults(options, {
group: null,
parallel: null,
+94 -102
View File
@@ -1,24 +1,10 @@
/* eslint-disable
brace-style,
no-cond-assign,
no-unused-vars,
one-var,
*/
// TODO: This file was created by bulk-decaffeinate.
// Fix any style issues and re-enable lint.
/*
* decaffeinate suggestions:
* DS102: Remove unnecessary code created because of implicit returns
* DS207: Consider shorter variations of null checks
* Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
*/
const _ = require('lodash')
const path = require('path')
const debug = require('debug')('cypress:server:args')
const minimist = require('minimist')
const coerce = require('./coerce')
const config = require('../config')
const proxy = require('./proxy')
const coerceUtil = require('./coerce')
const configUtil = require('../config')
const proxyUtil = require('./proxy')
const nestedObjectsInCurlyBracesRe = /\{(.+?)\}/g
const nestedArraysInSquareBracketsRe = /\[(.+?)\]/g
@@ -48,10 +34,10 @@ const normalizeBackslash = function (s) {
}
const normalizeBackslashes = function (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
// 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']
@@ -73,25 +59,23 @@ const stringify = function (val) {
}
const strToArray = function (str) {
let parsed
const parsed = tryJSONParse(str)
if (parsed = tryJSONParse(str)) {
if (parsed) {
return parsed
}
return [].concat(str.split(','))
}
const commasToPipes = (match, p1, p2, p3) =>
//# swap out comma's for bars
{
// swap out comma's for bars
const commasToPipes = (match, p1, p2, p3) => {
return match.split(',').join('|')
}
const pipesToCommas = (str) =>
//# convert foo=bar|version=1.2.3 to
//# foo=bar,version=1.2.3
{
// convert foo=bar|version=1.2.3 to
// foo=bar,version=1.2.3
const pipesToCommas = (str) => {
return str.split('|').join(',')
}
@@ -104,39 +88,41 @@ const tryJSONParse = function (str) {
}
const JSONOrCoerce = function (str) {
//# valid JSON? horray
let parsed
// valid JSON? horray
const parsed = tryJSONParse(str)
if (parsed = tryJSONParse(str)) {
if (parsed) {
return parsed
}
//# convert bars back to commas
// convert bars back to commas
str = pipesToCommas(str)
//# try to parse again?
if (parsed = tryJSONParse(str)) {
return parsed
// try to parse again?
const parsed2 = tryJSONParse(str)
if (parsed2) {
return parsed2
}
//# nupe :-(
return coerce(str)
// nupe :-(
return coerceUtil(str)
}
const sanitizeAndConvertNestedArgs = function (str) {
//# if this is valid JSON then just
//# parse it and call it a day
let parsed
// if this is valid JSON then just
// parse it and call it a day
const parsed = tryJSONParse(str)
if (parsed = tryJSONParse(str)) {
if (parsed) {
return parsed
}
//# invalid JSON, so assume mixed usage
//# first find foo={a:b,b:c} and bar=[1,2,3]
//# syntax and turn those into
//# foo: a:b|b:c
//# bar: 1|2|3
// invalid JSON, so assume mixed usage
// first find foo={a:b,b:c} and bar=[1,2,3]
// syntax and turn those into
// foo: a:b|b:c
// bar: 1|2|3
return _
.chain(str)
@@ -153,63 +139,69 @@ const sanitizeAndConvertNestedArgs = function (str) {
module.exports = {
toObject (argv) {
let c, envs, op, p, ro, spec
debug('argv array: %o', argv)
const alias = {
'app-path': 'appPath',
'exec-path': 'execPath',
'api-key': 'apiKey',
'smoke-test': 'smokeTest',
'app-path': 'appPath',
'ci-build-id': 'ciBuildId',
'clear-logs': 'clearLogs',
'exec-path': 'execPath',
'exit-with-code': 'exitWithCode',
'inspect-brk': 'inspectBrk',
'get-key': 'getKey',
'new-key': 'generateKey',
'clear-logs': 'clearLogs',
'run-project': 'runProject',
'output-path': 'outputPath',
'proxy-source': 'proxySource',
'reporter-options': 'reporterOptions',
'return-pkg': 'returnPkg',
'run-mode': 'isTextTerminal',
'ci-build-id': 'ciBuildId',
'exit-with-code': 'exitWithCode',
'reporter-options': 'reporterOptions',
'output-path': 'outputPath',
'inspect-brk': 'inspectBrk',
'proxy-source': 'proxySource',
'run-project': 'runProject',
'smoke-test': 'smokeTest',
}
//# takes an array of args and converts
//# to an object
// takes an array of args and converts
// to an object
let options = minimist(argv, {
alias,
})
const whitelisted = _.pick(argv, whitelist)
// were we invoked from the CLI or directly?
const invokedFromCli = Boolean(options.cwd)
options = _
.chain(options)
.defaults(whitelisted)
.omit(_.keys(alias)) //# remove aliases
.omit(_.keys(alias)) // remove aliases
.extend({ invokedFromCli })
.defaults({
//# set in case we
//# bypassed the cli
// set in case we
// bypassed the cli
cwd: process.cwd(),
})
.mapValues(coerce)
.mapValues(coerceUtil)
.value()
debug('argv parsed: %o', options)
//# if we are updating we may have to pluck out the
//# appPath + execPath from the options._ because
//# in previous versions up until 0.14.0 these args
//# were not passed as dashes and instead were just
//# regular arguments
// if we are updating we may have to pluck out the
// appPath + execPath from the options._ because
// in previous versions up until 0.14.0 these args
// were not passed as dashes and instead were just
// regular arguments
if (options.updating && !options.appPath) {
//# take the last two arguments that were unknown
//# and apply them to both appPath + execPath
// take the last two arguments that were unknown
// and apply them to both appPath + execPath
[options.appPath, options.execPath] = options._.slice(-2)
}
if (spec = options.spec) {
let { spec } = options
const { env, config, reporterOptions, outputPath } = options
const project = options.project || options.runProject
if (spec) {
const resolvePath = (p) => {
return path.resolve(options.cwd, p)
}
@@ -223,11 +215,11 @@ module.exports = {
options.spec = strToArray(spec).map(resolvePath)
}
if (envs = options.env) {
options.env = sanitizeAndConvertNestedArgs(envs)
if (env) {
options.env = sanitizeAndConvertNestedArgs(env)
}
const proxySource = proxy.loadSystemProxySettings()
const proxySource = proxyUtil.loadSystemProxySettings()
if (process.env.HTTP_PROXY) {
if (proxySource) {
@@ -238,46 +230,46 @@ module.exports = {
options.proxyBypassList = process.env.NO_PROXY
}
if (ro = options.reporterOptions) {
options.reporterOptions = sanitizeAndConvertNestedArgs(ro)
if (reporterOptions) {
options.reporterOptions = sanitizeAndConvertNestedArgs(reporterOptions)
}
if (c = options.config) {
//# convert config to an object
//# annd store the config
options.config = sanitizeAndConvertNestedArgs(c)
if (config) {
// convert config to an object
// annd store the config
options.config = sanitizeAndConvertNestedArgs(config)
}
//# get a list of the available config keys
const configKeys = config.getConfigKeys()
// get a list of the available config keys
const configKeys = configUtil.getConfigKeys()
//# and if any of our options match this
// and if any of our options match this
const configValues = _.pick(options, configKeys)
//# then set them on config
//# this solves situations where we accept
//# root level arguments which also can
//# be set in configuration
// then set them on config
// this solves situations where we accept
// root level arguments which also can
// be set in configuration
if (options.config == null) {
options.config = {}
}
_.extend(options.config, configValues)
//# remove them from the root options object
// remove them from the root options object
options = _.omit(options, configKeys)
options = normalizeBackslashes(options)
debug('options %o', options)
//# normalize project to projectRoot
if (p = options.project || options.runProject) {
options.projectRoot = path.resolve(options.cwd, p)
// normalize project to projectRoot
if (project) {
options.projectRoot = path.resolve(options.cwd, project)
}
//# normalize output path from previous current working directory
if (op = options.outputPath) {
options.outputPath = path.resolve(options.cwd, op)
// normalize output path from previous current working directory
if (outputPath) {
options.outputPath = path.resolve(options.cwd, outputPath)
}
if (options.runProject) {
@@ -294,10 +286,10 @@ module.exports = {
},
toArray (obj = {}) {
//# goes in reverse, takes an object
//# and converts to an array by picking
//# only the whitelisted properties and
//# mapping them to include the argument
// goes in reverse, takes an object
// and converts to an array by picking
// only the whitelisted properties and
// mapping them to include the argument
return _
.chain(obj)
.pick(...whitelist)
@@ -1470,4 +1470,3 @@ describe "e2e record", ->
snapshot: true
expectedExitCode: 0
})
@@ -32,7 +32,7 @@ describe "CLI Interface", ->
@currentTest.timeout(20000)
it "writes out ping value and exits", (done) ->
cp.exec "npm run dev -- --smoke-test --ping=12345", {env: env}, (err, stdout, stderr) ->
cp.exec "npm run dev -- --cwd=/foo/bar --smoke-test --ping=12345", {env: env}, (err, stdout, stderr) ->
done(err) if err
expect(clean(stdout)).to.eq("12345")
@@ -15,6 +15,7 @@ stripAnsi = require("strip-ansi")
pkg = require("@packages/root")
launcher = require("@packages/launcher")
extension = require("@packages/extension")
argsUtil = require("#{root}lib/util/args")
fs = require("#{root}lib/util/fs")
ciProvider = require("#{root}lib/util/ci_provider")
settings = require("#{root}lib/util/settings")
@@ -118,8 +119,13 @@ describe "lib/cypress", ->
sinon.stub(launcher, "detect").resolves(TYPICAL_BROWSERS)
sinon.stub(process, "exit")
sinon.stub(Server.prototype, "reset")
sinon.stub(errors, "warning")
.callThrough()
.withArgs("INVOKED_BINARY_OUTSIDE_NPM_MODULE")
.returns(null)
sinon.spy(errors, "log")
sinon.spy(errors, "warning")
sinon.spy(errors, "logException")
sinon.spy(console, "log")
@expectExitWith = (code) =>
@@ -925,9 +931,10 @@ describe "lib/cypress", ->
sinon.stub(env, "get").withArgs("CYPRESS_PROJECT_ID").returns(@projectId)
cypress.start([
"--cwd=/foo/bar"
"--run-project=#{@noScaffolding}",
"--record",
"--key=token-123"
"--key=token-123",
])
.then =>
expect(api.createRun).to.be.calledWithMatch({projectId: @projectId})
@@ -940,8 +947,9 @@ describe "lib/cypress", ->
.withArgs("CYPRESS_RECORD_KEY").returns("token")
cypress.start([
"--cwd=/foo/bar"
"--run-project=#{@noScaffolding}",
"--record"
"--record",
])
.then =>
expect(api.createRun).to.be.calledWithMatch({
@@ -1388,6 +1396,23 @@ describe "lib/cypress", ->
.then ->
expect(event.sender.send.withArgs("response").firstCall.args[1].data).to.eql(warning)
context "--cwd", ->
beforeEach ->
errors.warning.restore()
sinon.stub(electron.app, "on").withArgs("ready").yieldsAsync()
sinon.stub(interactiveMode, "ready").resolves()
sinon.spy(errors, "warning")
it "shows warning if Cypress has been started directly", ->
cypress.start().then ->
expect(errors.warning).to.be.calledWith("INVOKED_BINARY_OUTSIDE_NPM_MODULE")
expect(console.log).to.be.calledWithMatch("It looks like you are running the Cypress binary directly.")
expect(console.log).to.be.calledWithMatch("https://on.cypress.io/installing-cypress")
it "does not show warning if finds --cwd", ->
cypress.start(["--cwd=/foo/bar"]).then ->
expect(errors.warning).not.to.be.called
context "no args", ->
beforeEach ->
sinon.stub(electron.app, "on").withArgs("ready").yieldsAsync()
@@ -252,7 +252,11 @@ module.exports = {
return options
args: (options = {}) ->
args = ["--run-project=#{options.project}"]
args = [
## hides a user warning to go through NPM module
"--cwd=#{process.cwd()}",
"--run-project=#{options.project}"
]
if options.spec
args.push("--spec=#{options.spec}")
@@ -222,6 +222,7 @@ describe "lib/util/args", ->
cwd
_: []
getKey: true
invokedFromCli: false
config: @config
spec: @specs
})
@@ -250,6 +251,7 @@ describe "lib/util/args", ->
cwd
_: []
getKey: true
invokedFromCli: true
config: @config
spec: @specs
})
@@ -276,6 +278,7 @@ describe "lib/util/args", ->
config: {}
appPath: "/Applications/Cypress.app"
execPath: "/Applications/Cypress.app"
invokedFromCli: false
updating: true
})
@@ -299,6 +302,7 @@ describe "lib/util/args", ->
config: {}
appPath: "a"
execPath: "e"
invokedFromCli: false
updating: true
})
+78 -27
View File
@@ -7,55 +7,106 @@ logger = require("#{root}lib/logger")
describe "lib/errors", ->
beforeEach ->
@env = process.env.CYPRESS_ENV
@log = sinon.stub(console, "log")
afterEach ->
process.env.CYPRESS_ENV = @env
sinon.spy(console, "log")
context ".log", ->
it "uses red by default", ->
err = errors.get("NOT_LOGGED_IN")
errors.log(err).then =>
red = style.red
ret = errors.log(err)
expect(@log).to.be.calledWithMatch(red.open)
expect(@log).to.be.calledWithMatch(red.close)
expect(ret).to.be.undefined
red = style.red
expect(console.log).to.be.calledWithMatch(red.open)
expect(console.log).to.be.calledWithMatch(red.close)
it "can change the color", ->
err = errors.get("NOT_LOGGED_IN")
errors.log(err, "yellow").then =>
yellow = style.yellow
ret = errors.log(err, "yellow")
expect(@log).to.be.calledWithMatch(yellow.open)
expect(@log).to.be.calledWithMatch(yellow.close)
expect(ret).to.be.undefined
yellow = style.yellow
expect(console.log).to.be.calledWithMatch(yellow.open)
expect(console.log).to.be.calledWithMatch(yellow.close)
it "logs err.message", ->
err = errors.get("NO_PROJECT_ID", "foo/bar/baz")
errors.log(err).then =>
expect(@log).to.be.calledWithMatch("foo/bar/baz")
ret = errors.log(err)
expect(ret).to.be.undefined
expect(console.log).to.be.calledWithMatch("foo/bar/baz")
it "logs err.details", ->
err = errors.get("PLUGINS_FUNCTION_ERROR", "foo/bar/baz", "details huh")
errors.log(err).then =>
expect(@log).to.be.calledWithMatch("foo/bar/baz")
expect(@log).to.be.calledWithMatch("\n", "details huh")
ret = errors.log(err)
expect(ret).to.be.undefined
expect(console.log).to.be.calledWithMatch("foo/bar/baz")
expect(console.log).to.be.calledWithMatch("\n", "details huh")
it "logs err.stack in development", ->
process.env.CYPRESS_ENV = "development"
foo = new Error("foo")
errors.log(foo).then =>
expect(@log).to.be.calledWith(chalk.red(foo.stack))
err = new Error("foo")
ret = errors.log(err)
it "calls logger.createException", ->
expect(ret).to.eq(err)
expect(console.log).to.be.calledWith(chalk.red(err.stack))
context ".logException", ->
it "calls logger.createException with unknown error", ->
sinon.stub(logger, "createException").resolves()
sinon.stub(process.env, "CYPRESS_ENV").value("production")
process.env.CYPRESS_ENV = "production"
err = new Error("foo")
foo = new Error("foo")
errors.log(foo).then =>
expect(logger.createException).to.be.calledWith(foo)
errors.logException(err)
.then ->
expect(console.log).to.be.calledWith(chalk.red(err.stack))
expect(logger.createException).to.be.calledWith(err)
it "does not call logger.createException when known error", ->
sinon.stub(logger, "createException").resolves()
sinon.stub(process.env, "CYPRESS_ENV").value("production")
err = errors.get("NOT_LOGGED_IN")
errors.logException(err)
.then ->
expect(console.log).not.to.be.calledWith(err.stack)
expect(logger.createException).not.to.be.called
it "does not call logger.createException when not in production env", ->
sinon.stub(logger, "createException").resolves()
sinon.stub(process.env, "CYPRESS_ENV").value("development")
err = new Error("foo")
errors.logException(err)
.then ->
expect(console.log).not.to.be.calledWith(err.stack)
expect(logger.createException).not.to.be.called
it "swallows creating exception errors", ->
sinon.stub(logger, "createException").rejects(new Error("foo"))
sinon.stub(process.env, "CYPRESS_ENV").value("production")
err = errors.get("NOT_LOGGED_IN")
errors.logException(err)
.then (ret) ->
expect(ret).to.be.undefined
context ".clone", ->
it "converts err.message from ansi to html with span classes", ->
@@ -22,7 +22,7 @@ initialEnv = _.clone(process.env)
describe "lib/modes/record", ->
## QUESTION: why are these tests here when
## this is a module... ?
context "getCommitFromGitOrCi", ->
context ".getCommitFromGitOrCi", ->
gitCommit = {
branch: null
}
@@ -372,7 +372,8 @@ describe "lib/modes/record", ->
sinon.stub(api, "retryWithBackoff").yields().resolves()
recordMode.createRun(@options)
expect(api.retryWithBackoff).to.be.called
.then ->
expect(api.retryWithBackoff).to.be.called
it "logs on retry", ->
sinon.stub(api, "retryWithBackoff").yields().resolves()