Catch invalid env variable 1621 #1626
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jennifer-shehane
requested changes
May 15, 2018
There was a problem hiding this comment.
This still needs some work. See notes below:
Current error
The error should be printed in this red color:
Recommended error messaging and spacing
$ CYPRESS_ENV=myenv node ./cli/bin/cypress run --dev
Environment variable used with reserved name "CYPRESS_ENV".
Remove the "CYPRESS_ENV" variable and run Cypress again.
----------
CYPRESS_ENV=myenv
----------
Platform: darwin (14.5.0)
Cypress Version: 0.0.0
|
Hey @bahmutov, where did this land? Last work was done last April. |
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
So our typical error is red because it is thrown as an error. In this case I don't throw an error, but show it and then exit with specific code (which allows us to easily debug and test it). For now, I switched the first part of the error message to red color. I think this is enough for the user to understand.
|
jennifer-shehane
requested changes
Aug 29, 2019
jennifer-shehane
approved these changes
Sep 23, 2019
kuceb
pushed a commit
that referenced
this pull request
Sep 23, 2019
clean up cy chaining + eslint, decaffeinate suggestions rename .coffee to .js files in spec - reference own filename in test assertions - fix some element screenshots that were no longer chained off el. 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 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 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
flotwig
pushed a commit
that referenced
this pull request
Sep 24, 2019
* 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
kuceb
pushed a commit
that referenced
this pull request
Sep 24, 2019
clean up cy chaining + eslint, decaffeinate suggestions rename .coffee to .js files in spec - reference own filename in test assertions - fix some element screenshots that were no longer chained off el. 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 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 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
brian-mann
added a commit
that referenced
this pull request
Sep 24, 2019
* fix specs * use debugger protocol for cookie handling in electron * use latest gulp * use rimraf instead of gulp-clean * use electron 3.1.8 and node 10.2.1 * use gulp 4 in packages/static * fix sendCommandAsync, log Schema.getDomains on CDP connect * autofill e2e test name [skip ci] * electron@5.0.7, see what new failures exist * --no-sandbox for launching Electron * update cookies logic for electron * node 12 * update snapshot for new node * update error message for new node * stub sendCommandAsync * only connect to socket if path has been replaced, fixes #4776 * update node-sass to support node 12 * skip wacky socket tests for now * snapshot * fix run_plugins_spec snapshot, don't include stack trace * use --no-sandbox on linux to run as root * allow sendCommandAsync to resolve * use euid for root check * log domains even if undefined * don't worry about ending 1xx responses immediately anymore * use --max-http-header-size, change max size from 8kb to 1mb, fix #76 * do not send 502 on failed websocket, just send back ECONNRESET * update websocket spec port to not collide with other test * update outdated expect * Revert "only connect to socket if path has been replaced, fixes #4776" This reverts commit f179eda. * update gulp in root * update https-proxy unit tests * update network spec to properly close server * update reporter spec * update https-proxy-agent to fix node 10.10.0 change discussion: https://github.com/nodejs/node/issues/24474\#issuecomment-511963799 * only pass --max-http-header-size on node >=12 * use own server-destroy implementation that supports secureConnect events * oops * update socket_spec * electron 6.0.0 * console.table introduced in node 10 * change browserify entry to init.js * handle edge case when no response body * console.table added in node 10 * do not exit app when all BrowserWindows are closed * update e2e snapshots * value may not be null * update plugins spec * correct cookie expiry, use browser.getversion for CDP version check * fix snapshotting for require stacks * reorder cookies in spec * warn when depreated electron callback apis are used * only report 1 plugin error per process * cleanup * node 12.4.0, cypress/browsers:node12.4.0-chrome76 docker image * update shell.openExternal to promisified * update dialog.showOpenDialog to promisified * update webContents.session.setProxy to promisified * updating native dependencies since we don't need ancient node ABI support anymore * WIP: switch cookies to simpler, jar-less approach * WIP: switch cookies to simpler, jar-less approach * making tests pass * improve cookie filtering logic * Remove unneeded Promise.try * filter what makes it to the extension * properly re-set superdomain cookies on cross-origin cy.visit * allow comma-separated list of e2e tests * sort cookies in order of expiration date, ascending * updating tests, cleanup * update tests * version electron as a devDependency, electron@6.0.1 * cleanup, remove old automation * cleanup, remove old automation * bump chokidar to fix win10 + node12 issue was seeing this on windows: nuxt/nuxt#6035 fixed with version bump * enable now-supported quit role, re-enable old tests * don't need that arg there * remove last deprecated callback electron invocations * Delete cypress.json * responding to PR feedback * cleanup * invoke * use 'quit' role * Use new appMenu role for Cypress menu on mac * electron@6.0.2 * electron@6.0.3 * remove domain: cookie.domain and see what happens * remove setErrorHandler * Revert "remove domain: cookie.domain and see what happens" This reverts commit 49e9168. * add unit tests for cookies * ci * fix project-content css * electron@6.0.4 * fix specs_list test * electron@6.0.7 * some cleanup * electron@6.0.9 * Update 8_reporters_spec.coffee.js * electron@5.0.10 - Chromium 73, Node 12 * cli: fix the STDIN pipe on Windows (#5045) * cli: pipe stdin * uggh, here is the actual change * update cli unit tests * add unit test * more permissive check for json to include application/vnd.api+j… (#5166) * 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 * 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 * 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 * 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 * depromisify things that were promisified b/t electron 5 <=> 6 Revert "update shell.openExternal to promisified" This reverts commit 8b6460d. Revert "update dialog.showOpenDialog to promisified" This reverts commit 5f178b0. Revert "update webContents.session.setProxy to promisified" This reverts commit 727df3a. * node12.4.0-chrome76 => node12.0.0-chrome75 * fix tests for electron downgrade * node12.0.0-chrome75 => node12.0.0-chrome73 Co-authored-by: Zach Bloomquist <github@chary.us> Co-authored-by: Brian Mann <brian.mann86@gmail.com>
grabartley
pushed a commit
to grabartley/cypress
that referenced
this pull request
Oct 6, 2019
* server: check CYPRESS_ENV variable when merging configs * catch invalid CYPRESS_ENV value in CLI, close cypress-io#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
grabartley
pushed a commit
to grabartley/cypress
that referenced
this pull request
Oct 6, 2019
* fix specs * use debugger protocol for cookie handling in electron * use latest gulp * use rimraf instead of gulp-clean * use electron 3.1.8 and node 10.2.1 * use gulp 4 in packages/static * fix sendCommandAsync, log Schema.getDomains on CDP connect * autofill e2e test name [skip ci] * electron@5.0.7, see what new failures exist * --no-sandbox for launching Electron * update cookies logic for electron * node 12 * update snapshot for new node * update error message for new node * stub sendCommandAsync * only connect to socket if path has been replaced, fixes cypress-io#4776 * update node-sass to support node 12 * skip wacky socket tests for now * snapshot * fix run_plugins_spec snapshot, don't include stack trace * use --no-sandbox on linux to run as root * allow sendCommandAsync to resolve * use euid for root check * log domains even if undefined * don't worry about ending 1xx responses immediately anymore * use --max-http-header-size, change max size from 8kb to 1mb, fix cypress-io#76 * do not send 502 on failed websocket, just send back ECONNRESET * update websocket spec port to not collide with other test * update outdated expect * Revert "only connect to socket if path has been replaced, fixes cypress-io#4776" This reverts commit f179eda. * update gulp in root * update https-proxy unit tests * update network spec to properly close server * update reporter spec * update https-proxy-agent to fix node 10.10.0 change discussion: https://github.com/nodejs/node/issues/24474\#issuecomment-511963799 * only pass --max-http-header-size on node >=12 * use own server-destroy implementation that supports secureConnect events * oops * update socket_spec * electron 6.0.0 * console.table introduced in node 10 * change browserify entry to init.js * handle edge case when no response body * console.table added in node 10 * do not exit app when all BrowserWindows are closed * update e2e snapshots * value may not be null * update plugins spec * correct cookie expiry, use browser.getversion for CDP version check * fix snapshotting for require stacks * reorder cookies in spec * warn when depreated electron callback apis are used * only report 1 plugin error per process * cleanup * node 12.4.0, cypress/browsers:node12.4.0-chrome76 docker image * update shell.openExternal to promisified * update dialog.showOpenDialog to promisified * update webContents.session.setProxy to promisified * updating native dependencies since we don't need ancient node ABI support anymore * WIP: switch cookies to simpler, jar-less approach * WIP: switch cookies to simpler, jar-less approach * making tests pass * improve cookie filtering logic * Remove unneeded Promise.try * filter what makes it to the extension * properly re-set superdomain cookies on cross-origin cy.visit * allow comma-separated list of e2e tests * sort cookies in order of expiration date, ascending * updating tests, cleanup * update tests * version electron as a devDependency, electron@6.0.1 * cleanup, remove old automation * cleanup, remove old automation * bump chokidar to fix win10 + node12 issue was seeing this on windows: nuxt/nuxt#6035 fixed with version bump * enable now-supported quit role, re-enable old tests * don't need that arg there * remove last deprecated callback electron invocations * Delete cypress.json * responding to PR feedback * cleanup * invoke * use 'quit' role * Use new appMenu role for Cypress menu on mac * electron@6.0.2 * electron@6.0.3 * remove domain: cookie.domain and see what happens * remove setErrorHandler * Revert "remove domain: cookie.domain and see what happens" This reverts commit 49e9168. * add unit tests for cookies * ci * fix project-content css * electron@6.0.4 * fix specs_list test * electron@6.0.7 * some cleanup * electron@6.0.9 * Update 8_reporters_spec.coffee.js * electron@5.0.10 - Chromium 73, Node 12 * cli: fix the STDIN pipe on Windows (cypress-io#5045) * cli: pipe stdin * uggh, here is the actual change * update cli unit tests * add unit test * more permissive check for json to include application/vnd.api+j… (cypress-io#5166) * 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 * fix eslint for fixture specs (cypress-io#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 * Catch env variable with reserved name CYPRESS_ENV 1621 (cypress-io#1626) * server: check CYPRESS_ENV variable when merging configs * catch invalid CYPRESS_ENV value in CLI, close cypress-io#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 * Addresses cypress-io#2953 (cypress-io#5174) * Addresses cypress-io#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 * depromisify things that were promisified b/t electron 5 <=> 6 Revert "update shell.openExternal to promisified" This reverts commit 8b6460d. Revert "update dialog.showOpenDialog to promisified" This reverts commit 5f178b0. Revert "update webContents.session.setProxy to promisified" This reverts commit 727df3a. * node12.4.0-chrome76 => node12.0.0-chrome75 * fix tests for electron downgrade * node12.0.0-chrome75 => node12.0.0-chrome73 Co-authored-by: Zach Bloomquist <github@chary.us> Co-authored-by: Brian Mann <brian.mann86@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added catching
CYPRESS_ENVin both cli and server. There is a little bit of code duplication, but since we are not sharing code between CLI and the server I did not want to change it