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

3

36

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.

Sniperscope answered 21/6, 2014 at 12:19 Comment(7)
It's the new way to deal with async tests in Jasmine 2.0. It get's injected into the test-function and if you have not called it within 5 seconds the test fails. See jasmine.github.io/2.0/…Sniperscope
There was no separate tag for jasmine 2.0, or I would have tagged it. I can see how the syntax is confusing if you've not seen it before.Sniperscope
OK, I spent nearly all day yesterday on this, but I do think I figured it out. I will add it as an answer, and I may be wrong.Antlia
@Antlia that looks like some nice research, I'll see if I confirm what you found.Sniperscope
@Sniperscope please do, and post what you find.Antlia
@Antlia I'm seeing the same you're seeing. Good job digging into this.Sniperscope
Glad I found this but sad that everyone is probably spending a day on thisEec
A
74

$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

Antlia answered 8/10, 2014 at 8:9 Comment(12)
Yup, that seems to work and at least to me the explanation makes sense as well. Thanks!Sniperscope
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!Onwards
Months later, and still you made my day. I am really glad all that work digging helped others.Antlia
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!Inappropriate
@Inappropriate 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.Antlia
What happens if you want to introduce a delay in $httpBackend? Then the expects will never get called. What will be the solution then?Starflower
@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?Antlia
Worked for me as well.Portuna
your answer saved me from going (more) insane. Thanks.Lien
@ShaneRowatt I wish there were a "like" here. Your "more insane" was perfect!Antlia
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!Fibula
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.Antlia
J
12

@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.

Janejanean answered 23/10, 2015 at 19:27 Comment(2)
The setTimeout solution was what I was trying to avoid in the first place :)Sniperscope
You can put verifyNoOutstandingExpectation() in a timeout in your afterEach to make your 'it' cleaner instead.Janejanean
I
1

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();
});
Indoiranian answered 16/11, 2016 at 10:21 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.