fix($http): include interceptors as a part of $browser's outstandingRequest #13862
Conversation
| $browser = browser; | ||
| }])); | ||
|
|
||
| it('should update $brower outstandingRequestCount', function() { |
This allows protractor to more reliably detect when all outstanding requests have been completed. Fixes angular#13782 Closes angular#13862
This allows protractor to more reliably detect when all outstanding requests have been completed. Fixes angular#13782 Closes angular#13862
|
@wbyoko, I was trying to reproduce the problem in an e2e test, but couldn't. Do you happen to have a minimal reproduction (even a flaky one) ? |
|
This fiddle I included in #13782 shows the issue. http://jsfiddle.net/awtwff3x/ You could also use an interceptor with a setTimeout which resolves a promise. We do have request interceptors in our project which may be why we would see this issue. do you add e2e tests in |
|
@wbyoko, thx (it seems like I missed that). Regarding your question: One way to add e2e tests is to add a |
|
@wbyoko, this test access |
|
@gkalpak I added an e2e test. Uses the setTimeout method in an interceptor and doesn't pass without included updates to http. I can clean this up and squash / recreate this commit. Would you think this e2e test is needed? |
|
That test shouldn't pass on the master branch right now. |
|
I'd like to have an actual e2e test that showcases the issue. Let me take a look and get back to you :) |
|
@gkalpak Thanks. I ran this test on the master branch to make sure that it didn't pass. And this showcases the fix by including interceptors as a part of the outstanding request which normally protractor wasn't waiting for. I wouldn't understand how the e2e test is accessing Thanks again. |
|
Protractor uses |
|
But with the e2e test it's going through protractor to do it. So the fiddle calls it directly, the e2e test doesn't. It seems actual issue is that $browser's outstandingRequest should be including interceptors because otherwise using them has the potentially to make e2e tests inconsistent. Thanks. |
|
@gkalpak any updates on what you found? Thanks. |
|
@wbyoko, I didn't get a chance to properly look into it yet. I'll get back to this right after |
|
@gkalpak sounds good. Let me know when you have looked at this. Thanks. |
The issue is that using $http doesn't update $browser.outstandingRequestCount synchronously so that $browser.notifyWhenNoOutstandingRequests waits accordingly. Configuring this synchronization with the promise chain updates the outstandingRequestCount correctly. Closes angular#13782
…Count` Calling `$http` will not increment `$browser.outstandingRequestCount` until after all (potentially) asynchronous request interceptors have been processed and will decrement it before any (potentially) asynchronous response interceptors have been processed. This can lead to `$browser.notifyWhenNoOutstandingRequests` firing prematurely, which can be problematic in end-to-end tests. This commit fixes this, by synchronizing the increment/decrement operations with `$http`'s internal interceptor promise chain. Fixes angular#13782 Closes angular#13862
…Count` Calling `$http` will not increment `$browser.outstandingRequestCount` until after all (potentially) asynchronous request interceptors have been processed and will decrement it before any (potentially) asynchronous response interceptors have been processed. This can lead to `$browser.notifyWhenNoOutstandingRequests` firing prematurely, which can be problematic in end-to-end tests. This commit fixes this, by synchronizing the increment/decrement operations with `$http`'s internal interceptor promise chain. Fixes angular#13782 Closes angular#13862
The issue is that using $http doesn't update $browser.outstandingRequestCount synchronously so that $browser.notifyWhenNoOutstandingRequests waits accordingly.
Configuring this synchronization with the promise chain updates the outstandingRequestCount correctly.
Fixes #13782