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

Inconsistent handling of environment option methods for baseUrl #8680

Open
sethbattin opened this issue Sep 27, 2020 · 6 comments
Open

Inconsistent handling of environment option methods for baseUrl #8680

sethbattin opened this issue Sep 27, 2020 · 6 comments
Labels
E2E Issue related to end-to-end testing type: feature New feature that does not currently exist type: unexpected behavior User expected result, but got another

Comments

@sethbattin
Copy link

sethbattin commented Sep 27, 2020

I really wrote a novel here. I will tl;dr:

  1. Some cypress configuration methods respect ENV for setting baseUrl, and some do not.
  2. baseUrl is the only reason I would want to use a cypress.env.json file, and it doesn't work like that.
  3. The past confusion about this behavior is totally understandable, and it persists.

I promise this is in the spirit:
image


Prior discussion

This topic is different from previous issues about this behavior that discuss whether cypress.env.json should or should not work for this purpose. I would like to focus on the fact that the 4 ENV-based configuration-setting methods behave differently from each other. Perhaps if the consistency problems are resolved, the confusion about this behavior will also be resolved:

#2726
#2499
#909 (comment)
#909 (comment)
#909 (comment)

For reference:
https://docs.cypress.io/guides/guides/environment-variables.html

  1. Set in your configuration file
  2. Create a cypress.env.json
  3. Export as CYPRESS_*
  4. Pass in the CLI as --env
  5. Plugin (not covered)
  6. suite or test config (not covered)

I'm going to leave out option 5 (plugins) because that is a whole other category of running the software. Although we strictly could write cypress-specific code to resolve the fact that ENV doesn't work very well, ENV is still broken. Sticking to ENV allows job-running systems to express cypress config in a way that doesn't require perusing the test codebase. This is important for CI clarity, cross-team understanding, new developers, SREs, etc.

Option 6 is also not relevant since it's not a global setting.

Current behavior

I'll describe these in the order they are listed in the documentation. The cypress runner is exemplary for testing this because of its color-coding of the source of configuration settings.

option 0 (not using anything), default null:

$ cypress open

image

Cypress.config("baseUrl") returns null, and all visit and request calls will error unless they receive fully-qualified URLs. This means that either test code must contain the domain, or some other config setting must set baseUrl. This method is so atypical that the docs about ENV explicitly suggest setting it. In the environment setting docs, no less.

option 1, cypress.json:

$ cat cypress.json
{"baseUrl": "http://json.test.com/"}
$ cypress open

image

Works as expected. If a project wants this setting, it puts it in a json file and checks it to version control.

option 2, cypress.env.json:

$ cat cypress.json 
{"baseUrl": "http://json.test.com/"}
$ cat cypress.env.json 
{"baseUrl": "http://baseurl.test.com/", "CYPRESS_BASE_URL": "http://cybaseurl.test.com/"}
$ cypress open

image

Does not set baseUrl. A single developer cannot permanently override their project's setting for their specific task or setup. :( They've got other options, so it's not a dealbreaker.

Side note: setting {env: { baseUrl }} in cypress.json also doesn't work, though it does match cypres.env.json
image

option 3, actual ENV:

$ CYPRESS_BASE_URL="http://env.test.com/" cypress open

image

Sets baseUrl 👍
Does not work like cypress.env.json 👎
Does not set env

It actually conflicts with option 3 documentation:

Any OS-level environment variable on your machine that starts with either CYPRESS_ or cypress_ will automatically be added to Cypress’ environment variables and made available to you.
Conflicting values will override values from your configuration file (cypress.json by default) and cypress.env.json files.
Cypress will strip off the CYPRESS_ when adding your environment variables.

Of these three statements, the only one that is PARTIALLY true is the second one, but only because it sets the baseUrl. But according to previous explanations, config and ENV are separate. That's clearly not true here; setting ENV sets non-environment config and does not set cypress env. 😕

