angular / protractor

E2E test framework for Angular apps
http://www.protractortest.org
MIT License
8.75k stars 2.31k forks source link

Protractor does not wait to resolve promises on routes #789

Closed Zaylril closed 7 years ago

Zaylril commented 10 years ago

Hey,

I'm having a few issues getting protractor to wait for my dependencies to load in the route.

The basic code I have is this (which works fine in Angular itself)

$routeProvider.when('/login, {
    templateUrl: 'template.html',
    controller: 'LoginCtrl',
    resolve: {
         dependencies: function($q, $rootScope) {
          var def = $q.defer();
          yepnope([
            {
                load: [
                    'dependencies.js',
                ],
                complete: function () {
                    $rootScope.$apply(function () {
                        def.resolve();
                    });
                }
            }
        ]);
        return def.promise;
        }
     }
 });

Protractor doesn't wait for the route to resolve and doesn't even display any content from the route. If the resolve dependency is removed the route successfully loads (but the test fails as the dependency isn't loaded).

I've tried it with $timeout testing and that works fine, and as this isn't a $http request, is there something I'm missing, or is this just not supported?

Thanks

juliemr commented 10 years ago

Hm, interesting. This works if you using $timeout inside resolve, as is used in this part of the testapp. I believe your example fails because you are making a custom promise. This seems like a reasonable use case - it will probably require some support from the Angular side, we'll investigate.

morgs32 commented 10 years ago

I'm not one to +1 things, but this issue looked lonely.

edit: not an issue for me. had everything to do with fetching facebook login status in the promise. keep it up @juliemr and co

yxh10 commented 10 years ago

Hi Julie @juliemr ,

I think I am having similar issue when using ui-router. According the doc https://github.com/angular/protractor/blob/master/docs/timeouts.md

and your comment, protractor waits for all $http and $timeout to finish, so I wrapped my resolve with $timeout.

Please see my code below to see what I mean (in fact, pretty much the same as the link you provided)

 $stateProvider
      .state('my.state',{
        url: '',
        templateUrl: 'my-state-template.html',
        controller: 'MyStateController',
        resolve: {
          contracts: ['myModelService', '$timeout', function(model, $timeout){
            return $timeout(function () {
              model.list();
            });
          }]
        }
      });

My test is working fine now, but I wonder if this is good practice? I am asking this because $timeout calls $apply and that fires digest cycle?

yxh10 commented 10 years ago

I have just updated to 1.2.0 today. And everything is magically working fine now without $timeout. Thank you to all the protractor contributors. @juliemr

Updates: OK. Sadly it was just my machine running a lot quicker after loading everything in memory. After I left this for some time and come back again, the same issue remains.

Thanks to @appsciences posting the stackoverflow link. To save a bit of time for other people, some one posted a word around on that page. Basically instead of relying on protractor to wait for the state/page to be resolved, wait for an element you expect to see.

browser.wait(function() {
    return $('#test321').isPresent(); // keeps waiting until this statement resolves to true
});

expect($('#test321').isPresent()).toBe(true);  
appsciences commented 9 years ago

Hello,

I'm on 1.4, and I am having the same issue as described above. More detail on stackoverflow.

Smolations commented 9 years ago

I am experiencing a similar issue. I'm using Angular 1.3.1, Protractor 1.4, and angular-ui-router 0.2.11. The app I am working with uses ocLazyLoad to resolve dependencies for various states. Many child states utilize a resolve function to load data from a service before changing state. I am one of the few developers using a Vagrant VM to do my development, so I run the protractor tests from my Mac host against the server in the VM. My colleagues all run the tests and server from their Macs, so they haven't noticed this issue because, I guess, the processing is fast enough that the tests seem to pass consistently for them?

I receive a NoSuchElementError fairly consistently in one of our tests. The relevant snippets are below for your consideration:

// right after the describe statement, these lines run with no problem
helpers.chrome.openUserMenu();
element(by.cssContainingText('#mainUserDropMenu a', 'Settings')).click(); // this triggers a state change
browser.waitForAngular();

// here are the original lines that fail for me in Vagrant, but pass for my colleagues.
// the NoSuchElementError is thrown on the second line.
var $registerLink = element(by.css('a[ui-sref="some.stateLink"]'));
expect($registerLink.getText()).toEqual('Register Now');

// after much fiddling and googling, i discovered that adding a simple
// line to check for presence (though I'm not doing anything with it)
// makes the test pass consistently:
browser.isElementPresent(by.css('a[ui-sref="some.stateLink"]')); // promise also happens to resolve successfully
var $registerLink = element(by.css('a[ui-sref="some.stateLink"]'));
expect($registerLink.getText()).toEqual('Register Now'); // works!

// i then found this issue and the relevant stack overflow post and the solution
// given in both also works:
var elem = by.css('a[ui-sref="some.stateLink"]');
browser.driver.wait(function() {
    return browser.isElementPresent(elem);
}, 5000);
var $registerLink = element(elem);
expect($registerLink.getText()).toEqual('Register Now'); // works!

The template loaded by the route that contains the element in question is loaded into a parent 'layout' which is loaded into a <ui-view> from the main html page served up by the server. The anchor element's text is static (not an expression) and its containing element contains no directives which determine how/if the element should be rendered.

I am not sure if this is a protractor issue or a UI Router/ocLazyLoad issue, but I wanted to follow up in this thread due to its relevancy to my problem. Perhaps the talented, protractor expert @juliemr could give an update as to this investigation? =]

paulcwarren commented 9 years ago

I take back what I said yesterday...

I thought I was having exactly this issue using protractor 1.6.1 on a fairly large project that I am retro-fitting e2e tests to.

HOWEVER, after putting together a quick, small angular app to test with and I can't reproduce the behavior described here and seen in my large production project. That said I am not sure how accurate my test app is since it uses $timeout (inside a angular service) to fake a $http call that takes "some time". I think I'll convert that to a long-running service on my server. IF I can repro it I will commit my test app, in case that helps protractor folks. Stay tuned.

UPDATE 02/07

In case it is useful to others.

Turns out my problem was actually a ui-bootstrap 0.11 carousel problem whose implementation uses $timeout, rather than $interval. Initially I was getting timeouts waiting for sync and this had forced me down the road of using ignoreSynchronization=true (although I was ignorant of exactly why at the time, just trying stuff out really). Ignoring sync then caused all sorts of knock on effects, such as not waiting for resolves ultimately getting me here. Once I ruled out this problem out with my test app (becuse it didn't repro this resolve issue) I went all the way back to basics, removing ignoreSynchronization=true getting the timeout issues and this time properly investigating it eventually narrowing it down to the carousel.

The carousel FR is documented here:- https://github.com/angular-ui/bootstrap/pull/1630

My conclusion, don't use ignoreSynchronization=true and the wait for resolve bug is fixed (protractor 1.6.1).

facultymatt commented 9 years ago

Im getting the same issue where resolves cause tests to fail, using ui-router v0.2.13.

yxh10 commented 9 years ago

Guys please see the code below if your issue is timing related.You just need to ask Protractor to wait for the element to appear. Read the line with comment.

browser.wait(function() {
       return $('#test321').isPresent(); // keeps waiting until this statement resolves to true
      });

($('#test321').isPresent()).toBe(true); 
Smolations commented 9 years ago

@yxh10 That is not a long-term solution. Protractor should be aware of the views it's rendering, as well as when the DOM is ready. Using browser.wait in that fashion may solve the immediate problem, but if it happens across many tests, the spec files would be littered with all of those statements. Not only that, but how would one choose an appropriate timeToWaitInMilliseconds? Having multiple blocks such as the one you have above would significantly slow down testing for larger applications.

yxh10 commented 9 years ago

@Smolations Hi, Chris, I agree with you. It is a work around. You could use this before protractor solve the problem from the core.

Regarding to choose approriate timeToWaitInMilliseconds, I am not sure if you are talking about the time you set in protractor config, or the time you set with browser.wait. If you are talking about the later, you actually don't need to worry about it. If you read the line that come with comment, and the api doc, you will realize it is returning a promise. The rest of the code will only run when the dom is presented (ready). It is exactly doing that. No time needed to be set. And if you want to wait till dom is actually displayed, you can use isDisplayed() instead of isPresent.

So the only downside of this solution for now is too wordy for now. (You can always write your own util function to wrap it which is what I am doing in my current project.) You don't really need to worry about setting the length of time you wait. It is all handled by brower.wait waiting for the promise returned inside.

moneytree-doug commented 9 years ago

I am having this issue as well. As soon as I comment out my promises in the resolve of the ui-router, everything passes again.

paulfreeman commented 9 years ago

Also hitting problems with this today. Work around makes testing very slow, but at least it works. Note, I believe that the timeToWaitInMilliseconds must be long enough to allow the actual request to return, browser.wait will then wait up to that length of time, but will return sooner if the request does.

moneytree-doug commented 9 years ago

I ended up using $httpBackend to stub out those requests in the resolve, but setting up $httpBackend is another story.

yxh10 commented 9 years ago

@moneytree-doug , you could try the work around I posted above. But if your requirement is fine with mocked service, that is also cool. :+1:

moneytree-doug commented 9 years ago

@yxh10 I tried that, and it didn't work well. On top of that, I didn't want to throw an arbitrary number of seconds for waiting and prolong the test.

yxh10 commented 9 years ago

@moneytree-doug Hm... what didn't work well? Can you specify? What browser are you using for the test?

Protractor is developed to do End 2 End test. I think using mock sometimes is OK, but I think that defeat the original purpose. If you run against mock, just using Karma might be better?

paulfreeman commented 9 years ago

yxh10, I agree, with mocking you are doing another kind of testing, valid in its own way as an acceptance test of UX flow, but it isn't an end to end test.

Smolations commented 9 years ago

Not to get off topic here, but I think you guys (@yxh10 and @paulfreeman) have misunderstood what @moneytree-doug was trying to say. The way I read it, he had his original promises in place but was running into the alleged bug, which is the topic of this thread. He commented out his original code to verify that those promises were the issue, and was able to simulate their responses using $httpBackend in order to circumvent it. To me, he is most likely using protractor for proper e2e tests, but the problem is that this apparent bug prevents normal e2e tests from passing, regardless of whether his code is solid or not. I'm sure he tests his features manually during development to make sure the requests are properly formed, so mocked responses don't seem inappropriate to get around a potential bug. It seems like the available options are:

  1. Wrap element references in tests with $timeouts, using an arbitrary number of milliseconds, thus delaying tests and incurring tech debt once this apparent bug is fixed (hopefully!)
  2. Use mocked responses in the troubled route resolvers to ensure his tests will pass when valid data comes back from the server

E2E tests are meant to test organic user flows throughout an app. They are not necessarily used to test that a server returns valid responses, which is probably why he mocked the responses. Model/Unit tests can be used server-side to test the backend to ensure it behaves as it should.

Man, wish someone from the protractor team would jump in on this thread. Surely it's the most commented-on, open issue at this point! :smile:

paulfreeman commented 9 years ago

@Smolations I see your point. I recognise that is the standard methodology. I still think that its sometimes useful to have a fully integrated e2e test and given the possible misnomer created by the term 'end to end' and the expectations it creates it would be a 'nice to have' idea to have this work with moderately realistic response times (say up to 10 seconds which I get from my iOS functional testing suite for example). Alternatively the documentation could point out the current behavior and the suggested methodology so that it helps a beginner. For me I like to use a unit testing framework to test my backend API which I call an integration test, but I still see some value in aping the sequencing/timing of a realistic user scenario against a backend using the e2e test that can be run repetitively to simulate a realistic usage.

moneytree-doug commented 9 years ago

@Smolations Precisely!

Smolations commented 9 years ago

Alternatively the documentation could point out the current behavior and the suggested methodology so that it helps a beginner.

Right?! Google releases best practices on almost everything else on the web, but it seems lacking for protractor. I try to keep a cool head though. The Angular/Protractor team is busy enough with 2.0 I'm sure, haha.

moneytree-doug commented 9 years ago

Out of curiosity here, what does the label "need investigation" mean? Does it need a set of steps to reproduce the issue? Or are we passed that and we are investigating where the bug is in protractor?

krico commented 9 years ago

I am working on an app that uses google javascript api (gapi) as the means to communicate with the backend (google cloud endpoints). Unfortunately gapi works with 3rd party promises (neither $http or $timeout) and the proper way to handle this is to wrap those with $q.when() (please correct me if I'm wrong). As a result the specs need to be written based on workarounds and are not reliable at all. Here's an example of an imaginary blog service method to illustrate:

function listBlogEntries() {
  return $q.when(gapi.client.myBlogApp.listBlogEntries());
}

If you call this method as a route resolve, or even at the click of a button, protractor doesn't wait for it to "finish". This means that the tests need to either "sleep" and hope for the best, or you end up having to add elements to your app. So from my side, I give this issue a big :+1: and also volunteer to help ;-)

sjelin commented 8 years ago

@hankduan something to keep in mind for the testability API in Angular2?

sjelin commented 8 years ago

Minimal example:

<html ng-app="countryApp">
  <head>
    <meta charset="utf-8">
    <title>Angular.js Example</title>
    <script src="angular.js"></script>
    <script src="angular-route.js"></script>
    <script>
      var countryApp = angular.module('countryApp', ['ngRoute']);

      countryApp.config(function($routeProvider) {
        $routeProvider.
          when('/', {
            template: '<ul><li ng-repeat="country in countries">{{country.name}}</li><ul>',
            controller: 'CountryListCtrl',
            resolve: {
              countries: function($q) {
                var deferred = $q.defer();
                setTimeout(function() {
                  deferred.resolve([{
                    "name": "China",
                    "population": 1359821000,
                    "flagURL": "//upload.wikimedia.org/wikipedia/commons/f/fa/Flag_of_the_People%27s_Republic_of_China.svg",
                    "capital": "Beijing",
                    "gdp": 12261
                  }, {
                    "name": "India",
                    "population": 1205625000,
                    "flagURL": "//upload.wikimedia.org/wikipedia/en/4/41/Flag_of_India.svg",
                    "capital": "New Delhi",
                    "gdp": 4716
                  }, {
                    "name": "United States of America",
                    "population": 312247000,
                    "flagURL": "//upload.wikimedia.org/wikipedia/en/a/a4/Flag_of_the_United_States.svg",
                    "capital": "Washington, D.C.",
                    "gdp": 16244
                  }]);
                }, 3000);
                return deferred.promise;
              }
            }
          }).
          when('/:countryName', {
            template: '<h1>TODO create country detail view</h1>',
            controller: 'CountryDetailCtrl'
          }).
          otherwise({
            redirectTo: '/'
          });
      });

      countryApp.controller('CountryListCtrl', function ($scope, $http){
        $http.get('countries.json').success(function(data) {
          $scope.countries = data;
        });
      });

      countryApp.controller('CountryDetailCtrl', function ($scope, $routeParams){
        console.log($routeParams);
      });
    </script>
  </head>
  <body>
    <div ng-view></div>
  </body>
</html>
describe('', function() {
  it('', function() {
    browser.get('index.html');
    expect(element.all(by.tagName('li')).count()).toBe(3);
  });
});

Will have PR to angular team up in a few minutes

fgather commented 8 years ago

I see the same problem with Angular2 beta16 when using routes which load some additional data from the back end. Did someone already investigate this or am I the only one seeing this problem?

juliemr commented 8 years ago

@sjelin is there a link to a PR for angular?

gkalpak commented 8 years ago

@juliemr , I think @sjelin meant angular/angular.js#14159.

gkalpak commented 7 years ago

I think this can be closed now that angular/angular.js#14159 has been merged, but leaving it up to you :smiley: (The fix will be included in v1.6.2 btw.)

cc @sjelin, @juliemr