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: restart Cypress server and browser on baseUrl change #22154

Merged
merged 14 commits into from Jun 10, 2022

Conversation

ryanthemanuel
Copy link
Collaborator

User facing changelog

The baseUrl config option will now restart the Cypress server automatically when it is updated in a Cypress config file.

Additional details

There are several things that use the baseUrl during the server's initialization like the browser and xhr urls and remote states. In order to properly ensure they are reinitialized appropriately, we will need to restart the server. Thus, we change baseUrl's requireRestartOnChange property to server.

Steps to test

  1. cypress open a project with a baseUrl
  2. Execute a test within the project
  3. Update the baseUrl in cypress.config.js
  4. Ensure that the server/browser relaunches and that the url in the browser references the updated baseUrl

How has the user experience changed?

Before

baseUrl.-.Before.mov

After

baseUrl.-.After.mov

PR Tasks

  • Have tests been added/updated?
  • Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?

@ryanthemanuel ryanthemanuel requested review from tgriesser and a team as code owners June 7, 2022 00:33
@ryanthemanuel ryanthemanuel requested review from jennifer-shehane and removed request for a team June 7, 2022 00:33
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 7, 2022

Thanks for taking the time to open a PR!

@@ -190,7 +190,7 @@ describe('Choose a Browser Page', () => {
cy.get('h1').should('contain', 'Choose a Browser')
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are for unrelated flake.

@cypress
Copy link

cypress bot commented Jun 7, 2022



Test summary

37553 0 454 0Flakiness 3


Run details

Project cypress
Status Passed
Commit d226315
Started Jun 10, 2022 2:45 PM
Ended Jun 10, 2022 3:02 PM
Duration 17:06 💡
OS Linux Debian - 10.11
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

commands/actions/click.cy.js Flakiness
1 ... > scroll-behavior > can scroll to and click elements in html with scroll-behavior: smooth
cypress/proxy-logging.cy.ts Flakiness
1 Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second
2 Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@ryanthemanuel ryanthemanuel changed the title fix: restart server on baseUrl change fix: restart Cypress server and browser on baseUrl change Jun 7, 2022
@ryanthemanuel ryanthemanuel requested review from ZachJW34 and removed request for jennifer-shehane June 7, 2022 14:14
Copy link
Member

@tgriesser tgriesser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice - changes look pretty straightforward to me

@tgriesser tgriesser added the PATCH label Jun 7, 2022
@rockhold rockhold requested a review from tbiethman June 7, 2022 16:08
Copy link
Contributor

@tbiethman tbiethman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as advertised 👍

@ZachJW34
Copy link
Contributor

ZachJW34 commented Jun 7, 2022

Just wanted to call out #22072. We have baseUrl as a property that can be changed at the test level, but with these changes it looks like the documentation would be incorrect.

@mjhenkes mjhenkes merged commit 1e61923 into develop Jun 10, 2022
@mjhenkes mjhenkes deleted the ryanm/fix/restart-server-on-baseUrl-change branch June 10, 2022 15:11
@mjhenkes mjhenkes removed the PATCH label Jun 10, 2022
BeijiYang pushed a commit to BeijiYang/cypress that referenced this pull request Jun 23, 2022
…#22154)

* fix: restart server on baseUrl change

* Rework how baseUrl works

* Refactor how we determine if we should ping the base url

* Fix test

* Update packages/launchpad/cypress/e2e/choose-a-browser.cy.ts

* Update packages/launchpad/cypress/e2e/choose-a-browser.cy.ts

Co-authored-by: Matt Henkes <mjhenkes@gmail.com>
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.

Changing the baseUrl value in cypress.config.js will not take effect until restarting cypress
6 participants