option 4, CLI (two variants):

By the color coding, we see that these variants are the same flavor of setting config. But these also behave differently from each other. One sets ENV, and the other sets the thing we meant to set (via ENV).

First variant, --env

$ cypress open --env CYPRESS_BASE_URL="http://flag.test.com"

image
Sets env, but not the setting we really care about. Similar to option 2

Second variant, --config

$ cypress open --config baseUrl="http://config.test.com"

image

Works for this setting! 🎉
Doesn't work for another other cypress env, only config. 😞

Desired behavior

These are my opinions; I would be happy with any solution that allows migrating between env-setting methods without tripping over the differences.

For option 2: cypress.env.json should behave like a local-override of cypress.json

  1. This is how .env files typically behave, and it allows local users to vary from their project default
  2. These two files are not redundant; a project file is in version control, and a local file is not. They have clear use cases.
  3. Having to implement env-parsing logic in order to override settings, forbidding simple config files, makes it difficult to migrate from one configuration method to another without a big patch.

For option 3: make it match the docs description, without special handling of CYPRESS_BASE_URL

  1. Being able to set specific settings via ENV-name-parsing is a little bit arcane, but it works.
  2. If you list specific ENV values that automatically become config values, that's no worse than having to document all the config settings at all. Other documents allude to this behavior already, but don't list those ENV that work: https://docs.cypress.io/guides/references/configuration.html#Environment-Variables
  3. The test runner has its color-coding to resolve any confusion.
  4. If ENV requires special parsing logic in order to set baseUrl, make that be the case everywhere. Don't allow option 3 to be an exception to that rule (even though it's incredibly convenient, it's confusing).

For option 4: make internal logic consistent.

  1. e.g. first set cypress.env from --env, then set cypress config from cypress.env.
  2. this allows gradual migrations from simple static settings to more complicated ENV

For option 5: genuinely this isn't a good solution to the problem. We use ENV because it's explicit and it doesn't complicate the code. I tried to change my project's default setting for my own local env, and I would have to implement an entirely different bootstrapping process for our suites in order to change one setting. I just want to inject CYPRESS_BASE_URL into our npm scripts automatically. They already wrap up cypress open or run the node-module version. Instead i'm going to be mildly annoyed typing longer CLI commands by hand.

I'm sure there are some extreme complications to changing this behavior, but right now it's not good :(

Test code to reproduce

See above

Versions

I made all my screenshots with cypress 5.1.0

@peterjaap
Copy link

I don't understand why this hasn't changed - this is causing utter confusion all over the place.

@sethbattin
Copy link
Author

@peterjaap this issue was been updated very recently with those labels, actually. They look like it has been taken into the product roadmap discussion, which is honestly all that I hoped for when I typed this up.

I would expect any real consistency fix to require a big overhaul because the current behavior is the result of a long period of incremental development. Such is software.

@lwohlhart
Copy link

+1

@cypress-app-bot
Copy link
Collaborator

This issue has not had any activity in 180 days. Cypress evolves quickly and the reported behavior should be tested on the latest version of Cypress to verify the behavior is still occurring. It will be closed in 14 days if no updates are provided.

@cypress-app-bot cypress-app-bot added the stale no activity on this issue for a long period label May 17, 2023
@sethbattin
Copy link
Author

Thanks @cypress-app-bot , I would also love an update!

@nagash77
Copy link
Contributor

Hi @sethbattin . thanks for confirming this is still relevant to your usage of Cypress. I will update with the appropriate labels and move it on to our product team for review.

@nagash77 nagash77 added type: feature New feature that does not currently exist E2E Issue related to end-to-end testing and removed stage: proposal 💡 No work has been done of this issue stale no activity on this issue for a long period labels May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E2E Issue related to end-to-end testing type: feature New feature that does not currently exist type: unexpected behavior User expected result, but got another
Projects
None yet
Development

No branches or pull requests

6 participants