Getting "$digest already in progress" in async test with Jasmine 2.0

Question

I know that calling $digest or $apply manually during a digest cycle will cause a "$digest already in progress" error but I have no idea why I am getting it here.

This is a unit test for a service that wraps $http, the service is simple enough, it just prevents making duplicate calls to the server while ensuring that code that attempts to do the calls still gets the data it expected.

angular.module('services')
    .factory('httpService', ['$http', function($http) {

        var pendingCalls = {};

        var createKey = function(url, data, method) {
            return method + url + JSON.stringify(data);
        };

        var send = function(url, data, method) {
            var key = createKey(url, data, method);
            if (pendingCalls[key]) {
                return pendingCalls[key];
            }
            var promise = $http({
                method: method,
                url: url,
                data: data
            });
            pendingCalls[key] = promise;
            promise.then(function() {
                delete pendingCalls[key];
            });
            return promise;
        };

        return {
            post: function(url, data) {
                return send(url, data, 'POST');
            },
            get: function(url, data) {
                return send(url, data, 'GET');
            },
            _delete: function(url, data) {
                return send(url, data, 'DELETE');
            }
        };
    }]);

The unit-test is also pretty straight forward, it uses $httpBackend to expect the request.

it('does GET requests', function(done) {
    $httpBackend.expectGET('/some/random/url').respond('The response');

    service.get('/some/random/url').then(function(result) {
        expect(result.data).toEqual('The response');
        done();
    });
    $httpBackend.flush();
});

This blows up as sone as done() gets called with a "$digest already in progress" error. I've no idea why. I can solve this by wrapping done() in a timeout like this

setTimeout(function() { done() }, 1);

That means done() will get queued up and run after the $digest is done but while that solves my problem I want to know

• Why is Angular in a digest-cycle in the first place?
• Why does calling done() trigger this error?

I had the exact same test running green with Jasmine 1.3, this only happened after I upgraded to Jasmine 2.0 and rewrote the test to use the new async-syntax.

Solution 1

$httpBacked.flush() actually starts and completes a $digest() cycle. I spent all day yesterday digging into the source of ngResource and angular-mocks to get to the bottom of this, and still don't fully understand it.

As far as I can tell, the purpose of $httpBackend.flush() is to avoid the async structure above entirely. In other words, the syntax of it('should do something',function(done){}); and $httpBackend.flush() do not play nicely together. The very purpose of .flush() is to push through the pending async callbacks and then return. It is like one big done wrapper around all of your async callbacks.

So if I understood correctly (and it works for me now) the correct method would be to remove the done() processor when using $httpBackend.flush():

it('does GET requests', function() {
    $httpBackend.expectGET('/some/random/url').respond('The response');

    service.get('/some/random/url').then(function(result) {
        expect(result.data).toEqual('The response');
    });
    $httpBackend.flush();
});

If you add console.log statements, you will find that all of the callbacks consistently happen during the flush() cycle:

it('does GET requests', function() {
    $httpBackend.expectGET('/some/random/url').respond('The response');

    console.log("pre-get");
    service.get('/some/random/url').then(function(result) {
        console.log("async callback begin");
        expect(result.data).toEqual('The response');
        console.log("async callback end");
    });
    console.log("pre-flush");
    $httpBackend.flush();
    console.log("post-flush");
});

Then the output will be:

pre-get

pre-flush

async callback begin

async callback end

post-flush

Every time. If you really want to see it, grab the scope and look at scope.$$phase

var scope;
beforeEach(function(){
    inject(function($rootScope){
        scope = $rootScope;
    });
});
it('does GET requests', function() {
    $httpBackend.expectGET('/some/random/url').respond('The response');

    console.log("pre-get "+scope.$$phase);
    service.get('/some/random/url').then(function(result) {
        console.log("async callback begin "+scope.$$phase);
        expect(result.data).toEqual('The response');
        console.log("async callback end "+scope.$$phase);
    });
    console.log("pre-flush "+scope.$$phase);
    $httpBackend.flush();
    console.log("post-flush "+scope.$$phase);
});

And you will see the output:

pre-get undefined

pre-flush undefined

async callback begin $digest

async callback end $digest

post-flush undefined

Solution 2

@deitch is right, that $httpBacked.flush() triggers a digest. The problem is that when $httpBackend.verifyNoOutstandingExpectation(); is run after each it is completed it also has a digest. So here's the sequence of events:

1. you call flush() which triggers a digest
2. the then() is executed
3. the done() is executed
4. verifyNoOutstandingExpectation() is run which triggers a digest, but you are already in one so you get an error.

done() is still important since we need to know that the 'expects' within the then() are even executed. If the then doesn't run then you might now know there were failures. The key is to make sure the digest is complete before firing the done().

it('does GET requests', function(done) {
    $httpBackend.expectGET('/some/random/url').respond('The response');

    service.get('/some/random/url').then(function(result) {
        expect(result.data).toEqual('The response');
        setTimeout(done, 0); // run the done() after the current $digest is complete.
    });
    $httpBackend.flush();
});

Putting done() in a timeout will make it executes immediately after the current digest is complete(). This will ensure that all of the expects that you wanted to run will actually run.

Solution 3

Adding to @deitch's answer. To make the tests more robust you can add a spy before your callback. This should guarantee that your callback actually gets called.

it('does GET requests', function() {
  var callback = jasmine.createSpy().and.callFake(function(result) {
    expect(result.data).toEqual('The response');
  });

  $httpBackend.expectGET('/some/random/url').respond('The response');
  service.get('/some/random/url').then(callback);
  $httpBackend.flush();

  expect(callback).toHaveBeenCalled();
});

Comments

[ivarni]

Yup, that seems to work and at least to me the explanation makes sense as well. Thanks!

[spikeheap]

If I could upvote this more than once I would. This saved me from digging through the source after banging my head on it all morning. Cheers!

[deitch]

Months later, and still you made my day. I am really glad all that work digging helped others.

[markrian]

Wow, this has been driving me crazy this afternoon. Once again, the angular docs let me down. Maybe the answer's in there, but it's not obvious. Thanks for a great answer!

[deitch]

@markrian excellent! Little bothers me more than 2 people doing the same work. I struggled through it, and now my efforts were helpful to you. That is a good day in my book.

[Adrian Ber]

What happens if you want to introduce a delay in $httpBackend? Then the expects will never get called. What will be the solution then?

[deitch]

@AdrianBer why? I think $httpBackend.flush() will still wait for everything to complete before returning. Unless I didn't understand how you want to introduce a delay?

[ivarni]

The setTimeout solution was what I was trying to avoid in the first place :)

[Matt Slocum]

You can put verifyNoOutstandingExpectation() in a timeout in your afterEach to make your 'it' cleaner instead.

[Claudio Mezzasalma]

Worked for me as well.

[Shane Rowatt]

your answer saved me from going (more) insane. Thanks.

[deitch]

@ShaneRowatt I wish there were a "like" here. Your "more insane" was perfect!

[Michael Trouw]

Jesus H. Christ. Wasn't testing one of Angular's so called strong points? Testing should be easy... not this kind of need-to-know-internals shite. Thanks for the elaborate answer and clear explaining!

[deitch]

You are welcome. I banged my head on this last year. I still do some Angular, but when I need client-side, I have been leaning towards Aurelia for components or React for reactive. Just easier.