Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat($httpBackend): flush requests in desired order #14967

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions src/ngMock/angular-mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -1788,24 +1788,29 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
* @ngdoc method
* @name $httpBackend#flush
* @description
* Flushes all pending requests using the trained responses.
* Flushes pending requests using the trained responses in the order they arrived.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would mention the start/skip parameter in the description.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll do.
(I am not a native speaker so I will be thankful to get any idea about the text.)

* If there are no pending requests when the flush method is called
* an exception is thrown (as this typically a sign of programming error).
*
* @param {number=} count Number of responses to flush (in the order they arrived). If undefined,
* all pending requests will be flushed. If there are no pending requests when the flush method
* is called an exception is thrown (as this typically a sign of programming error).
* @param {number=} count Number of responses to flush. If undefined,
* all pending requests will be flushed beginning at `start`.
* @param {number=} [start=0] Sequential request number at which to begin to flush.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think skip is a better name for this param. It is more intuitive imo.

(Also, have you checked how the resulting docs look like? Can dgeni really understand the [<name>=<default value>] syntax?)

Copy link
Contributor Author

@dizel3d dizel3d Jul 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about skip name but it looks like "remove all items before this and go on", doesn't it?
Yes, default value looks good in doc. Also this syntax is already in use in other parts of the project.

*/
$httpBackend.flush = function(count, digest) {
$httpBackend.flush = function(count, start, digest) {
if (digest !== false) $rootScope.$digest();
if (!responses.length) throw new Error('No pending request to flush !');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to account for start here as well.


start = angular.isDefined(start) && start !== null ? start : 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: start = start || 0 should work just as well.


if (angular.isDefined(count) && count !== null) {
while (count--) {
if (!responses.length) throw new Error('No more pending request to flush !');
responses.shift()();
var part = responses.splice(start, 1);
if (!part.length) throw new Error('No more pending request to flush !');
part[0]();
}
} else {
while (responses.length) {
responses.shift()();
while (responses.length > start) {
responses.splice(start, 1)[0]();
}
}
$httpBackend.verifyNoOutstandingExpectation(digest);
Expand Down
6 changes: 3 additions & 3 deletions test/ng/httpSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2210,7 +2210,7 @@ describe('$http with $applyAsync', function() {
// Ensure requests are sent
$rootScope.$digest();

$httpBackend.flush(null, false);
$httpBackend.flush(null, null, false);
expect($rootScope.$applyAsync).toHaveBeenCalledOnce();
expect(handler).not.toHaveBeenCalled();

Expand All @@ -2228,7 +2228,7 @@ describe('$http with $applyAsync', function() {
// Ensure requests are sent
$rootScope.$digest();

$httpBackend.flush(null, false);
$httpBackend.flush(null, null, false);
expect(log).toEqual([]);

$browser.defer.flush();
Expand All @@ -2253,7 +2253,7 @@ describe('$http with $applyAsync', function() {
expect(log).toEqual(['response 1', 'response 2']);

// Finally, third response is received, and a second coalesced $apply is started
$httpBackend.flush(null, false);
$httpBackend.flush(null, null, false);
$browser.defer.flush();
expect(log).toEqual(['response 1', 'response 2', 'response 3']);
});
Expand Down
32 changes: 32 additions & 0 deletions test/ngMock/angular-mocksSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1532,6 +1532,38 @@ describe('ngMock', function() {
});


it('should flush given number of pending requests beginning at specified request', function() {
var dontCallMe = jasmine.createSpy('dontCallMe');

hb.when('GET').respond(200, '');
hb('GET', '/some', null, dontCallMe);
hb('GET', '/some', null, callback);
hb('GET', '/some', null, callback);
hb('GET', '/some', null, dontCallMe);

hb.flush(2, 1);
expect(dontCallMe).not.toHaveBeenCalled();
expect(callback).toHaveBeenCalled();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is redundant.

Copy link
Contributor Author

@dizel3d dizel3d Jul 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#Yes it is.
(I just noticed toHaveBeenCalled together with toHaveBeenCalledTimes in test above. Line 1530.)

expect(callback).toHaveBeenCalledTimes(2);
});


it('should flush all pending requests beginning at specified request', function() {
var dontCallMe = jasmine.createSpy('dontCallMe');

hb.when('GET').respond(200, '');
hb('GET', '/some', null, dontCallMe);
hb('GET', '/some', null, dontCallMe);
hb('GET', '/some', null, callback);
hb('GET', '/some', null, callback);

hb.flush(null, 2);
expect(dontCallMe).not.toHaveBeenCalled();
expect(callback).toHaveBeenCalled();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right.

expect(callback).toHaveBeenCalledTimes(2);
});


it('should throw exception when flushing more requests than pending', function() {
hb.when('GET').respond(200, '');
hb('GET', '/url', null, callback);
Expand Down