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

ngMock.$httpBackend: statement defined in beforeEach block should be possible to have response redefined in it() block #5766

Closed
jrencz opened this issue Jan 13, 2014 · 9 comments

Comments

@jrencz
Copy link

jrencz commented Jan 13, 2014

I was trying to write a test of error branch of $http request.
I found it's not possible to redefine response for certain url after it's defined in beforeEach() block, which I find wrong.

Take a look at http://plnkr.co/edit/ygWP6nIV7L5PjkHrtmbG

It should be possible to define 2xx response in beforeEach() and then change its response code in one test if it's needed.

This problem doesn't seem to be new: I found half year old Stack Overflow thread describing how to solve the problem but declaring $httpBackend.when() on each it() block: http://stackoverflow.com/questions/17858955/how-to-override-mocked-response-of-httpbackend-in-angular but i'm convinced this is not the best, or most elegant way to do.

@IgorMinar
Copy link
Contributor

The relevant test code:

describe('Testing a Hello World controller', function() {
  var $scope = null;
  var ctrl = null;
  var $httpBackend;

  //you need to indicate your module in a test
  beforeEach(module('plunker'));

  beforeEach(inject(function($rootScope, $controller, _$httpBackend_) {
    $httpBackend = _$httpBackend_;
    $scope = $rootScope.$new();

    ctrl = $controller('MainCtrl', {
      $scope: $scope
    });

    $httpBackend.when('GET', 'things.json').respond({name: 'Earth'});
  }));

  it('should say hallo to the Earth', function() {
    expect($scope.name).toEqual('World');
    $httpBackend.flush();
    expect($scope.name).toEqual('Earth');
  });

  it('should say hallo to Mars', function() {
    $httpBackend.when('GET', 'things.json').respond(401);
    expect($scope.name).toEqual('World');
    $httpBackend.flush();
    expect($scope.name).not.toEqual('Earth');
    expect($scope.name).toEqual('Error - unauthorized');
  });
});

@IgorMinar
Copy link
Contributor

I agree that we could do better here if anyone wants to send a PR, we'll get it in.

@IgorMinar IgorMinar added this to the Backlog milestone Jul 24, 2014
@shahata
Copy link
Contributor

shahata commented Jul 25, 2014

I will take this.

@IgorMinar do you think this is solved simply by reversing the order in which definitions are traversed or do you have a better idea? Reversing traversal has the problem of breaking tests for a lot of ppl since ppl used to put the definitions with broader regexp last and now they will need to put them first.

For example:

$httpBackend.whenGET(/what\/ever$/).respond('shahar');
$httpBackend.whenGET(/\/ever$/).respond('talmi');

Will need to become:

$httpBackend.whenGET(/\/ever$/).respond('talmi');
$httpBackend.whenGET(/what\/ever$/).respond('shahar');

Some alternatives I see to resolve this:

  1. Let when override a previous when if it has the same definition. (problematic since url, data and headers might be functions)
  2. Expose overrideWhen or add some kind of priority argument so ppl can say that this definition needs to be evaluated first.
  3. Specifically let when inside it be evaluated before when inside beforeEach.

None of the options seems very good, this is a bit frustrating. :) WDYT?

@shahata
Copy link
Contributor

shahata commented Jul 25, 2014

Another option to solve this, which works without any code changes is something like:

describe('Testing a Hello World controller', function() {
  var $httpBackend, whenThings;

  beforeEach(inject(function($rootScope, $controller, _$httpBackend_) {
    $httpBackend = _$httpBackend_;
    var whenThings = $httpBackend.when('GET', 'things.json');
    whenThings.respond({name: 'Earth'});
  }));

  it('should say hallo to Mars', function() {
    whenThings.respond(401);
    $httpBackend.flush();
  });
});

I could change respond to return the chain so syntax will be a bit easier, but other than that I think this is a better method to accomplish this than anything I proposed in the previous comment.

@IgorMinar
Copy link
Contributor

@shahata reversing the order is definitely not the way to go.

I was thinking of just replacing the old definition (your option 1.) why is that problematic? as far as I can tell all the definition state is contained in the definitions variable which we iterate over on every request.

@shahata
Copy link
Contributor

shahata commented Jul 26, 2014

@IgorMinar the problem with replacing old definitions is that we might have something like:

$httpBackend.whenGET(function (url) { return url[15] === 'c'; }).respond('hi');

So, theoretically, we want the user to be able to override this by using the exact same arguments. But how can we do that? Does it need to be the exact same function reference? Also, from the user's perspective I think this is confusing.

It seems to me that the best option to accomplish this is to use the method I described in my second comment, which doesn't even require any code changes.

@AGoliath
Copy link

Why don´t we add some kind of "ruleID" to the return of $httpBackend.when*, and then have an $httpBackend.unset(ruleID)? I´m not too familar with the inners of httpBackend, but shouldn´t be too much of an hazzle

@IgorMinar
Copy link
Contributor

@shahata I see. I missed your second comment. I like it. Could you please
write a test and update docs with this info.

I don't think that returning the chain from respond method would make be
that beneficial unless I'm not understanding your intentions well.

On Sat, Jul 26, 2014, 6:30 AM André Goliath notifications@github.com
wrote:

Why don´t we add some kind of "ruleID" to the return of
$httpBackend.when*, and then have an $httpBackend.unset(ruleID)? I´m not
too familar with the inners of httpBackend, but shouldn´t be too much of an
hazzle


Reply to this email directly or view it on GitHub
#5766 (comment).

@shahata
Copy link
Contributor

shahata commented Jul 27, 2014

@IgorMinar There's already a PR (#8352) with the change I've proposed. If you think it is redundant, I can remove the returning of chain. (@petebacondarwin seems to like it :D)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants