fixup! fix(ngRoute): make route.resolve count as a pending request

Author: gkalpak

src/ngMock/angular-mocks.js

@@ -42,7 +42,8 @@ angular.mock.$Browser = function() {
   var outstandingRequestCount = 0;
   var outstandingRequestCallbacks = [];
   self.$$incOutstandingRequestCount = function() { outstandingRequestCount++; };
-  self.$$completeOutstandingRequest = function() {
+  self.$$completeOutstandingRequest = function(fn) {
+    fn();
     outstandingRequestCount--;
     if (!outstandingRequestCount) {
       while (outstandingRequestCallbacks.length) {

test/e2e/fixtures/ng-route-promise/script.js

@@ -4,22 +4,40 @@ angular.
   module('lettersApp', ['ngRoute']).
   config(function($routeProvider) {
     $routeProvider.
-      when('/foo', {
-        resolveRedirectTo: function($q) {
+      otherwise(resolveRedirectTo('/foo1')).
+      when('/foo1', resolveRedirectTo('/bar1')).
+      when('/bar1', resolveRedirectTo('/baz1')).
+      when('/baz1', resolveRedirectTo('/qux1')).
+      when('/qux1', {
+        template: '<ul><li ng-repeat="letter in $resolve.letters">{{ letter }}</li></ul>',
+        resolve: resolveLetters()
+      }).
+      when('/foo2', resolveRedirectTo('/bar2')).
+      when('/bar2', resolveRedirectTo('/baz2')).
+      when('/baz2', resolveRedirectTo('/qux2')).
+      when('/qux2', {
+        template: '{{ $resolve.letters.length }}',
+        resolve: resolveLetters()
+      });
+
+    // Helpers
+    function resolveLetters() {
+      return {
+        letters: function($q) {
           return $q(function(resolve) {
-            window.setTimeout(resolve, 1000, '/bar');
+            window.setTimeout(resolve, 1000, ['a', 'b', 'c', 'd', 'e']);
           });
         }
-      }).
-      when('/bar', {
-        template: '<ul><li ng-repeat="letter in $resolve.letters">{{ letter }}</li></ul>',
-        resolve: {
-          letters: function($q) {
-            return $q(function(resolve) {
-              window.setTimeout(resolve, 1000, ['a', 'b', 'c', 'd', 'e']);
-            });
-          }
+      };
+    }
+
+    function resolveRedirectTo(path) {
+      return {
+        resolveRedirectTo: function($q) {
+          return $q(function(resolve) {
+            window.setTimeout(resolve, 250, path);
+          });
         }
-      }).
-      otherwise('/foo');
+      };
+    }
   });

test/e2e/tests/ng-route-promise.spec.js

@@ -15,17 +15,19 @@ describe('ngRoute promises', function() {
     browser.waitForAngular().then(function() {
       fail('waitForAngular() should have timed out, but didn\'t');
     }, function(error) {
-      var errorRe = new RegExp(
-          'Timed out waiting for asynchronous Angular tasks to finish after ' +
-          '(?:1\\.5 seconds|15\\d\\d ?ms)\\.', '');
-      expect(error.message).toMatch(errorRe);
+      expect(error.message).toContain('Timed out waiting for asynchronous Angular tasks to finish');
     });
   });
 
+  it('should wait for route promises when navigating to another route', function() {
+    browser.setLocation('/foo2');
+    expect(element(by.tagName('body')).getText()).toBe('5');
+  });
+
   afterEach(function(done) {
     // Restore old timeout limit
     browser.getProcessedConfig().then(function(config) {
       return browser.manage().timeouts().setScriptTimeout(config.allScriptsTimeout);
-    }).then(function() { done(); });
+    }).then(done);
   });
 });

test/ngRoute/routeSpec.js

@@ -2088,12 +2088,15 @@ describe('$route', function() {
       var deferred;
 
       module(function($provide, $routeProvider) {
-        $routeProvider.when('/path', { template: '', resolve: {
-          a: function($q) {
-            deferred = $q.defer();
-            return deferred.promise;
+        $routeProvider.when('/path', {
+          template: '',
+          resolve: {
+            a: function($q) {
+              deferred = $q.defer();
+              return deferred.promise;
+            }
           }
-        } });
+        });
       });
 
       inject(function($location, $route, $rootScope, $httpBackend, $$testability) {
@@ -2114,12 +2117,15 @@ describe('$route', function() {
       var deferred;
 
       module(function($provide, $routeProvider) {
-        $routeProvider.when('/path', { template: '', resolve: {
-          a: function($q) {
-            deferred = $q.defer();
-            return deferred.promise;
+        $routeProvider.when('/path', {
+          template: '',
+          resolve: {
+            a: function($q) {
+              deferred = $q.defer();
+              return deferred.promise;
+            }
           }
-        } });
+        });
       });
 
       inject(function($location, $route, $rootScope, $httpBackend, $$testability) {
@@ -2140,7 +2146,8 @@ describe('$route', function() {
       var deferred;
 
       module(function($provide, $routeProvider) {
-        $routeProvider.when('/path', { template: '', resolveRedirectTo: function($q) {
+        $routeProvider.when('/path', {
+          resolveRedirectTo: function($q) {
             deferred = $q.defer();
             return deferred.promise;
           }
@@ -2165,7 +2172,8 @@ describe('$route', function() {
       var deferred;
 
       module(function($provide, $routeProvider) {
-        $routeProvider.when('/path', { template: '', resolveRedirectTo: function($q) {
+        $routeProvider.when('/path', {
+          resolveRedirectTo: function($q) {
             deferred = $q.defer();
             return deferred.promise;
           }
@@ -2187,45 +2195,76 @@ describe('$route', function() {
     });
 
     it('should wait for all route promises before calling callbacks', function() {
-      var redirectDeferred;
-      var localsDeferred;
+      var deferreds = {};
 
       module(function($provide, $routeProvider) {
-        $routeProvider.
-          when('/foo', { template: '', resolveRedirectTo: function($q) {
-            redirectDeferred = $q.defer();
-            return redirectDeferred.promise;
-          } }).
-          when('/bar', { template: '', resolve: {
+        // While normally `$browser.defer()` modifies the `outstandingRequestCount`, the mocked
+        // version (provided by `ngMock`) does not. This doesn't matter in most tests, but it does
+        // here:
+        // `$browser.defer()` will be indirectly called as a result of `$locationWatch`'s call to
+        // `$rootScope.$evalAsync()`. In the async function, the `$locationChangeSuccess` event will
+        // be broadcasted, which will in turn trigger `commitRoute()` in `ngRoute`.
+        // `outstandingRequestCount` must not reach 0 during the time between calling
+        // `$rootScope.$evalAsync()` and executing the async function.
+        $provide.decorator('$browser', function($delegate) {
+          var oldDefer = $delegate.defer;
+          var newDefer = function(fn, delay) {
+            var requestCountAwareFn = function() { $delegate.$$completeOutstandingRequest(fn); };
+            $delegate.$$incOutstandingRequestCount();
+            return oldDefer.call($delegate, requestCountAwareFn, delay);
+          };
+
+          $delegate.defer = angular.extend(newDefer, oldDefer);
+
+          return $delegate;
+        });
+
+        addRouteWithAsyncRedirect('/foo', '/bar');
+        addRouteWithAsyncRedirect('/bar', '/baz');
+        addRouteWithAsyncRedirect('/baz', '/qux');
+        $routeProvider.when('/qux', {
+          template: '',
+          resolve: {
             a: function($q) {
-              localsDeferred = $q.defer();
-              return localsDeferred.promise;
+              var deferred = deferreds['/qux'] = $q.defer();
+              return deferred.promise;
+            }
+          }
+        });
+
+        // Helpers
+        function addRouteWithAsyncRedirect(fromPath, toPath) {
+          $routeProvider.when(fromPath, {
+            resolveRedirectTo: function($q) {
+              var deferred = deferreds[fromPath] = $q.defer();
+              return deferred.promise.then(function() { return toPath; });
             }
-          } });
+          });
+        }
       });
 
-      inject(function($browser, $location, $route, $rootScope, $httpBackend, $$testability) {
+      inject(function($browser, $location, $rootScope, $route, $$testability) {
         $location.path('/foo');
         $rootScope.$digest();
 
         var callback = jasmine.createSpy('callback');
         $$testability.whenStable(callback);
         expect(callback).not.toHaveBeenCalled();
 
-        // ngRoute code is wrapped in a $browser.defer call (via $rootScope.evalAsync), which in
-        // production would automatically call $$incOutstandingRequestCount and
-        // $$completeOutstandingRequest before/after execution.  However, ngMock does not call these
-        // functions in its $browser.defer implementation, as this logic would make many tests
-        // difficult to write.  In this one case we need that logic, however, so we call the
-        // functions manually.
-        $browser.$$incOutstandingRequestCount();
-        redirectDeferred.resolve('/bar');
-        $rootScope.$digest();
-        $browser.$$completeOutstandingRequest();
+        deferreds['/foo'].resolve();
+        $browser.defer.flush();
         expect(callback).not.toHaveBeenCalled();
 
-        localsDeferred.resolve();
-        $rootScope.$digest();
+        deferreds['/bar'].resolve();
+        $browser.defer.flush();
+        expect(callback).not.toHaveBeenCalled();
+
+        deferreds['/baz'].resolve();
+        $browser.defer.flush();
+        expect(callback).not.toHaveBeenCalled();
+
+        deferreds['/qux'].resolve();
+        $browser.defer.flush();
         expect(callback).toHaveBeenCalled();
       });
     });