cypress-io / cypress

Fast, easy and reliable testing for anything that runs in a browser.
https://cypress.io
MIT License
46.8k stars 3.17k forks source link

Overwritten cy.click() function not executing originalFn from within a Cypress.Promise #25149

Open ryanshellberg opened 1 year ago

ryanshellberg commented 1 year ago

Current behavior

My team uses Turbo Drive and has been using the solution discussed here to overwrite the cy.click() function wait wait for the turbo:load event before proceeding. After upgrading to Cypress 12.0.2 our overwritten function no longer executes the original click, and the promise never returns.

Desired behavior

Execute the original click function correctly, resolving the promise after the "turbo:load" event is fired.

Test code to reproduce

Cypress.Commands.overwrite("click", (originalFn, subject, options) => {
  // Check if turbo could be active for this click event - could be improved
  if (options?.turbo) {
    // Wait for turbo to finish loading the page before proceeding with the next Cypress instructions.
    // First, get the document
    cy.document({ log: false }).then(($document) => {
      Cypress.log({
        $el: subject,
        name: "click",
        displayName: "click",
        message: "click and wait for page to load",
        consoleProps: () => ({ subject: subject }),
      });
      // Make Cypress wait for this promise which waits for the turbo:load event
      return new Cypress.Promise((resolve) => {
        // Once we receive the event,
        const onTurboLoad = () => {
          // clean up
          $document.removeEventListener("turbo:load", onTurboLoad);
          // signal to Cypress that we're done
          resolve();
        };
        // Add our logic as event listener
        $document.addEventListener("turbo:load", onTurboLoad);
        // Finally, we are ready to perform the actual click operation
        originalFn(subject, options);
      });
    });
  } else {
    return originalFn(subject, options);
  }
});

With this command overridden, I can reproduce with the following code called on any element that triggers a turbo load

cy.get("button")
  .click({ turbo: true });

Cypress Version

12.0.2

Node version

16.13.2

Operating System

macOS 13.0.1

Debug Logs

No response

Other

No response

Moumouls commented 1 year ago

Hi @BlueWinds @ryanshellberg

I can confirm the regression.

A simple use case to reproduce the issue of .click overwrite not working

Cypress.Commands.overwrite(
    'click',
    (originalFn, subject, positionOrX, y, options = {}) => {
        return cy
            .screenshot()
            .then(() => originalFn(subject, positionOrX, y, options))
    },
)

We use this kind of code to generate screenshot documentation based on clicks on our test suite.

BlueWinds commented 1 year ago

Figured out what's going on here, and it's pretty messy, deep in the Cypress internals. The tl;dr of it is that the originalFn is reading the wrong subject, and getting the subject of .screenshot() (which is null) rather than the one of the parent command.

I have a PR to address this, but it's a really messy edge case caused by the overwrite API having been written (and therefore set in stone) before queries / subject chains existed.

BlueWinds commented 1 year ago

Apologies for letting this languish - it is still on my radar, but I've been out sick a lot recently. Will get back to this as soon as I'm able.

ryanshellberg commented 1 year ago

@BlueWinds any updates on this?

nijikon commented 2 months ago

@jennifer-shehane Is there a plan to fix this, or is there a workaround I can use in the meantime?

jennifer-shehane commented 2 months ago

This isn't currently something our team is focused on addressing.