Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

ngMock - why does $httpBackend match the first and not the last mock definition? #11637

Closed
szimek opened this issue Apr 17, 2015 · 9 comments
Closed

Comments

@szimek
Copy link

szimek commented Apr 17, 2015

Is there any reason why $httpBackend matches the first and not the last mock definition? If it worked the other way around (i.e. $httpBackend.when added new mock definitions to the beginning of definitions array instead of the end of it) it would be much easier to overwrite already mocked requests.

I've got some default mock definitions common for almost all tests that are loaded before each test and would like to be able to easily overwrite them in some specific tests.

I wanted to prepare PR that changes this, but there's even a unit test that explicitly checks for this behavior.

EDIT: In ngMock module unit tests like should match headers if specified or should match data if specified, the first mocks are more specific than the later ones. In a real app, I'd expect something opposite - one would create general mocks for most tests and then define more specific ones in selected tests.

@szimek szimek changed the title ngMock - why does $httpBackend.when match the first and not the last mock definition? ngMock - why does $httpBackend match the first and not the last mock definition? Apr 18, 2015
@petebacondarwin
Copy link
Contributor

I think this sounds like a good idea, but of course it would be quite a breaking change to applications.
Let's see if the rest of the team agrees and if so then aim to put it into 1.5

@drewzh
Copy link

drewzh commented Aug 17, 2015

This is actually a really annoying feature when it comes to E2E testing with Protactor, as currently there's no module available that will allow you to keep reference to the promise returned by the mock $httpBackend to re-purpose the redirect after it's already been set (I'm currently using https://www.npmjs.com/package/http-backend-proxy to allow interaction with $httpBackend from my Protractor tests).

So essentially, it's impossible to test a success response and then immediately test a failure from the same API endpoint without having to do a full browser refresh to clear down the intercepts.

Allowing the option of matching the last added regex instead of the first would make this kind of testing trivial. But essentially for me at the moment, integrating Protractor into my tests is impossible due to the high number of tests that can't be changed easily.

@szimek
Copy link
Author

szimek commented Dec 8, 2015

@petebacondarwin Are there still any chances this could land in 1.5, as there already were 2 beta releases?
How does it work in Angular 2? Is there the same problem?

@Narretz Narretz self-assigned this Feb 2, 2017
@yogeshgadge
Copy link

Why don't we just add something like resetDefinitions() along the lines of resetExpectations() ? This will at-least help in setting new definitions.

@szimek
Copy link
Author

szimek commented Mar 7, 2017

You could also add it as an option that would default to the current code, so it would be backwards compatible. It's really just a matter of changing push to unshift in one place (which we do ourselves after every release of Angular 1.x 😉 )

@yogeshgadge
Copy link

yogeshgadge commented Mar 7, 2017

Am I reading this correctly - I see return; statement inside that while loop so then what purpose does holding on to several definitions of same {METHOD, URL} definition is served.
If we are going to return this loop on the first match then why even bother to push/unshift - perhaps it should be a map than an array since there can be only one response.

    var i = -1, definition;
    while ((definition = definitions[++i])) {
      if (definition.match(method, url, data, headers || {})) {
        if (definition.response) {
          // if $browser specified, we do auto flush all requests
          ($browser ? $browser.defer : responsesPush)(wrapResponse(definition));
        } else if (definition.passThrough) {
          $delegate(method, url, data, callback, headers, timeout, withCredentials, responseType, eventHandlers, uploadEventHandlers);
        } else throw new Error('No response defined !');
        return;//why are we returning
      }
    }

@Narretz
Copy link
Contributor

Narretz commented Mar 7, 2017

I was also thinking of making this configurable. In any case, a PoC PR is very welcome.

@petebacondarwin
Copy link
Contributor

We are happy to merge a PR that implements a non-breaking change form of this feature. It looks like something along the lines of resetDefinitions would be best - or perhaps something like whenOverride?

@szimek
Copy link
Author

szimek commented Oct 2, 2017

Personally, I'd prefer whenOverride, because this would allow me to keep my generic mocks for most requests and override just the ones I need in a given test. I'll try to find some time to come up with a PR.

szimek added a commit to szimek/angular.js that referenced this issue Oct 2, 2017
This PR adds a new method to $httpBackend that allows to override any existing definition,
by adding the new definition at the beginning of the definition list, not at the end of it,
like $httpBackend.when does.

Closes angular#11637
szimek added a commit to szimek/angular.js that referenced this issue Oct 2, 2017
This PR adds a new method to $httpBackend that allows to override any existing definition,
by adding the new definition at the beginning of the definition list, not at the end of it,
like $httpBackend.when does.

Closes angular#11637
szimek added a commit to szimek/angular.js that referenced this issue Oct 3, 2017
This PR adds a new method to $httpBackend that allows to override any existing definition,
by adding the new definition at the beginning of the definition list, not at the end of it,
like $httpBackend.when does.

Closes angular#11637
@Narretz Narretz removed their assignment Oct 5, 2017
@Narretz Narretz modified the milestones: 1.7.0, 1.6.x Oct 30, 2017
@Narretz Narretz modified the milestones: 1.6.x, 1.7.x Apr 12, 2018
@Narretz Narretz self-assigned this May 3, 2018
Narretz added a commit to Narretz/angular.js that referenced this issue May 9, 2018
Narretz added a commit to Narretz/angular.js that referenced this issue May 9, 2018
@petebacondarwin petebacondarwin self-assigned this May 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.