From 084f25f0927cb1da2780e4e8697338c0ddc5db25 Mon Sep 17 00:00:00 2001 From: Gleb Bahmutov Date: Thu, 19 Sep 2019 13:49:57 -0400 Subject: [PATCH 1/5] cli: fix the STDIN pipe on Windows (#5045) * cli: pipe stdin * uggh, here is the actual change * update cli unit tests * add unit test --- cli/lib/exec/spawn.js | 20 +++++++++++++++++--- cli/test/lib/exec/spawn_spec.js | 8 +++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/cli/lib/exec/spawn.js b/cli/lib/exec/spawn.js index dbb9a6507473..2f235f418a68 100644 --- a/cli/lib/exec/spawn.js +++ b/cli/lib/exec/spawn.js @@ -135,12 +135,25 @@ module.exports = { child.on('close', resolve) child.on('error', reject) - child.stdin && child.stdin.pipe(process.stdin) - child.stdout && child.stdout.pipe(process.stdout) + // if stdio options is set to 'pipe', then + // we should set up pipes: + // process STDIN (read stream) => child STDIN (writeable) + // child STDOUT => process STDOUT + // child STDERR => process STDERR with additional filtering + if (child.stdin) { + debug('piping process STDIN into child STDIN') + process.stdin.pipe(child.stdin) + } + + if (child.stdout) { + debug('piping child STDOUT to process STDOUT') + child.stdout.pipe(process.stdout) + } // if this is defined then we are manually piping for linux // to filter out the garbage - child.stderr && + if (child.stderr) { + debug('piping child STDERR to process STDERR') child.stderr.on('data', (data) => { const str = data.toString() @@ -158,6 +171,7 @@ module.exports = { // else pass it along! process.stderr.write(data) }) + } // https://github.com/cypress-io/cypress/issues/1841 // In some versions of node, it will throw on windows diff --git a/cli/test/lib/exec/spawn_spec.js b/cli/test/lib/exec/spawn_spec.js index 75f46650ef5b..9cfd1e64d1b3 100644 --- a/cli/test/lib/exec/spawn_spec.js +++ b/cli/test/lib/exec/spawn_spec.js @@ -39,7 +39,10 @@ describe('lib/exec/spawn', function () { }, } - sinon.stub(process, 'stdin').value(new EE) + // process.stdin is both an event emitter and a readable stream + this.processStdin = new EE() + this.processStdin.pipe = sinon.stub().returns(undefined) + sinon.stub(process, 'stdin').value(this.processStdin) sinon.stub(cp, 'spawn').returns(this.spawnedProcess) sinon.stub(xvfb, 'start').resolves() sinon.stub(xvfb, 'stop').resolves() @@ -292,6 +295,9 @@ describe('lib/exec/spawn', function () { return spawn.start() .then(() => { expect(cp.spawn.firstCall.args[2].stdio).to.deep.eq('pipe') + // parent process STDIN was piped to child process STDIN + expect(this.processStdin.pipe, 'process.stdin').to.have.been.calledOnce + .and.to.have.been.calledWith(this.spawnedProcess.stdin) }) }) From 7100a419f84c2435e3109ad786aaa1c53c6c36dc Mon Sep 17 00:00:00 2001 From: Jim Jazwiecki Date: Fri, 20 Sep 2019 10:27:55 -0400 Subject: [PATCH 2/5] =?UTF-8?q?more=20permissive=20check=20for=20json=20to?= =?UTF-8?q?=20include=20application/vnd.api+j=E2=80=A6=20(#5166)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * more permissive check for json to include * add json test for content-type application/vnd.api+json * cruder solution passes e2e tests locally, so let's go with that * Remove 'charset' from content-type before checking if JSON --- packages/server/lib/request.coffee | 3 ++- packages/server/test/unit/request_spec.coffee | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/server/lib/request.coffee b/packages/server/lib/request.coffee index 543a02c2b2ca..1cbab199a46a 100644 --- a/packages/server/lib/request.coffee +++ b/packages/server/lib/request.coffee @@ -483,7 +483,8 @@ module.exports = (options = {}) -> contentTypeIsJson: (response) -> ## TODO: use https://github.com/jshttp/type-is for this - response?.headers?["content-type"]?.includes("application/json") + ## https://github.com/cypress-io/cypress/pull/5166 + response?.headers?["content-type"]?.split(';', 2)[0].endsWith("json") parseJsonBody: (body) -> try diff --git a/packages/server/test/unit/request_spec.coffee b/packages/server/test/unit/request_spec.coffee index 2352ee0bdda2..e9b85490d0ea 100644 --- a/packages/server/test/unit/request_spec.coffee +++ b/packages/server/test/unit/request_spec.coffee @@ -449,6 +449,20 @@ describe "lib/request", -> .then (resp) -> expect(resp.body).to.deep.eq({status: "ok"}) + it "parses response body as json if content-type application/vnd.api+json response headers", -> + nock("http://localhost:8080") + .get("/status.json") + .reply(200, JSON.stringify({status: "ok"}), { + "Content-Type": "application/vnd.api+json" + }) + + request.sendPromise({}, @fn, { + url: "http://localhost:8080/status.json" + cookies: false + }) + .then (resp) -> + expect(resp.body).to.deep.eq({status: "ok"}) + it "revives from parsing bad json", -> nock("http://localhost:8080") .get("/status.json") From b5b79fc9afa4a3b9377283d4db4ae3725c1d7853 Mon Sep 17 00:00:00 2001 From: Jennifer Shehane Date: Fri, 20 Sep 2019 16:52:22 -0400 Subject: [PATCH 3/5] fix eslint for fixture specs (#5176) * update eslint to lint files within 'fixtures' in support files - ignore some edge cases like jquery, jsx and obvious js files we wrote with broken code * Fixes from eslint to 'fixtures' files --- .eslintignore | 7 ++++++- .../busted-support-file/cypress/support/index.js | 2 +- .../cypress/integration/default_layout_spec.js | 1 + .../browserify_babel_es2015_failing_spec.js | 2 +- .../cypress/integration/https_passthru_spec.js | 1 + .../integration/network_error_handling_spec.js | 2 ++ .../projects/e2e/cypress/plugins/index.js | 4 +++- .../projects/e2e/cypress/support/foo/bar.js | 3 ++- .../support/fixtures/projects/e2e/lib/bar.js | 4 ++-- .../support/fixtures/projects/e2e/lib/baz.js | 4 ++-- .../fixtures/projects/e2e/reporters/custom.js | 1 + .../projects/ids/cypress/integration/bar.js | 4 ++-- .../projects/ids/cypress/integration/baz.js | 2 +- .../cypress/plugins/index.js | 16 ++++++++++++---- .../projects/no-server/helpers/includes.js | 4 ++-- .../projects/no-server/my-tests/test1.js | 5 +++-- .../projects/plugin-extension/ext/background.js | 2 -- .../projects/plugin-extension/ext/manifest.json | 12 +++++++++--- .../plugins-async-error/cypress/plugins/index.js | 2 -- .../fixtures/projects/todos/tests/test1.js | 5 +++-- .../cypress/integration/another_spec.js | 3 +-- 21 files changed, 55 insertions(+), 31 deletions(-) diff --git a/.eslintignore b/.eslintignore index c50ce0052813..45d0e83020f6 100644 --- a/.eslintignore +++ b/.eslintignore @@ -7,7 +7,12 @@ **/dist **/dist-test **/node_modules -**/support/fixtures +**/support/fixtures/* +!**/support/fixtures/projects +**/support/fixtures/projects/**/_fixtures/* +**/support/fixtures/projects/**/*.jsx +**/support/fixtures/projects/**/jquery.js +**/support/fixtures/projects/**/fail.js **/test/fixtures **/vendor diff --git a/packages/server/test/support/fixtures/projects/busted-support-file/cypress/support/index.js b/packages/server/test/support/fixtures/projects/busted-support-file/cypress/support/index.js index b634e9380711..fbab6df357e4 100644 --- a/packages/server/test/support/fixtures/projects/busted-support-file/cypress/support/index.js +++ b/packages/server/test/support/fixtures/projects/busted-support-file/cypress/support/index.js @@ -1 +1 @@ -import "./does/not/exist" \ No newline at end of file +import './does/not/exist' diff --git a/packages/server/test/support/fixtures/projects/default-layout/cypress/integration/default_layout_spec.js b/packages/server/test/support/fixtures/projects/default-layout/cypress/integration/default_layout_spec.js index e2fbbbe9ab71..7e5e612bae4e 100644 --- a/packages/server/test/support/fixtures/projects/default-layout/cypress/integration/default_layout_spec.js +++ b/packages/server/test/support/fixtures/projects/default-layout/cypress/integration/default_layout_spec.js @@ -1 +1,2 @@ +/* eslint-disable mocha/no-global-tests */ it('works', () => {}) diff --git a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/browserify_babel_es2015_failing_spec.js b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/browserify_babel_es2015_failing_spec.js index 16a6f89e5e57..8bcc6576e58b 100644 --- a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/browserify_babel_es2015_failing_spec.js +++ b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/browserify_babel_es2015_failing_spec.js @@ -1 +1 @@ -import "../../lib/fail" \ No newline at end of file +import '../../lib/fail' diff --git a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/https_passthru_spec.js b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/https_passthru_spec.js index ae3a83c6480d..b7315852265f 100644 --- a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/https_passthru_spec.js +++ b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/https_passthru_spec.js @@ -23,6 +23,7 @@ describe('https passthru retries', () => { img.onload = () => { reject(new Error('onload event fired, but should not have. expected onerror to fire.')) } + img.onerror = resolve }) }) diff --git a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/network_error_handling_spec.js b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/network_error_handling_spec.js index b62f31b30c8c..318ddc13ae52 100644 --- a/packages/server/test/support/fixtures/projects/e2e/cypress/integration/network_error_handling_spec.js +++ b/packages/server/test/support/fixtures/projects/e2e/cypress/integration/network_error_handling_spec.js @@ -27,8 +27,10 @@ describe('network error handling', function () { }) .get('input[type=text]') .type('bar') + cy.get('input[type=submit]') .click() + cy.contains('{"foo":"bar"}') }) }) diff --git a/packages/server/test/support/fixtures/projects/e2e/cypress/plugins/index.js b/packages/server/test/support/fixtures/projects/e2e/cypress/plugins/index.js index ed471c9b69e3..2d920c90a544 100644 --- a/packages/server/test/support/fixtures/projects/e2e/cypress/plugins/index.js +++ b/packages/server/test/support/fixtures/projects/e2e/cypress/plugins/index.js @@ -101,6 +101,7 @@ module.exports = (on) => { 'record:fast_visit_spec' ({ percentiles, url, browser, currentRetry }) { percentiles.forEach(([percent, percentile]) => { + // eslint-disable-next-line no-console console.log(`${percent}%\t of visits to ${url} finished in less than ${percentile}ms`) }) @@ -110,8 +111,9 @@ module.exports = (on) => { currentRetry, ...percentiles.reduce((acc, pair) => { acc[pair[0]] = pair[1] + return acc - }, {}) + }, {}), } return performance.track('fast_visit_spec percentiles', data) diff --git a/packages/server/test/support/fixtures/projects/e2e/cypress/support/foo/bar.js b/packages/server/test/support/fixtures/projects/e2e/cypress/support/foo/bar.js index be19a41deb62..80446ed767db 100644 --- a/packages/server/test/support/fixtures/projects/e2e/cypress/support/foo/bar.js +++ b/packages/server/test/support/fixtures/projects/e2e/cypress/support/foo/bar.js @@ -1 +1,2 @@ -console.log("bar") \ No newline at end of file +/* eslint-disable no-console */ +console.log('bar') diff --git a/packages/server/test/support/fixtures/projects/e2e/lib/bar.js b/packages/server/test/support/fixtures/projects/e2e/lib/bar.js index b6a1e32eb015..fec0747a32df 100644 --- a/packages/server/test/support/fixtures/projects/e2e/lib/bar.js +++ b/packages/server/test/support/fixtures/projects/e2e/lib/bar.js @@ -1,3 +1,3 @@ -import baz from "./baz" +import baz from './baz' -export default baz \ No newline at end of file +export default baz diff --git a/packages/server/test/support/fixtures/projects/e2e/lib/baz.js b/packages/server/test/support/fixtures/projects/e2e/lib/baz.js index 1ddc68846578..72796455a560 100644 --- a/packages/server/test/support/fixtures/projects/e2e/lib/baz.js +++ b/packages/server/test/support/fixtures/projects/e2e/lib/baz.js @@ -1,3 +1,3 @@ export default () => { - return "baz" -} \ No newline at end of file + return 'baz' +} diff --git a/packages/server/test/support/fixtures/projects/e2e/reporters/custom.js b/packages/server/test/support/fixtures/projects/e2e/reporters/custom.js index b8577e357f5e..af6bdba91c33 100644 --- a/packages/server/test/support/fixtures/projects/e2e/reporters/custom.js +++ b/packages/server/test/support/fixtures/projects/e2e/reporters/custom.js @@ -1,3 +1,4 @@ +/* eslint-disable no-console */ module.exports = function Reporter (runner) { runner.on('test end', function (test) { console.log(test.title) diff --git a/packages/server/test/support/fixtures/projects/ids/cypress/integration/bar.js b/packages/server/test/support/fixtures/projects/ids/cypress/integration/bar.js index 62dbdba0395b..1b64ff8bcc23 100644 --- a/packages/server/test/support/fixtures/projects/ids/cypress/integration/bar.js +++ b/packages/server/test/support/fixtures/projects/ids/cypress/integration/bar.js @@ -1,3 +1,3 @@ -context("some context[i9w]", function(){ +context('some context[i9w]', function () { it('tests[abc]') -}) \ No newline at end of file +}) diff --git a/packages/server/test/support/fixtures/projects/ids/cypress/integration/baz.js b/packages/server/test/support/fixtures/projects/ids/cypress/integration/baz.js index fb5b944d2aac..fd32a7c442d5 100644 --- a/packages/server/test/support/fixtures/projects/ids/cypress/integration/baz.js +++ b/packages/server/test/support/fixtures/projects/ids/cypress/integration/baz.js @@ -1 +1 @@ -import "./dom.jsx" \ No newline at end of file +import './dom.jsx' diff --git a/packages/server/test/support/fixtures/projects/multiple-task-registrations/cypress/plugins/index.js b/packages/server/test/support/fixtures/projects/multiple-task-registrations/cypress/plugins/index.js index 2979d06e965d..8c66f332da02 100644 --- a/packages/server/test/support/fixtures/projects/multiple-task-registrations/cypress/plugins/index.js +++ b/packages/server/test/support/fixtures/projects/multiple-task-registrations/cypress/plugins/index.js @@ -1,11 +1,19 @@ module.exports = (on) => { on('task', { - 'one' () { return 'one' }, - 'two' () { return 'two' }, + 'one' () { + return 'one' + }, + 'two' () { + return 'two' + }, }) on('task', { - 'two' () { return 'two again' }, - 'three' () { return 'three' }, + 'two' () { + return 'two again' + }, + 'three' () { + return 'three' + }, }) } diff --git a/packages/server/test/support/fixtures/projects/no-server/helpers/includes.js b/packages/server/test/support/fixtures/projects/no-server/helpers/includes.js index 03f300c4fa91..81969706f702 100644 --- a/packages/server/test/support/fixtures/projects/no-server/helpers/includes.js +++ b/packages/server/test/support/fixtures/projects/no-server/helpers/includes.js @@ -1,3 +1,3 @@ -beforeEach(function(){ +beforeEach(function () { -}); \ No newline at end of file +}) diff --git a/packages/server/test/support/fixtures/projects/no-server/my-tests/test1.js b/packages/server/test/support/fixtures/projects/no-server/my-tests/test1.js index d995e2b550f8..e3a5395eee36 100644 --- a/packages/server/test/support/fixtures/projects/no-server/my-tests/test1.js +++ b/packages/server/test/support/fixtures/projects/no-server/my-tests/test1.js @@ -1,3 +1,4 @@ -it("tests without a server", function(){ +/* eslint-disable mocha/no-global-tests */ +it('tests without a server', function () { -}); \ No newline at end of file +}) diff --git a/packages/server/test/support/fixtures/projects/plugin-extension/ext/background.js b/packages/server/test/support/fixtures/projects/plugin-extension/ext/background.js index aa1a7bbebb23..c8bd7e7d9306 100644 --- a/packages/server/test/support/fixtures/projects/plugin-extension/ext/background.js +++ b/packages/server/test/support/fixtures/projects/plugin-extension/ext/background.js @@ -1,5 +1,3 @@ -/* global document */ - const el = document.getElementById('extension') if (el) { diff --git a/packages/server/test/support/fixtures/projects/plugin-extension/ext/manifest.json b/packages/server/test/support/fixtures/projects/plugin-extension/ext/manifest.json index f99b8dac309a..68a6c0163a9c 100644 --- a/packages/server/test/support/fixtures/projects/plugin-extension/ext/manifest.json +++ b/packages/server/test/support/fixtures/projects/plugin-extension/ext/manifest.json @@ -3,16 +3,22 @@ "version": "0", "description": "tests adding user extension into Cypress", "permissions": [ - "tabs", "webNavigation", "" + "tabs", + "webNavigation", + "" ], "content_scripts": [ { - "matches": [""], + "matches": [ + "" + ], "exclude_matches": [ "*://*/__cypress/*", "*://*/__/*" ], - "js": ["background.js"], + "js": [ + "background.js" + ], "run_at": "document_end", "all_frames": true } diff --git a/packages/server/test/support/fixtures/projects/plugins-async-error/cypress/plugins/index.js b/packages/server/test/support/fixtures/projects/plugins-async-error/cypress/plugins/index.js index 30ae36270881..9689fd8b504b 100644 --- a/packages/server/test/support/fixtures/projects/plugins-async-error/cypress/plugins/index.js +++ b/packages/server/test/support/fixtures/projects/plugins-async-error/cypress/plugins/index.js @@ -1,5 +1,3 @@ -/* global Promise */ - module.exports = (on) => { on('file:preprocessor', () => { return new Promise(() => { diff --git a/packages/server/test/support/fixtures/projects/todos/tests/test1.js b/packages/server/test/support/fixtures/projects/todos/tests/test1.js index 06281e2ddb99..e5b9547cd65a 100644 --- a/packages/server/test/support/fixtures/projects/todos/tests/test1.js +++ b/packages/server/test/support/fixtures/projects/todos/tests/test1.js @@ -1,3 +1,4 @@ -it("is truthy", function(){ +/* eslint-disable mocha/no-global-tests */ +it('is truthy', function () { expect(true).to.be.true -}) \ No newline at end of file +}) diff --git a/packages/server/test/support/fixtures/projects/working-preprocessor/cypress/integration/another_spec.js b/packages/server/test/support/fixtures/projects/working-preprocessor/cypress/integration/another_spec.js index e67642893051..7bf33f67833f 100644 --- a/packages/server/test/support/fixtures/projects/working-preprocessor/cypress/integration/another_spec.js +++ b/packages/server/test/support/fixtures/projects/working-preprocessor/cypress/integration/another_spec.js @@ -1,5 +1,4 @@ -/* global it, expect */ - +/* eslint-disable mocha/no-global-tests */ it('is another spec', () => { expect(false).to.be.false }) From 9f082d97ca45d4512478f8b212a2abe89bb30fb9 Mon Sep 17 00:00:00 2001 From: Gleb Bahmutov Date: Mon, 23 Sep 2019 11:59:49 -0400 Subject: [PATCH 4/5] Catch env variable with reserved name CYPRESS_ENV 1621 (#1626) * server: check CYPRESS_ENV variable when merging configs * catch invalid CYPRESS_ENV value in CLI, close #1621 * linting * sanitize platform in test snapshot * linting * update error message text * add missing comma * fix finally merge in JS code * pass CLI linter * fix log reference, should be debug * use correct sinon reference * update message, show first part in red * update error message text --- cli/__snapshots__/cli_spec.js | 29 +++++ cli/__snapshots__/errors_spec.js | 1 + cli/index.js | 2 +- cli/lib/cli.js | 107 +++++++++++++++---- cli/lib/errors.js | 79 ++++++++++---- cli/lib/util.js | 21 +++- cli/package.json | 3 +- cli/test/lib/cli_spec.js | 52 +++++++++ packages/server/lib/config.coffee | 12 ++- packages/server/lib/errors.coffee | 8 ++ packages/server/test/unit/config_spec.coffee | 30 +++++- 11 files changed, 293 insertions(+), 51 deletions(-) diff --git a/cli/__snapshots__/cli_spec.js b/cli/__snapshots__/cli_spec.js index d471f77b5da0..5465cc92f7d9 100644 --- a/cli/__snapshots__/cli_spec.js +++ b/cli/__snapshots__/cli_spec.js @@ -354,3 +354,32 @@ exports['shows help for run --foo 1'] = ` ------- ` + +exports['cli CYPRESS_ENV allows staging environment 1'] = ` + code: 0 + stderr: + ------- + + ------- + +` + +exports['cli CYPRESS_ENV catches environment "foo" 1'] = ` + code: 11 + stderr: + ------- + The environment variable with the reserved name "CYPRESS_ENV" is set. + + Unset the "CYPRESS_ENV" environment variable and run Cypress again. + + ---------- + + CYPRESS_ENV=foo + + ---------- + + Platform: xxx + Cypress Version: 1.2.3 + ------- + +` diff --git a/cli/__snapshots__/errors_spec.js b/cli/__snapshots__/errors_spec.js index f8333d37acba..44b1a631d786 100644 --- a/cli/__snapshots__/errors_spec.js +++ b/cli/__snapshots__/errors_spec.js @@ -32,6 +32,7 @@ exports['errors individual has the following errors 1'] = [ "failedDownload", "failedUnzip", "invalidCacheDirectory", + "invalidCypressEnv", "invalidSmokeTestDisplayError", "missingApp", "missingDependency", diff --git a/cli/index.js b/cli/index.js index 7da84f9ec87b..250d3d4b52b1 100644 --- a/cli/index.js +++ b/cli/index.js @@ -23,6 +23,6 @@ switch (args.exec) { break default: - // export our node module interface + debug('exporting Cypress module interface') module.exports = require('./lib/cypress') } diff --git a/cli/lib/cli.js b/cli/lib/cli.js index dc6a6e82e7c8..fb94e25eebc6 100644 --- a/cli/lib/cli.js +++ b/cli/lib/cli.js @@ -5,6 +5,7 @@ const logSymbols = require('log-symbols') const debug = require('debug')('cypress:cli') const util = require('./util') const logger = require('./logger') +const errors = require('./errors') const cache = require('./tasks/cache') // patch "commander" method called when a user passed an unknown option @@ -27,7 +28,9 @@ const coerceFalse = (arg) => { const spaceDelimitedSpecsMsg = (files) => { logger.log() logger.warn(stripIndent` - ${logSymbols.warning} Warning: It looks like you're passing --spec a space-separated list of files: + ${ + logSymbols.warning +} Warning: It looks like you're passing --spec a space-separated list of files: "${files.join(' ')}" @@ -54,7 +57,8 @@ const parseVariableOpts = (fnArgs, args) => { const nextOptOffset = _.findIndex(_.slice(args, argIndex), (arg) => { return _.startsWith(arg, '--') }) - const endIndex = nextOptOffset !== -1 ? argIndex + nextOptOffset : args.length + const endIndex = + nextOptOffset !== -1 ? argIndex + nextOptOffset : args.length const maybeSpecs = _.slice(args, argIndex, endIndex) const extraSpecs = _.intersection(maybeSpecs, fnArgs) @@ -70,11 +74,34 @@ const parseVariableOpts = (fnArgs, args) => { } const parseOpts = (opts) => { - opts = _.pick(opts, - 'project', 'spec', 'reporter', 'reporterOptions', 'path', 'destination', - 'port', 'env', 'cypressVersion', 'config', 'record', 'key', - 'browser', 'detached', 'headed', 'global', 'dev', 'force', 'exit', - 'cachePath', 'cacheList', 'cacheClear', 'parallel', 'group', 'ciBuildId') + opts = _.pick( + opts, + 'project', + 'spec', + 'reporter', + 'reporterOptions', + 'path', + 'destination', + 'port', + 'env', + 'cypressVersion', + 'config', + 'record', + 'key', + 'browser', + 'detached', + 'headed', + 'global', + 'dev', + 'force', + 'exit', + 'cachePath', + 'cacheList', + 'cacheClear', + 'parallel', + 'group', + 'ciBuildId' + ) if (opts.exit) { opts = _.omit(opts, 'exit') @@ -86,16 +113,23 @@ const parseOpts = (opts) => { } const descriptions = { - record: 'records the run. sends test results, screenshots and videos to your Cypress Dashboard.', - key: 'your secret Record Key. you can omit this if you set a CYPRESS_RECORD_KEY environment variable.', + record: + 'records the run. sends test results, screenshots and videos to your Cypress Dashboard.', + key: + 'your secret Record Key. you can omit this if you set a CYPRESS_RECORD_KEY environment variable.', spec: 'runs a specific spec file. defaults to "all"', - reporter: 'runs a specific mocha reporter. pass a path to use a custom reporter. defaults to "spec"', + reporter: + 'runs a specific mocha reporter. pass a path to use a custom reporter. defaults to "spec"', reporterOptions: 'options for the mocha reporter. defaults to "null"', port: 'runs Cypress on a specific port. overrides any value in cypress.json.', - env: 'sets environment variables. separate multiple values with a comma. overrides any value in cypress.json or cypress.env.json', - config: 'sets configuration values. separate multiple values with a comma. overrides any value in cypress.json.', - browserRunMode: 'runs Cypress in the browser with the given name. if a filesystem path is supplied, Cypress will attempt to use the browser at that path.', - browserOpenMode: 'path to a custom browser to be added to the list of available browsers in Cypress', + env: + 'sets environment variables. separate multiple values with a comma. overrides any value in cypress.json or cypress.env.json', + config: + 'sets configuration values. separate multiple values with a comma. overrides any value in cypress.json.', + browserRunMode: + 'runs Cypress in the browser with the given name. if a filesystem path is supplied, Cypress will attempt to use the browser at that path.', + browserOpenMode: + 'path to a custom browser to be added to the list of available browsers in Cypress', detached: 'runs Cypress application in detached mode', project: 'path to the project', global: 'force Cypress into global mode as if its globally installed', @@ -108,11 +142,25 @@ const descriptions = { cacheList: 'list cached binary versions', cacheClear: 'delete all cached binaries', group: 'a named group for recorded runs in the Cypress dashboard', - parallel: 'enables concurrent runs and automatic load balancing of specs across multiple machines or processes', - ciBuildId: 'the unique identifier for a run on your CI provider. typically a "BUILD_ID" env var. this value is automatically detected for most CI providers', + parallel: + 'enables concurrent runs and automatic load balancing of specs across multiple machines or processes', + ciBuildId: + 'the unique identifier for a run on your CI provider. typically a "BUILD_ID" env var. this value is automatically detected for most CI providers', } -const knownCommands = ['version', 'run', 'open', 'install', 'verify', '-v', '--version', 'help', '-h', '--help', 'cache'] +const knownCommands = [ + 'version', + 'run', + 'open', + 'install', + 'verify', + '-v', + '--version', + 'help', + '-h', + '--help', + 'cache', +] const text = (description) => { if (!descriptions[description]) { @@ -123,9 +171,11 @@ const text = (description) => { } function includesVersion (args) { - return _.includes(args, 'version') || + return ( + _.includes(args, 'version') || _.includes(args, '--version') || _.includes(args, '-v') + ) } function showVersions () { @@ -147,6 +197,14 @@ module.exports = { args = process.argv } + if (!util.isValidCypressEnvValue(process.env.CYPRESS_ENV)) { + debug('invalid CYPRESS_ENV value', process.env.CYPRESS_ENV) + + return errors.exitWithError(errors.errors.invalidCypressEnv)( + `CYPRESS_ENV=${process.env.CYPRESS_ENV}` + ) + } + const program = new commander.Command() // bug in commaner not printing name @@ -177,7 +235,10 @@ module.exports = { .option('-k, --key ', text('key')) .option('-s, --spec ', text('spec')) .option('-r, --reporter ', text('reporter')) - .option('-o, --reporter-options ', text('reporterOptions')) + .option( + '-o, --reporter-options ', + text('reporterOptions') + ) .option('-p, --port ', text('port')) .option('-e, --env ', text('env')) .option('-c, --config ', text('config')) @@ -218,7 +279,9 @@ module.exports = { program .command('install') .usage('[options]') - .description('Installs the Cypress executable matching this package\'s version') + .description( + 'Installs the Cypress executable matching this package\'s version' + ) .option('-f, --force', text('forceInstall')) .action((opts) => { require('./tasks/install') @@ -229,7 +292,9 @@ module.exports = { program .command('verify') .usage('[options]') - .description('Verifies that Cypress is installed correctly and executable') + .description( + 'Verifies that Cypress is installed correctly and executable' + ) .option('--dev', text('dev'), coerceFalse) .action((opts) => { const defaultOpts = { force: true, welcomeMessage: false } diff --git a/cli/lib/errors.js b/cli/lib/errors.js index 1a83b96ce434..ee45a1c21c4a 100644 --- a/cli/lib/errors.js +++ b/cli/lib/errors.js @@ -36,7 +36,9 @@ const failedUnzip = { const missingApp = (binaryDir) => { return { - description: `No version of Cypress is installed in: ${chalk.cyan(binaryDir)}`, + description: `No version of Cypress is installed in: ${chalk.cyan( + binaryDir + )}`, solution: stripIndent` \nPlease reinstall Cypress by running: ${chalk.cyan('cypress install')} `, @@ -59,7 +61,8 @@ const binaryNotExecutable = (executable) => { const notInstalledCI = (executable) => { return { - description: 'The cypress npm package is installed, but the Cypress binary is missing.', + description: + 'The cypress npm package is installed, but the Cypress binary is missing.', solution: stripIndent`\n We expected the binary to be installed here: ${chalk.cyan(executable)} @@ -114,7 +117,7 @@ const smokeTestFailure = (smokeTestCommand, timedOut) => { const invalidSmokeTestDisplayError = { code: 'INVALID_SMOKE_TEST_DISPLAY_ERROR', description: 'Cypress verification failed.', - solution (msg) { + solution (msg) { return stripIndent` Cypress failed to start after spawning a new Xvfb server. @@ -152,7 +155,8 @@ const missingDependency = { } const invalidCacheDirectory = { - description: 'Cypress cannot write to the cache directory due to file permissions', + description: + 'Cypress cannot write to the cache directory due to file permissions', solution: stripIndent` See discussion and possible solutions at ${chalk.blue(util.getGitHubIssueUrl(1281))} @@ -165,7 +169,8 @@ const versionMismatch = { } const unexpected = { - description: 'An unexpected error occurred while verifying the Cypress executable.', + description: + 'An unexpected error occurred while verifying the Cypress executable.', solution: stripIndent` Please search Cypress documentation for possible solutions: @@ -179,10 +184,19 @@ const unexpected = { `, } +const invalidCypressEnv = { + description: + chalk.red('The environment variable with the reserved name "CYPRESS_ENV" is set.'), + solution: chalk.red('Unset the "CYPRESS_ENV" environment variable and run Cypress again.'), + exitCode: 11, +} + const removed = { CYPRESS_BINARY_VERSION: { description: stripIndent` - The environment variable CYPRESS_BINARY_VERSION has been renamed to CYPRESS_INSTALL_BINARY as of version ${chalk.green('3.0.0')} + The environment variable CYPRESS_BINARY_VERSION has been renamed to CYPRESS_INSTALL_BINARY as of version ${chalk.green( + '3.0.0' + )} `, solution: stripIndent` You should set CYPRESS_INSTALL_BINARY instead. @@ -190,7 +204,9 @@ const removed = { }, CYPRESS_SKIP_BINARY_INSTALL: { description: stripIndent` - The environment variable CYPRESS_SKIP_BINARY_INSTALL has been removed as of version ${chalk.green('3.0.0')} + The environment variable CYPRESS_SKIP_BINARY_INSTALL has been removed as of version ${chalk.green( + '3.0.0' + )} `, solution: stripIndent` To skip the binary install, set CYPRESS_INSTALL_BINARY=0 @@ -210,8 +226,7 @@ const CYPRESS_RUN_BINARY = { } function getPlatformInfo () { - return util.getOsVersionAsync() - .then((version) => { + return util.getOsVersionAsync().then((version) => { return stripIndent` Platform: ${os.platform()} (${version}) Cypress Version: ${util.pkgVersion()} @@ -220,8 +235,7 @@ function getPlatformInfo () { } function addPlatformInformation (info) { - return getPlatformInfo() - .then((platform) => { + return getPlatformInfo().then((platform) => { return merge(info, { platform }) }) } @@ -231,18 +245,18 @@ function addPlatformInformation (info) { * and if possible a way to solve it. Resolves with a string. */ function formErrorText (info, msg, prevMessage) { - return addPlatformInformation(info) - .then((obj) => { + return addPlatformInformation(info).then((obj) => { const formatted = [] function add (msg) { - formatted.push( - stripIndents(msg) - ) + formatted.push(stripIndents(msg)) } - la(is.unemptyString(obj.description), - 'expected error description to be text', obj.description) + la( + is.unemptyString(obj.description), + 'expected error description to be text', + obj.description + ) // assuming that if there the solution is a function it will handle // error message and (optional previous error message) @@ -258,8 +272,11 @@ function formErrorText (info, msg, prevMessage) { `) } else { - la(is.unemptyString(obj.solution), - 'expected error solution to be text', obj.solution) + la( + is.unemptyString(obj.solution), + 'expected error solution to be text', + obj.solution + ) add(` ${obj.description} @@ -312,13 +329,30 @@ const raise = (info) => { const throwFormErrorText = (info) => { return (msg, prevMessage) => { - return formErrorText(info, msg, prevMessage) - .then(raise(info)) + return formErrorText(info, msg, prevMessage).then(raise(info)) + } +} + +/** + * Forms full error message with error and OS details, prints to the error output + * and then exits the process. + * @param {ErrorInformation} info Error information {description, solution} + * @example return exitWithError(errors.invalidCypressEnv)('foo') + */ +const exitWithError = (info) => { + return (msg) => { + return formErrorText(info, msg).then((text) => { + // eslint-disable-next-line no-console + console.error(text) + process.exit(info.exitCode || 1) + }) } } module.exports = { raise, + exitWithError, + // formError, formErrorText, throwFormErrorText, hr, @@ -334,6 +368,7 @@ module.exports = { unexpected, failedDownload, failedUnzip, + invalidCypressEnv, invalidCacheDirectory, removed, CYPRESS_RUN_BINARY, diff --git a/cli/lib/util.js b/cli/lib/util.js index 9d0044835675..029c0bb1893c 100644 --- a/cli/lib/util.js +++ b/cli/lib/util.js @@ -119,6 +119,25 @@ function stdoutLineMatches (expectedLine, stdout) { return lines.some(lineMatches) } +/** + * Confirms if given value is a valid CYPRESS_ENV value. Undefined values + * are valid, because the system can set the default one. + * + * @param {string} value + * @example util.isValidCypressEnvValue(process.env.CYPRESS_ENV) + */ +function isValidCypressEnvValue (value) { + if (_.isUndefined(value)) { + // will get default value + return true + } + + // names of config environments, see "packages/server/config/app.yml" + const names = ['development', 'test', 'staging', 'production'] + + return _.includes(names, value) +} + /** * Prints NODE_OPTIONS using debug() module, but only * if DEBUG=cypress... is set @@ -158,7 +177,7 @@ const dequote = (str) => { const util = { normalizeModuleOptions, - + isValidCypressEnvValue, printNodeOptions, isCi () { diff --git a/cli/package.json b/cli/package.json index 30e1e8fa1780..37ff7f30be79 100644 --- a/cli/package.json +++ b/cli/package.json @@ -86,7 +86,8 @@ "shelljs": "0.8.3", "sinon": "7.2.2", "snap-shot-it": "7.8.0", - "spawn-mock": "1.0.0" + "spawn-mock": "1.0.0", + "strip-ansi": "4.0.0" }, "files": [ "bin", diff --git a/cli/test/lib/cli_spec.js b/cli/test/lib/cli_spec.js index 952441ad1132..42ed40e53a41 100644 --- a/cli/test/lib/cli_spec.js +++ b/cli/test/lib/cli_spec.js @@ -73,6 +73,58 @@ describe('cli', () => { }) }) + context('CYPRESS_ENV', () => { + /** + * Replaces line "Platform: ..." with "Platform: xxx" + * @param {string} s + */ + const replacePlatform = (s) => { + return s.replace(/Platform: .+/, 'Platform: xxx') + } + + /** + * Replaces line "Cypress Version: ..." with "Cypress Version: 1.2.3" + * @param {string} s + */ + const replaceCypressVersion = (s) => { + return s.replace(/Cypress Version: .+/, 'Cypress Version: 1.2.3') + } + + const sanitizePlatform = (text) => { + return text + .split(os.eol) + .map(replacePlatform) + .map(replaceCypressVersion) + .join(os.eol) + } + + it('allows staging environment', () => { + const options = { + env: { + CYPRESS_ENV: 'staging', + }, + // we are only interested in the exit code + filter: ['code', 'stderr'], + } + + return execa('bin/cypress', ['help'], options).then(snapshot) + }) + + it('catches environment "foo"', () => { + const options = { + env: { + CYPRESS_ENV: 'foo', + }, + // we are only interested in the exit code + filter: ['code', 'stderr'], + } + + return execa('bin/cypress', ['help'], options) + .then(sanitizePlatform) + .then(snapshot) + }) + }) + context('cypress version', () => { const binaryDir = '/binary/dir' diff --git a/packages/server/lib/config.coffee b/packages/server/lib/config.coffee index 7777795fec20..96f9a36bd1a8 100644 --- a/packages/server/lib/config.coffee +++ b/packages/server/lib/config.coffee @@ -10,7 +10,7 @@ origin = require("./util/origin") coerce = require("./util/coerce") settings = require("./util/settings") v = require("./util/validation") -debug = require("debug")("cypress:server:config") +debug = require("debug")("cypress:server:config") pathHelpers = require("./util/path_helpers") CYPRESS_ENV_PREFIX = "CYPRESS_" @@ -207,6 +207,11 @@ hideSpecialVals = (val, key) -> module.exports = { getConfigKeys: -> configKeys + isValidCypressEnvValue: (value) -> + # names of config environments, see "config/app.yml" + names = ["development", "test", "staging", "production"] + _.includes(names, value) + whitelist: (obj = {}) -> _.pick(obj, configKeys.concat(breakingConfigKeys)) @@ -265,7 +270,12 @@ module.exports = { ## split out our own app wide env from user env variables ## and delete envFile config.env = @parseEnv(config, options.env, resolved) + config.cypressEnv = process.env["CYPRESS_ENV"] + debug("using CYPRESS_ENV %s", config.cypressEnv) + if not @isValidCypressEnvValue(config.cypressEnv) + errors.throw("INVALID_CYPRESS_ENV", config.cypressEnv) + delete config.envFile ## when headless diff --git a/packages/server/lib/errors.coffee b/packages/server/lib/errors.coffee index 02ff1114092f..b5c329493b0c 100644 --- a/packages/server/lib/errors.coffee +++ b/packages/server/lib/errors.coffee @@ -826,6 +826,14 @@ getMsgByType = (type, arg1 = {}, arg2) -> """ Cypress detected policy settings on your computer that may cause issues with using this browser. For more information, see https://on.cypress.io/bad-browser-policy """ + when "INVALID_CYPRESS_ENV" + """ + We have detected unknown or unsupported CYPRESS_ENV value + + #{chalk.yellow(arg1)} + + Please do not modify CYPRESS_ENV value. + """ get = (type, arg1, arg2) -> msg = getMsgByType(type, arg1, arg2) diff --git a/packages/server/test/unit/config_spec.coffee b/packages/server/test/unit/config_spec.coffee index 8880304be789..57e4f20e47a6 100644 --- a/packages/server/test/unit/config_spec.coffee +++ b/packages/server/test/unit/config_spec.coffee @@ -3,10 +3,11 @@ require("../spec_helper") _ = require("lodash") path = require("path") R = require("ramda") -config = require("#{root}lib/config") -configUtil = require("#{root}lib/util/config") -scaffold = require("#{root}lib/scaffold") -settings = require("#{root}lib/util/settings") +config = require("#{root}lib/config") +errors = require("#{root}lib/errors") +configUtil = require("#{root}lib/util/config") +scaffold = require("#{root}lib/scaffold") +settings = require("#{root}lib/util/settings") describe "lib/config", -> beforeEach -> @@ -17,6 +18,27 @@ describe "lib/config", -> afterEach -> process.env = @env + context "environment name check", -> + it "throws an error for unknown CYPRESS_ENV", -> + sinon.stub(errors, "throw").withArgs("INVALID_CYPRESS_ENV", "foo-bar") + process.env.CYPRESS_ENV = "foo-bar" + cfg = { + projectRoot: "/foo/bar/" + } + options = {} + config.mergeDefaults(cfg, options) + expect(errors.throw).have.been.calledOnce + + it "allows known CYPRESS_ENV", -> + sinon.stub(errors, "throw") + process.env.CYPRESS_ENV = "test" + cfg = { + projectRoot: "/foo/bar/" + } + options = {} + config.mergeDefaults(cfg, options) + expect(errors.throw).not.to.be.called + context ".get", -> beforeEach -> @projectRoot = "/_test-output/path/to/project" From e0fb38fcab6cde257ef54f357b6c9aff135ce600 Mon Sep 17 00:00:00 2001 From: ryan-snyder <36665494+ryan-snyder@users.noreply.github.com> Date: Mon, 23 Sep 2019 14:47:51 -0300 Subject: [PATCH 5/5] Addresses #2953 (#5174) * Addresses #2953 * Added proper test for new error message * Didn't realize it ran this test as well, whoops * Implementing changes as suggested by @jennifer-shehane * Fixing tests and error output. Moved the checks to the start of the get command to ensure we always catch improper options * Removing issue test since the querying spec covers it * Using coffescript isArray check --- packages/driver/src/cy/commands/querying.coffee | 8 +++++--- packages/driver/src/cypress/error_messages.coffee | 1 + .../cypress/integration/commands/querying_spec.coffee | 7 +++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/driver/src/cy/commands/querying.coffee b/packages/driver/src/cy/commands/querying.coffee index 934edb0b584a..25ddb2fcf4fd 100644 --- a/packages/driver/src/cy/commands/querying.coffee +++ b/packages/driver/src/cy/commands/querying.coffee @@ -68,7 +68,10 @@ module.exports = (Commands, Cypress, cy, state, config) -> get: (selector, options = {}) -> ctx = @ - + + if options is null or Array.isArray(options) or typeof options isnt 'object' then return $utils.throwErrByPath "get.invalid_options", { + args: { options } + } _.defaults(options, { retry: true withinSubject: cy.state("withinSubject") @@ -78,7 +81,6 @@ module.exports = (Commands, Cypress, cy, state, config) -> }) consoleProps = {} - start = (aliasType) -> return if options.log is false @@ -467,4 +469,4 @@ module.exports = (Commands, Cypress, cy, state, config) -> cy.state("withinSubject", null) return subject - }) + }) \ No newline at end of file diff --git a/packages/driver/src/cypress/error_messages.coffee b/packages/driver/src/cypress/error_messages.coffee index 58740aed0964..1c98ecfdac07 100644 --- a/packages/driver/src/cypress/error_messages.coffee +++ b/packages/driver/src/cypress/error_messages.coffee @@ -292,6 +292,7 @@ module.exports = { get: alias_invalid: "'{{prop}}' is not a valid alias property. Only 'numbers' or 'all' is permitted." alias_zero: "'0' is not a valid alias property. Are you trying to ask for the first response? If so write @{{alias}}.1" + invalid_options: "#{cmd('get')} only accepts an options object for its second argument. You passed {{options}}" getCookie: invalid_argument: "#{cmd('getCookie')} must be passed a string argument for name." diff --git a/packages/driver/test/cypress/integration/commands/querying_spec.coffee b/packages/driver/test/cypress/integration/commands/querying_spec.coffee index 11c79a85dd82..a6303e9c22f4 100644 --- a/packages/driver/test/cypress/integration/commands/querying_spec.coffee +++ b/packages/driver/test/cypress/integration/commands/querying_spec.coffee @@ -1173,7 +1173,14 @@ describe "src/cy/commands/querying", -> .server() .route(/users/, {}).as("getUsers") .get("@getUsers.all ") + _.each ["", "foo", [], 1, null ], (value) => + it "throws when options property is not an object. Such as: #{value}", (done) -> + cy.on "fail", (err) -> + expect(err.message).to.include "only accepts an options object for its second argument. You passed #{value}" + done() + cy.get("foobar", value) + it "logs out $el when existing $el is found even on failure", (done) -> button = cy.$$("#button").hide()