chore: [Multi-origin] Correctly find the spec bridge if chromeWebSecurity set to false. #21117
Conversation
…rity set to false.
mjhenkes
added
the
topic: cy.origin
mjhenkes
requested review from
chrisbreiding,
mschile,
AtofStryker and
emilyrohrbough
|
Thanks for taking the time to open a PR!
|
|
Test summary
Run details
View run in Cypress Dashboard ➡️ FlakinessThis 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 |
| @@ -198,6 +198,15 @@ describe('cy.origin', () => { | |||
| .should('equal', 'Welcome TJohnson') | |||
| }) | |||
|
|
|||
| it('finds the correct spec bridge even if a previous spec bridge host is a subset of the current host', () => { | |||
There was a problem hiding this comment.
Given the PR title, I expect this test or a few tests to explicitly set chromeWebSecurity: false. Given we can't update that mid-test, it seems like a system-test should have been added to validate this??
There was a problem hiding this comment.
The 'positive' case of the code is tested for each origin. We verify the port, protocol, and that the host ends with the host from the spec bridge. You are correct that the 'negative' is not tested. I made the call that testing this case did not nessitate the cost of adding a system test (System tests seem expensive to me 🤷♂️ ). I can add a system test if the group disagrees with that reasoning.
There was a problem hiding this comment.
I could see the possibility of users maybe having chromeWebSecurity set to false while experimenting with cy.origin if they are trying the command out in their test suite without trying to refactor the rest of their tests, though I don't think this would be a pattern we would want to encourage. IMO, The system test could prove valuable, which we could shed the cost of once we remove chromeWebSecurity.
There was a problem hiding this comment.
I've updated an existing system test for chromeWebSecurity to also test this case.
Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>
| @@ -198,6 +198,15 @@ describe('cy.origin', () => { | |||
| .should('equal', 'Welcome TJohnson') | |||
| }) | |||
|
|
|||
| it('finds the correct spec bridge even if a previous spec bridge host is a subset of the current host', () => { | |||
There was a problem hiding this comment.
I could see the possibility of users maybe having chromeWebSecurity set to false while experimenting with cy.origin if they are trying the command out in their test suite without trying to refactor the rest of their tests, though I don't think this would be a pattern we would want to encourage. IMO, The system test could prove valuable, which we could shed the cost of once we remove chromeWebSecurity.
User facing changelog
n/a
Additional details
If chromeWebSecurity is set to false we can't rely on a security error to identify the correct spec bridge.
This PR adds some check to verify we are on the correct origin policy for the spec bridge.
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?cypress.schema.json?