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

Cannot overwrite get command #7267

Closed
ncknuna opened this issue May 7, 2020 · 3 comments
Closed

Cannot overwrite get command #7267

ncknuna opened this issue May 7, 2020 · 3 comments

Comments

@ncknuna
Copy link

ncknuna commented May 7, 2020

Situation: There are some things we would almost always want to assert on the elements we find, most notably that the element(s) is visible and the number that we find is what we expect (defaulting to 1). I want to add this behavior to the default get command rather than creating a separate command, so more junior developers don't end up using the wrong one and then miss the default assertions.

Current behavior:

Unable to overwrite get command. In case it's unclear, I created a minimal repro repo, and this file in it has the two different things I tried:

// This doesn't work with actions.spec.js because it says
// originalFn(...).should is not a function
Cypress.Commands.overwrite("get", (originalFn, selector, options) => {
  return originalFn(selector, options).should("be.visible");
});

// This doesn't work with assertions.spec.js because .contains fails with

// Cypress detected that you returned a promise from a command while also invoking one or more cy commands in that promise.
// The command that returned the promise was:

//   > cy.contains()

// The cy command you invoked inside the promise was:

//   > cy.wrap()

// Because Cypress commands are already promise-like, you don't need to wrap them or return your own promise.

// Cypress will resolve your command with whatever the final Cypress command yields.

// The reason this is an error instead of a warning is because Cypress internally queues commands serially whereas Promises execute as soon as they are invoked. Attempting to reconcile this would prevent Cypress from ever resolving
Cypress.Commands.overwrite("get", (originalFn, selector, options) => {
  return cy.wrap(originalFn(selector, options)).should("be.visible");
});

Desired behavior:

Cypress allows me to overwrite get, or at least gives me a more informative error message about why that isn't supported

Test code to reproduce

https://github.com/ncknuna/cypress-no-get-overwrite

Versions

Cypress version 4.5.0
Mac OS X v 10.15.4
Chrome Version 80.0.3987.163

@jennifer-shehane
Copy link
Member

You would basically need to reimplement what Cypress does in our own command definitions if you want to overwrite the .get() command to ensure an element is visible.

Overwrite should always return the originalFn, so you can't chain something off of it like .should().

In this situation I would recommend just creating a new custom command for this usage since you would need to use cy.get() within the command itself in order to chain off of it, which would create an infinite loop inside of the get overwrite.

Cypress.Commands.add("getVisible", (selector, options) => {
  return cy.get(selector).should('be.visible')
});

it("get overwrite", () => {
  cy.visit("https://example.cypress.io/commands/actions");
  cy.getVisible(".action-email")
})

@cypress-bot cypress-bot bot added the stage: awaiting response Potential fix was proposed; awaiting response label May 12, 2020
@jennifer-shehane
Copy link
Member

Unfortunately we have to close this issue due to inactivity. Please comment if there is new information to provide concerning the original issue and we can reopen.

@jennifer-shehane jennifer-shehane removed the stage: awaiting response Potential fix was proposed; awaiting response label May 18, 2020
@ncknuna
Copy link
Author

ncknuna commented May 18, 2020

@jennifer-shehane Apologies for the delay in replying. Just for my education, is

Overwrite should always return the originalFn, so you can't chain something off of it like .should().

somewhere in the docs? If not, would you be open to explaining why that's the case? I'd also be happy to send an MR for a docs update once I do understand. Thanks so much!

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

No branches or pull requests

2 participants