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

fix: issue with array environment variables not being handled correct… #8135

Closed
wants to merge 12 commits into from
Closed

Conversation

dmoini
Copy link
Contributor

@dmoini dmoini commented Jul 31, 2020

User facing changelog

Fixes an issue where Cypress environment variables intended to be arrays were saved as strings.

Additional details

When setting Cypress environment variables as arrays, they are stored as a string of the array instead of an array. By default, all environment variables are stored as strings, but some environment variables are intended to be numbers or booleans. That issue gets resolved in packages/server/lib/util/coerce.js by checking if any Cypress environment variables are meant to be a number or boolean, and if so converting them. But there is no check to see if a Cypress environment variable is meant to be an array.

This issue is resolved by added a helper function (toArray) to check if the Cypress environment variable can be converted into an array and adding toArray to be checked in the coerce function. One important part in toArray is updating the toString method for the returned array. This is because the default array toString method returns one string containing each array element separated by commas, but without '[' or ']'. If an environment variable is intended to be an array, it will be a string beginning and ending with '[' and ']' respectively. For example, if export CYPRESS_ARR=[1,2,3], then process.env['CYPRESS_ARR']: '[1,2,3]'. But [1,2,3].toString() = '1,2,3', so comparing the two values would return false and not correctly update the Cypress environment variable to be an array. To correctly compare the Cypress environment variable with the returned array from toArray, the returned array's toString method must be updated to include '[' and ']'.

How has the user experience changed?

Now when setting Cypress environment variables as arrays, they are correctly converted to arrays. Below are examples from #6810.

export CYPRESS_BLACKLIST_HOSTS=["www.google-analytics.com","*.hotjar.com"]
export CYPRESS_testFiles="[\"**/196*\", \"**/475*\", \"**/476*\"]"

Before:
errorBlacklistHosts
errorTestFiles

After:
blacklistHosts
testFiles

The example with CYPRESS_testFiles was originally intended as a workaround for #6810 before this pull request. While CYPRESS_testFiles is technically parsed correctly, I doubt that a user would want the array elements to contain quotation marks within their strings and have an element such as " "**/475*"" instead of "**/475*". To avoid this, make sure to set array Cypress environment variables with each element in quotation marks and each element delimited by a single comma without a whitespace (i.e. CYPRESS_testFiles=["**/196*","**/475*","**/476*"].

This also works when setting Cypress environment variables via --env.

cypress run --env foo="[bar1,bar2,bar3]"

image

PR Tasks

  • Have tests been added/updated?
  • Has the original issue been tagged with a release in ZenHub?
  • Has a PR for user-facing changes been opened in cypress-documentation?
  • Have API changes been updated in the type definitions?
  • Have new configuration options been added to the cypress.schema.json?

…ly (#6810)

Signed-off-by: Donovan Moini <donovanmoini@gmail.com>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 31, 2020

Thanks for taking the time to open a PR!

@CLAassistant
Copy link

CLAassistant commented Jul 31, 2020

CLA assistant check
All committers have signed the CLA.

@dmoini dmoini closed this Jul 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Array in environment variables CYPRESS_BLACKLIST_HOSTS, CYPRESS_testFiles not handled correctly
6 participants