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

Cypress doesn't match the newest route stub when url matches previous route #3248

Closed
nitzansan opened this issue Jan 30, 2019 · 13 comments
Closed

Comments

@nitzansan
Copy link

nitzansan commented Jan 30, 2019

Current behavior:

When having multiple routes with paths that match a request, the newest one assigned does not match and a different stub is used.
E.g - (with the following endpoint: mysite.com/entitiesCollection)

cy.route('mysite.com/entitiesCollection**', response1).as('firstCall');
// something happens here that triggers the call to the relevant url --
cy.wait('@firstCall') // Success!
cy.route('mysite.com/entitiesCollection**page[offset]=20**', response2).as('secondCall');
// something happens here that triggers the call to the relevant url --
cy.wait('@secondCall') // Fails since cypress recognizes the xhr as `firstCall`

Desired behavior:

When matching routes, newer should take precedence over older.

Steps to reproduce: (app code and test code)

See the example above

Versions

cypress 3.1.4, OSX Mojave, Chrome 71

@jennifer-shehane
Copy link
Member

The alias does indeed match the first route found that it matches. We would have to think about changing this behavior.

For now, you should likely update your route url to something more specific for each route. Perhaps use REGEXP to match the first one with /mysite.com\/entitiesCollection$/ or something similar depending on what you are matching against, so that it does not match both routes.

@jennifer-shehane jennifer-shehane added the stage: proposal 💡 No work has been done of this issue label Jan 30, 2019
@nitzansan
Copy link
Author

nitzansan commented Jan 30, 2019

There are several workarounds that I can do indeed, just that the behavior that would seem more intuitive is that the newest route matching the path would be triggered and not the oldest (aka "first").

This is more intuitive for several reasons:

  1. Currently you support overriding a route with identical paths, for example:
    cy.route('mysite.com/entitiesCollection**', response1).as('firstCall');
    cy.route('mysite.com/entitiesCollection**', response2).as('secondCall');
    // something happens here that triggers the call to the relevant url --
    cy.wait('@secondCall') // Success!
    This kind of behavior implies precedence of newer over older
  2. The locality-in-time of adding a route makes more sense to match them from newer to older. For example, if I have 100 routes and I just assigned a new route (regardless of the path), it is far more likely that I'm going to trigger the 101th route than any of the previous ones, and to go and test 100 routes prior is less efficient.
  3. Similar to that, when your cypress tests are becoming bigger and more complex, you're using functions and test-units that other people wrote, which might have routes assigned inside them.

If I explicitly write a route in one of my tests that consume a shared logic, I expect my route (which is newer) to take precedence. (This is actually the case I have right now, that I now need to edit shared testing code because of the route match order).

@nitzansan
Copy link
Author

nitzansan commented Jan 30, 2019

Also related to this issue - I was looking for a way to cancel/remove a cy.route assignment and didn't find anything.

Is such a thing possible?

@jennifer-shehane
Copy link
Member

@nitzansan Thanks for the thorough explanation for this feature. We'll have to discuss this proposal and the potential impact of these changes as a team.

I was looking for a way to cancel/remove a cy.route assignment and didn't find anything.

This is a good question. You want to remove the alias .as() assignment right? I can't think of a way to do this. 🤔

@jennifer-shehane jennifer-shehane added type: breaking change Requires a new major release version pkg/driver This is due to an issue in the packages/driver directory labels Jan 31, 2019
@nitzansan
Copy link
Author

I want to remove the entire cy.route() assignment based on it's alias or exact path.
I think having the ability to do this can help mitigate the problem I've described before.

@nitzansan
Copy link
Author

nitzansan commented Jan 31, 2019

Another find regarding this issue - apparently when passing a regex to cy.route it has precedence over glob patterns and regex pattern routes indeed match in the expected behavior (newer beats older).
e.g:

cy.route('mysite.com/entitiesCollection**', response1).as('firstCall');
// something happens here that triggers the call to the relevant url --
cy.wait('@firstCall') // Success!
cy.route(new RegExp('/mysite.com/entitiesCollection.*page[offset]=20.*'), response2).as('secondCall');
// something happens here that triggers the call to the relevant url with page[offset]=20--
cy.wait('@secondCall') // Success
cy.route(new RegExp('/mysite.com/entitiesCollection.*page\[offset\]=20\&page\[limit\]=100.*'), response3).as('thirdCall');
// something happens here that triggers the call to the url + page[offset]=20&page[limit]=100 query params
cy.wait('@thirdCall') // Success

@jennifer-shehane
Copy link
Member

Yes, it does appear that routes that match RegExp would have precedence over routes that match by string. https://github.com/cypress-io/cypress/blob/develop/packages/driver/src/cypress/server.coffee#L266

@jennifer-shehane jennifer-shehane changed the title Cypress doesn't match the newest route stub Cypress doesn't match the newest route stub when url matches previous route Mar 26, 2019
@metas-ts
Copy link

metas-ts commented May 7, 2019

Another find regarding this issue - apparently when passing a regex to cy.route it has precedence over glob patterns and regex pattern routes indeed match in the expected behavior (newer beats older).

Thanks a lot for this hint! While actually waiting for #387 I'm going to try changing my route urls to Regex now. Hope that works.

metas-ts added a commit to metasfresh/metasfresh-e2e-legacy that referenced this issue May 7, 2019
* cypress.json - generally set requestTimeout=30secs (see comment)
* sales_order_spec.js - specifically wait for the POST to quickInput (that finally resets the quickinput fields)
  * we have a bunch of less specific aliases in this test and don't want to accidentally match any of those
* form.js and general.js - change route url globs to regexes, because hopefully that way the newest route is matched as the preferred one; see cypress-io/cypress#3248 (comment)
https://github.com/metasfresh/metasfresh-e2e/issues/
@metas-ts
Copy link

Another find regarding this issue - apparently when passing a regex to cy.route it has precedence over glob patterns and regex pattern routes indeed match in the expected behavior (newer beats older).

Unfortunately this didn't work out either..
@jennifer-shehane is there any hint you can give, regarding in which order the routes are matched? E.g. is it worth trying to prefix them such that their creation order is related to their lexical order?

@hakuna0829
Copy link

Is there any solution to support multi cy.route for same request url?

@jennifer-shehane
Copy link
Member

Going to merge this issue into this duplicate issue with a reproducible example. #4460

@jennifer-shehane jennifer-shehane removed pkg/driver This is due to an issue in the packages/driver directory stage: proposal 💡 No work has been done of this issue type: breaking change Requires a new major release version labels Jul 23, 2019
@front-end-developer
Copy link

Hi All,

I still see this issue in the latest version of Cypress v4.11.0,

its now Aug 2020, has this been solved?

@jennifer-shehane
Copy link
Member

@front-end-developer Please see this issue: #4460

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

5 participants