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

Array in environment variables CYPRESS_BLACKLIST_HOSTS, CYPRESS_testFiles not handled correctly #6810

Closed
tobiashinz opened this issue Mar 23, 2020 · 6 comments · Fixed by #8151 · May be fixed by Omrisnyk/npm-lockfiles#145 or Omrisnyk/npm-lockfiles#164
Labels

Comments

@tobiashinz
Copy link

tobiashinz commented Mar 23, 2020

Current behavior:

Setting multiple hosts via the environment variable CYPRESS_BLACKLIST_HOSTS as an array leads to them not picked up corretly.

Example:

export CYPRESS_BLACKLIST_HOSTS=["www.google-analytics.com","*.hotjar.com"]

The environment variable is always handled as a string

Desired behavior:

The desired behavior is to analyze if an array was provided and handled each item separately.

Test code to reproduce

export CYPRESS_BLACKLIST_HOSTS=["www.google-analytics.com","*.hotjar.com"]

cypress open

Versions

Node.js Version 12.8.1
Cypress Version 4.1.0

@jennifer-shehane
Copy link
Member

Can confirm, this is resolved as a string when passed as an array in via environment variables.

CYPRESS_BLACKLIST_HOSTS=["www.google-analytics.com","*.hotjar.com"] cypress open

Screen Shot 2020-03-23 at 4 36 03 PM copy

This is the only configuration value passable via CYPRESS_ that accepts a non string or number, so I imagine we're just not handling it at all.

@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label Mar 23, 2020
@JosiahOne
Copy link

Looks like there is at least one other array-based CYPRESS_ env var that behaves incorrectly: CYPRESS_testFiles.

CYPRESS_testFiles="[\"**/196*\", \"**/475*\", \"**/476*\"]" npx cypress open

Screen Shot 2020-07-16 at 1 57 24 PM

@jennifer-shehane jennifer-shehane changed the title Array in environment variable CYPRESS_BLACKLIST_HOSTS not handled correctly Array in environment variables CYPRESS_BLACKLIST_HOSTS, CYPRESS_testFiles not handled correctly Jul 23, 2020
@jennifer-shehane jennifer-shehane added the good first issue Good for newcomers label Jul 23, 2020
@dmoini
Copy link
Contributor

dmoini commented Jul 24, 2020

Would I be able to work on this issue? If so I had a few questions since I am new at contributing open source.

  1. Where should I look in the codebase to address this issue, specifically where environment variables are parsed?
  2. How would I be able to debug and test my changes locally?

@jennifer-shehane
Copy link
Member

@dmoini Check out our contributing doc for general guidance on running changes and tests locally. Generally, part of the work to parse env vars prefixed with CYPRESS_ is done in our config.js here https://github.com/cypress-io/cypress/blob/develop/packages/server/lib/config.js#L930:L930

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 14, 2020

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

@cypress-bot cypress-bot bot removed the stage: needs review The PR code is done & tested, needs review label Aug 14, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 20, 2020

Released in 5.0.0.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Aug 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.