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

should is not retrying when chained after then with no commands and returning undefined/null/void #30230

Open
DCzajkowski opened this issue Sep 12, 2024 · 3 comments
Labels
type: breaking change Requires a new major release version

Comments

@DCzajkowski
Copy link
Contributor

DCzajkowski commented Sep 12, 2024

Current behavior

When calling .should after .then, that does not call any Cypress commands and returns undefined/null/void, the should assertion is not retried.

According to the docs, when void is returned from .then, and no calls to any Cypress commands are made, the subject will not be modified.

However, there is a change in behavior when using .then compared to when not using it or explicitly wrapping an element in cy.wrap.

Desired behavior

.should should be always retried, regardless if .then was used or not

Test code to reproduce

Good

cy
  //
  .get('body')
  .should(() => {
    expect(true).to.be.false // retries as expected
  })
cy
  //
  .get('body')
  .then(($el) => {
    cy.wrap($el) // this yields `body` as a next subject
  })
  .should(() => {
    expect(true).to.be.false // retries as expected
  })

Bad, here is a bug

cy
  //
  .get('body')
  .then(($el) => {
    // not calling any Cypress commands nor returning anything
  })
  .should(() => {
    expect(true).to.be.false // called just one time, no retries, fails
  })

Cypress Version

13.14.1

Node version

v20.16.0

Operating System

macOS Sonoma 14.6.1

Debug Logs

No response

Other

Keywords: no retry, should, assertion, not retrying, after then, implicit subject

@Dynalon
Copy link

Dynalon commented Oct 22, 2024

This bit me too and took me days to figure out, as it used .then() calls in my beforeEach() with various nesting levels of those beforeEach() - nasty bug.

@jennifer-shehane jennifer-shehane added type: enhancement Requested enhancement of existing feature type: breaking change Requires a new major release version and removed type: bug type: enhancement Requested enhancement of existing feature labels Nov 5, 2024
@jennifer-shehane
Copy link
Member

@DCzajkowski What's the use case for needing the should to retry here? The example doesn't really highlight why that's needed.

@DCzajkowski
Copy link
Contributor Author

@jennifer-shehane The reason depends on the specific situation.

One example could be that you need to do some imperative action in .then that cannot be done using regular .click, .invoke etc.

cy
  .get('button')
  // this `.click()` can be anything
  .then(($el) => { $el.click() })
  .should(/* any assertion here will fail unless the change to the button happens synchronously */)

Our specific use-case is that we are modifying the state of redux inside .then and want to continue with our checks when the state changes. We would wait for something to change in the DOM (with a .should assertion) and proceed when it succeeds. But because .should would not retry, it would sometimes fail, as the rerender hasn't happened yet.

This bug means we have to be very careful what we chain the .should assertion off of.


The fact .should behaves differently depending on what's up in the chain is very unintuitive. It's also not documented anywhere in the docs — the docs suggest that returning void from .then does not modify the subject, but there is a change in behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: breaking change Requires a new major release version
Projects
None yet
Development

No branches or pull requests

3 participants