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

null should be replaced with default timeout #29425

Closed
Dale-777 opened this issue Apr 28, 2024 · 2 comments
Closed

null should be replaced with default timeout #29425

Dale-777 opened this issue Apr 28, 2024 · 2 comments
Labels
stage: wontfix Cypress does not regard this as an issue or will not implement this feature

Comments

@Dale-777
Copy link

Dale-777 commented Apr 28, 2024

Current behavior

release 13.8.1 resulted in 50% of our tests failing due to tests requiring updates.

#29323
#29327
#28872

I believe the changes documented here, did not consider the use of Page Object Model, when a selector/element is referenced from the test it block (from many), when ONLY in some instances do you need to specify the timeout (often you might not need to change a timeout), and thus you do not bother to pass a value. This is a valid, and I am sure a common use case.

Screenshot 2024-04-28 at 21 21 23

Desired behavior

Rather when null it should be set to the default timeout rather than fail with the error

Test code to reproduce

File 1 (test 1):

globalNavigation.elements.companyNameDropdown().should("be.visible");

File 2 (test 2):

globalNavigation.elements.companyNameDropdown(90000).should("be.visible");

File 3 (page object):

  elements = {
    companyNameDropdown: (timeout?: number) => cy.get('[data-cy="avatar"]', { timeout })
  };

Cypress Version

13.8.1

Node version

v18.16.0

Operating System

All OS

Debug Logs

rolling back to 13.8.0, resolves the issue

Other

No response

@Dale-777
Copy link
Author

@jennifer-shehane
Copy link
Member

Hi @Dale-777, thanks for reaching out. I'm sorry this resulted in unexpectedly breaking tests.

The only valid option for timeout has always been a Number. The addition of validations was to ensure one wouldn't get into a situation where Cypress would hang with invalid options passed to timeout as seen in this issue: #29323

This is not a change of any of our behavior as it has always only ever accepted a Number. We don't intend to make an update to the logic of timeout to accept a null or undefined value for timeout as this may cause confusion if that were introduced. Does undefined mean they want it to be 0 timeout or to use the default?

We suggest updating any custom commands for Page Object Models to handle when the timeout option is not passed via the calling function. An example is shown below that falls back to default timeout if it is not present.

const elements = {
  companyNameDropdown: (timeout) => cy.get('[data-cy="avatar"]', (timeout ? { timeout } : {}))
}

it('test without timeout', () => {
  elements.companyNameDropdown().should("be.visible")
})

it('test with timeout', () => {
  elements.companyNameDropdown(7000).should("be.visible")
})

Screenshot 2024-04-29 at 12 02 30 PM

@jennifer-shehane jennifer-shehane closed this as not planned Won't fix, can't repro, duplicate, stale Apr 29, 2024
@jennifer-shehane jennifer-shehane added the stage: wontfix Cypress does not regard this as an issue or will not implement this feature label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage: wontfix Cypress does not regard this as an issue or will not implement this feature
Projects
None yet
Development

No branches or pull requests

2 participants