Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn when using reserved key CYPRESS_ENV with non production value #6436

Closed
jennifer-shehane opened this issue Feb 13, 2020 · 3 comments · Fixed by #6437
Closed

Warn when using reserved key CYPRESS_ENV with non production value #6436

jennifer-shehane opened this issue Feb 13, 2020 · 3 comments · Fixed by #6437
Labels
cli type: error message type: unexpected behavior User expected result, but got another

Comments

@jennifer-shehane
Copy link
Member

jennifer-shehane commented Feb 13, 2020

Current behavior:

CYPRESS_ENV is a reserved key used to internally point to the API database to be used for communication from the Test Runner.

We previously included erroring when invalid keys were sent to CYPRESS_ENV because a very unexpected crash would happen in these cases: #1621

But, we do not warn users when setting the CYPRESS_ENV to a valid value that this is a reserved key. This is likely a very small use case (although we had two customers do this), but the consequences are very, very confusing - there is no indication that setting the CYPRESS_ENV would have been a mistake on their part.

Desired behavior:

Why is someone doing this?

They simply want to access an environment variable with the name env. This is a legitimate use case and this is not the first person to set CYPRESS_ENV environment variable. For example, if you do CYPRESS_HOST=foo, you can then access this later using CYPRESS.env('HOST') which will be 'foo'.

Unfortunately, this will not work as intended since CYPRESS_ENV is reserved for internal uses.

We need to warn when using CYPRESS_ENV to set a value other than production, that this is a reserved key and will not do what they intend. We cannot throw because our internal team needs to continue connecting to these API databases for testing and development.

Test code to reproduce

Mostly this problem is exhibited the worst during record since we hit our databases to record then. One example below:

CYPRESS_ENV=test cypress run --record --key abc123

Versions

4.0.1

@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label Feb 13, 2020
@jennifer-shehane jennifer-shehane added cli type: unexpected behavior User expected result, but got another labels Feb 13, 2020
@jennifer-shehane jennifer-shehane changed the title Warn when using reserved key CYPRESS_ENV Warn when using reserved key CYPRESS_ENV with non production value Feb 13, 2020
@jennifer-shehane
Copy link
Member Author

Upon more discussion, we've decided to additionally rename the internal environment variable used to something less likely to be used by our users themselves.

@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: ready for work The issue is reproducible and in scope labels Mar 4, 2020
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Mar 6, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Mar 6, 2020

The code for this is done in cypress-io/cypress#6437, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Mar 16, 2020

Released in 4.2.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v4.2.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Mar 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cli type: error message type: unexpected behavior User expected result, but got another
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant