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: commands not chained off of cy.get() or queries properly error when cy.origin() is not used and the AUT has navigated away from the primary origin #30858

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

AtofStryker
Copy link
Contributor

@AtofStryker AtofStryker commented Jan 10, 2025

Additional details

While performing the Cypress 14 bug hunt, @jennifer-shehane ran into an issue where this test was failing:

it(`Verify that items under category News are visible, intentionally will fail`, () => {
  cy.visit("https://ca.yahoo.com/")
  
  cy.findByRole("link", {name: 'News'})
      .should("be.visible")
      .click({force: true})
  
  cy.scrollTo("bottom")
})

This has to due with the AUT navigating away from ca.yahoo.com to ca.news.yahoo.com, which before worked OK since we would inject document domain and Cypress could still communicate with the subdomain of the navigated AUT, even though the frame is in a cross-origin (same site) context. Now that we do not inject document domain be default, scrollTo was trying to be run in a cross-origin (same-site) context, which would fail. When Cypress attempts to access parts of the error, it is unable to due to the frame being in a cross-origin context, which throws the Can't set property message of which only has a getter error.

This is no unique to the cy.scrollTo() command. The cy.window(), cy.document(), cy.title(), cy.url(), cy.location() ,cy.hash(), cy.go(), and cy.reload() will throw a similar error in this situation, or return erroneous values as the commands cannot access the AUT. This commands are exceptions because they do not chain off a queryable to run, like .get(), which checks to make sure the command is being run on the correct origin.

To fix this test, the test needs to be:

it(`Verify that items under category News are visible, now will pass`, () => {
  cy.visit("https://ca.yahoo.com/")
  
  cy.findByRole("link", {name: 'News'})
      .should("be.visible")
      .click({force: true})
  
  cy.origin('https://ca.news.yahoo.com', () => {
    cy.scrollTo("bottom")
  })
})

However, the Can't set property message of which only has a getter should not occur in the first place and the user should be prompted on how to fix the issue. To fix this, Cypress should error when the above commands are run when the AUT has navigated away from the primary origin to prompt the user to fix where the command is being run. This gives the user context that they need cy.origin() to run the command correctly.

Steps to test

Either run the sample test with the published binary (only linux built on this PR so if testing on a different machine check out the code and run in global mode against a project with this test) or remove that added code and run the regression tests in actions.cy.ts to see the commands are allowed to run outside the origin context, which is incorrect.

How has the user experience changed?

Now the scrollTo in our described test works correctly.

Before
Screenshot 2025-01-13 at 10 16 52 AM Screenshot 2025-01-13 at 10 16 35 AM
After
Screenshot 2025-01-13 at 10 15 25 AM Screenshot 2025-01-13 at 10 15 16 AM

PR Tasks

Copy link

cypress bot commented Jan 10, 2025

cypress    Run #59869

Run Properties:  status check passed Passed #59869  •  git commit 09b006d33a: fix: make sure scrollTo, window, document, title, go, reload, location, hash, an...
Project cypress
Branch Review fix/check_actionability_on_try
Run status status check passed Passed #59869
Run duration 10m 07s
Commit git commit 09b006d33a: fix: make sure scrollTo, window, document, title, go, reload, location, hash, an...
Committer AtofStryker
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 14
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 810
View all changes introduced in this branch ↗︎

Warning

No Report: Something went wrong and we could not generate a report for the Application Quality products.

@AtofStryker AtofStryker force-pushed the fix/check_actionability_on_try branch 2 times, most recently from 63dcfef to a85b64a Compare January 10, 2025 22:56
@AtofStryker AtofStryker changed the title fix: fix actionability on retry to make sure Cypress and communicate with AUT [run ci] fix: actionability on retry to make sure Cypress and communicate with AUT [run ci] Jan 10, 2025
@AtofStryker AtofStryker force-pushed the fix/check_actionability_on_try branch 4 times, most recently from 87e6010 to 36e3b89 Compare January 13, 2025 14:57
@AtofStryker AtofStryker changed the title fix: actionability on retry to make sure Cypress and communicate with AUT [run ci] fix: commands not chained off of cy.get() or queries properly error when cy.origin() is not used and the AUT has navigated away from the primary origin Jan 13, 2025
@AtofStryker AtofStryker self-assigned this Jan 13, 2025
@AtofStryker AtofStryker force-pushed the fix/check_actionability_on_try branch from 36e3b89 to d2aaba8 Compare January 13, 2025 15:23
@AtofStryker AtofStryker force-pushed the fix/check_actionability_on_try branch from d2aaba8 to 237f950 Compare January 13, 2025 15:25
…n, hash, and url commands can communicate with the AUT
@AtofStryker AtofStryker force-pushed the fix/check_actionability_on_try branch from 237f950 to 09b006d Compare January 13, 2025 20:01
Copy link
Collaborator

@ryanthemanuel ryanthemanuel left a comment

Choose a reason for hiding this comment

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

Looks good. Nice work!

@AtofStryker AtofStryker merged commit 249cfde into develop Jan 13, 2025
86 of 88 checks passed
@AtofStryker AtofStryker deleted the fix/check_actionability_on_try branch January 13, 2025 20:57
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.

Testing Library can fails with a cryptic error rather than the expected error in Cy 14 Prerelease
3 participants