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

feat: ability to specify the default browser in cypress config #30517

Merged
merged 49 commits into from
Nov 14, 2024

Conversation

alexsch01
Copy link
Contributor

@alexsch01 alexsch01 commented Nov 1, 2024

Additional details

  • Why was this change necessary?

I would like every cypress test I write to go against Chrome instead of Electron

  • What is affected by this change?

cypress config file option "defaultBrowser" will be used for this so it will have to be documented -
cypress-io/cypress-documentation#5991

putting in "defaultBrowser": "chrome" in the cypress config file will work the same way as "--browser chrome" in the CLI

  • Any implementation details to explain?

configDefaultBrowser gets the "defaultBrowser" attribute from the object created from the cypress config file

The browser option in the cypress configuration file is only used if configDefaultBrowser exists and if '--browser' / '-b' is not part of the CLI args list.

Steps to test

  • In a Cypress project, put defaultBrowser: "chrome" in their cypress config file
  • npx cypress run
  • Notice how it launches Google Chrome
  • In a Cypress project, remove defaultBrowser: "chrome" from their cypress config file
  • npx cypress run
  • Notice how it launches Electron

How has the user experience changed?

User can pass in defaultBrowser: "BROWSER" to their cypress config file and they won't need to pass in --browser BROWSER to launch BROWSER

  • User can still pass in --browser electron to open the Electron browser if cypress config file has the option defaultBrowser set

PR Tasks

@cypress-app-bot
Copy link
Collaborator

@alexsch01 alexsch01 mentioned this pull request Nov 1, 2024
@alexsch01
Copy link
Contributor Author

alexsch01 commented Nov 3, 2024

@jennifer-shehane can you review this? I have simplified the implementation as much as I can. It is still using defaultBrowser as the option since Cypress internally already uses the browser option for something else

After you review it, I'll make a PR for the cypress-documentation repo

@alexsch01
Copy link
Contributor Author

alexsch01 commented Nov 4, 2024

run-webpack-dev-server-integration-tests
seems unrelated to my change

image

driver-integration-tests-webkit
unsure if this PR changes this behavior

image

* oops

* oops
@mschile mschile requested review from mschile and cacieprins November 5, 2024 17:04
@alexsch01
Copy link
Contributor Author

alexsch01 commented Nov 5, 2024

@jennifer-shehane can you allow the contributor-pr to run? weird why it needs approval when you did the commit

@alexsch01
Copy link
Contributor Author

alexsch01 commented Nov 6, 2024

I just fixed the system-test for defaultBrowser, ran the test successfully locally

@mschile
Copy link
Contributor

mschile commented Nov 6, 2024

Hi @alexsch01 👋, thanks for this contribution! I approved the contributor-pr in circleci to kick off the rest of the workflow. It appears there are some unit tests that need to be updated to account for the new config value.

@alexsch01
Copy link
Contributor Author

driver-integration-tests-electron (2718711) - cypress-io/cypress
doesn't seem related
image

@mschile
Copy link
Contributor

mschile commented Nov 6, 2024

driver-integration-tests-electron (2718711) - cypress-io/cypress doesn't seem related image

Yeah, don't worry about that one but the other ones appear to be related.

* make test run with all versions of system-tests

* Update index.spec.ts.js

* Update index.spec.ts.js

* Update utils.spec.ts

* Update results_spec.ts.js
@alexsch01
Copy link
Contributor Author

unit-tests

lerna ERR! yarn test exited 1 in '@packages/https-proxy'
image

Not related to my changes

@alexsch01 alexsch01 requested a review from mschile November 13, 2024 16:02
@alexsch01 alexsch01 changed the title feat: ability to specify the default browser in cypress config file feat: ability to specify the default browser in cypress config Nov 13, 2024
@alexsch01
Copy link
Contributor Author

run-app-integration-tests-chrome
Screenshot_20241113-193300

@mschile
Copy link
Contributor

mschile commented Nov 14, 2024

I'll take a look.

@alexsch01
Copy link
Contributor Author

alexsch01 commented Nov 14, 2024

FYI: I updated isBrowserGivenByCli so it gets set regardless of which mode you are in

packages/server/lib/modes/index.ts

@alexsch01
Copy link
Contributor Author

system-tests-chrome
don't believe it is related to this PR
image

@alexsch01
Copy link
Contributor Author

Thanks for re-running, all tests passed!

@mschile mschile merged commit 82f45c5 into cypress-io:develop Nov 14, 2024
69 checks passed
@mschile
Copy link
Contributor

mschile commented Nov 14, 2024

@alexsch01, thanks again for the contribution! 🎉

@alexsch01
Copy link
Contributor Author

I tested the beta build for this PR and it works great!

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 19, 2024

Released in 13.16.0.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Nov 19, 2024
@alexsch01 alexsch01 deleted the defaultBrowser-config branch November 21, 2024 11:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot specify the default running browser in cypress configuration file
5 participants