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

Callback form of should chained off cy.window receives null when retried #22587

Closed
david-sabata opened this issue Jun 29, 2022 · 8 comments · Fixed by #23734
Closed

Callback form of should chained off cy.window receives null when retried #22587

david-sabata opened this issue Jun 29, 2022 · 8 comments · Fixed by #23734

Comments

@david-sabata
Copy link

david-sabata commented Jun 29, 2022

Current behavior

My goal is to have a retriable check of a window property that my app exposes. To achieve that, I'm using the callback form of should.

The window object is correctly passed into the callback only for the first time. If the assertions fail and trigger a retry, the callback argument is null. (See the repro code)

Maybe I missed something, but I went through the docs and I don't think this is expected behavior.

image

Desired behavior

The callback of should always receives the window object

Test code to reproduce

it('retries should on window object', () => {
    cy.window().should(win => {
      // this logs <window> on the first run, but null on the next
      cy.log('window =', win).then(() => {
        // failing to trigger a retry
        expect(true).to.equal(false)
      })
    })
})

Cypress Version

10.3.0

Other

No response

@david-sabata
Copy link
Author

Possibly related to #8346 ?

@cypress-bot cypress-bot bot added the stage: investigating Someone from Cypress is looking into this label Jun 29, 2022
@emilyrohrbough
Copy link
Member

@david-sabata The issue you have logged is indeed a duplicate of #8346 (thank you for linking!) The cy.log() has a bug because it logs twice.

With the example you linked out, because the .then() command fails, it won't actually trigger a retry.

I assume your example is a rough high-level example of the behavior you are trying to accomplish? For the cy.window() command to re-poll when the .should() command fails, you'll want to stick with sync assertions and avoid non-cypress commands.

@cypress-bot cypress-bot bot added stage: awaiting response Potential fix was proposed; awaiting response and removed stage: investigating Someone from Cypress is looking into this labels Jul 1, 2022
@david-sabata
Copy link
Author

david-sabata commented Jul 4, 2022

I don't mind logging twice :) The unexpected part is that should's target changes when retried and becomes null. That makes the test fail (correctly), but at the wrong place, leading to confusing errors. And according to the docs, should should keep its target when retried.

It does seem to be connected with log, because it works properly without it. But the issue isn't the logging.

Here's a simplified example and screenshot. Notice how the error isn't about the missing property, but about Target cannot be null or undefined

it.only('retries should on window object', () => {
	cy.window().should(win => {
		// this logs <window> on the first run, but null on the next
		cy.log('window =', win)

		// when retried, win is null unexpectedly
		expect(win).to.haveOwnProperty('foo')
	})
})

image

@cypress-bot cypress-bot bot added stage: investigating Someone from Cypress is looking into this and removed stage: awaiting response Potential fix was proposed; awaiting response labels Jul 11, 2022
@BlueWinds
Copy link
Contributor

Very interesting, thanks for the precise repro. I'm currently working in this area - how subjects are determined and how commands are retried - in relation to #7306.

The short version is that should is not really designed for this use case, and we don't provide anything that is designed for this use case either. ^^;; I'm aiming to address that in my work. I'm going to assign myself here so I remember to follow up on this specific case.

A temporary workaround would be to ensure that you don't have any cypress commands inside your should callback:

it.only('retries should on window object', () => {
      let count = 0
      cy.window().should(win => {
        count++
        if (count < 4) { throw new Error('foobar') }
        expect(win).to.haveOwnProperty('location')
      })
    })

This passes; it's only when the should callback contains other cypress commands that cy gets confused over what the subject should be during retries.

@BlueWinds
Copy link
Contributor

For anyone following along, this will be "fixed" in 11.0.0. Fixed is in quotes because it'll be fixed by completely disallowing Cypress commands from inside .should(), throwing an explicit error that this is unsupported rather than doing weird things (like logging twice, losing track of the subject, etc).

@mschile
Copy link
Contributor

mschile commented Nov 7, 2022

The code for this is done in #23734, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@mschile mschile closed this as completed Nov 7, 2022
@emilyrohrbough
Copy link
Member

emilyrohrbough commented Nov 15, 2022

This was released with 11.0.0.
EDIT: This was NOT released with v11.0.0. This will be released in v12.0.0.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Dec 6, 2022

Released in 12.0.0.

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

@cypress-bot cypress-bot bot removed the stage: needs review The PR code is done & tested, needs review label Dec 6, 2022
@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Dec 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants