Skip to content

Commit 7974e7e

Browse files
DiPengIgorMinar
DiPeng
authored and
IgorMinar
committed
refactor($browser): hide startPoll and poll methods
Breaks $browser.poll() method is moved inline to $browser.startpoll() Breaks $browser.startpoll() method is made private Refactor tests to reflect updated browser API Closes angular#387
1 parent f9b4c9d commit 7974e7e

File tree

5 files changed

+43
-59
lines changed

5 files changed

+43
-59
lines changed

‎src/Browser.js

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ var XHR = window.XMLHttpRequest || function () {
1010

1111

1212
/**
13-
* @private
14-
* @name Browser
13+
* @ngdoc service
14+
* @name angular.service.$browser
15+
* @requires $log
1516
*
1617
* @description
1718
* Constructor for the object exposed as $browser service.
@@ -21,6 +22,11 @@ var XHR = window.XMLHttpRequest || function () {
2122
* - hide all the global state in the browser caused by the window object
2223
* - abstract away all the browser specific features and inconsistencies
2324
*
25+
* For tests we provide {@link angular.mock.service.$browser mock implementation} of the `$browser`
26+
* service, which can be used for convenient testing of the application without the interaction with
27+
* the real browser apis.
28+
*/
29+
/**
2430
* @param {object} window The global window object.
2531
* @param {object} document jQuery wrapped document.
2632
* @param {object} body jQuery wrapped document.body.
@@ -121,6 +127,11 @@ function Browser(window, document, body, XHR, $log) {
121127
* @param {function()} callback Function that will be called when no outstanding request
122128
*/
123129
self.notifyWhenNoOutstandingRequests = function(callback) {
130+
// force browser to execute all pollFns - this is needed so that cookies and other pollers fire
131+
// at some deterministic time in respect to the test runner's actions. Leaving things up to the
132+
// regular poller would result in flaky tests.
133+
forEach(pollFns, function(pollFn){ pollFn(); });
134+
124135
if (outstandingRequestCount === 0) {
125136
callback();
126137
} else {
@@ -134,16 +145,6 @@ function Browser(window, document, body, XHR, $log) {
134145
var pollFns = [],
135146
pollTimeout;
136147

137-
/**
138-
* @workInProgress
139-
* @ngdoc method
140-
* @name angular.service.$browser#poll
141-
* @methodOf angular.service.$browser
142-
*/
143-
self.poll = function() {
144-
forEach(pollFns, function(pollFn){ pollFn(); });
145-
};
146-
147148
/**
148149
* @workInProgress
149150
* @ngdoc method
@@ -159,14 +160,12 @@ function Browser(window, document, body, XHR, $log) {
159160
* @returns {function()} the added function
160161
*/
161162
self.addPollFn = function(fn) {
162-
if (!pollTimeout) self.startPoller(100, setTimeout);
163+
if (!pollTimeout) startPoller(100, setTimeout);
163164
pollFns.push(fn);
164165
return fn;
165166
};
166167

167168
/**
168-
* @workInProgress
169-
* @ngdoc method
170169
* @name angular.service.$browser#startPoller
171170
* @methodOf angular.service.$browser
172171
*
@@ -177,9 +176,9 @@ function Browser(window, document, body, XHR, $log) {
177176
* Configures the poller to run in the specified intervals, using the specified
178177
* setTimeout fn and kicks it off.
179178
*/
180-
self.startPoller = function(interval, setTimeout) {
181-
(function check(){
182-
self.poll();
179+
function startPoller(interval, setTimeout) {
180+
(function check() {
181+
forEach(pollFns, function(pollFn){ pollFn(); });
183182
pollTimeout = setTimeout(check, interval);
184183
})();
185184
};

‎src/angular-mocks.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,13 @@ function MockBrowser() {
304304
}
305305
MockBrowser.prototype = {
306306

307+
/**
308+
* @name angular.mock.service.$browser#poll
309+
* @methodOf angular.mock.service.$browser
310+
*
311+
* @description
312+
* run all fns in pollFns
313+
*/
307314
poll: function poll(){
308315
angular.forEach(this.pollFns, function(pollFn){
309316
pollFn();

‎src/scenario/Application.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ angular.scenario.Application.prototype.executeAction = function(action) {
8989
return action.call(this, $window, _jQuery($window.document));
9090
}
9191
var $browser = $window.angular.service.$browser();
92-
$browser.poll();
9392
$browser.notifyWhenNoOutstandingRequests(function() {
9493
action.call(self, $window, _jQuery($window.document));
9594
});

‎test/BrowserSpecs.js

Lines changed: 19 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ describe('browser', function(){
88
}
99

1010
fakeSetTimeout.flush = function() {
11-
forEach(setTimeoutQueue, function(fn) {
11+
var currentTimeoutQueue = setTimeoutQueue;
12+
setTimeoutQueue = [];
13+
forEach(currentTimeoutQueue, function(fn) {
1214
fn();
1315
});
1416
};
@@ -364,54 +366,34 @@ describe('browser', function(){
364366
});
365367

366368
describe('poller', function(){
367-
beforeEach(function() {
368-
spyOn(browser, 'startPoller');
369-
});
370369

371-
it('should call all fns on poll', function(){
370+
it('should call functions in pollFns in regular intervals', function(){
372371
var log = '';
373372
browser.addPollFn(function(){log+='a';});
374373
browser.addPollFn(function(){log+='b';});
375374
expect(log).toEqual('');
376-
browser.poll();
375+
fakeSetTimeout.flush();
377376
expect(log).toEqual('ab');
378-
browser.poll();
377+
fakeSetTimeout.flush();
379378
expect(log).toEqual('abab');
380379
});
381380

382381
it('should startPoller', function(){
383-
var log = '';
384-
var setTimeoutSpy = jasmine.createSpy('setTimeout');
385-
browser.addPollFn(function(){log+='.';});
386-
browser.startPoller.andCallThrough();
387-
browser.startPoller(50, setTimeoutSpy);
388-
expect(log).toEqual('.');
389-
expect(setTimeoutSpy.mostRecentCall.args[1]).toEqual(50);
390-
setTimeoutSpy.mostRecentCall.args[0]();
391-
expect(log).toEqual('..');
382+
expect(setTimeoutQueue.length).toEqual(0);
383+
384+
browser.addPollFn(function(){});
385+
expect(setTimeoutQueue.length).toEqual(1);
386+
387+
//should remain 1 as it is the check fn
388+
browser.addPollFn(function(){});
389+
expect(setTimeoutQueue.length).toEqual(1);
392390
});
393391

394392
it('should return fn that was passed into addPollFn', function() {
395393
var fn = function() { return 1; };
396394
var returnedFn = browser.addPollFn(fn);
397395
expect(returnedFn).toBe(fn);
398396
});
399-
400-
it('should auto start poller when first fn registered', function() {
401-
browser.addPollFn(function() {});
402-
403-
expect(browser.startPoller).toHaveBeenCalled();
404-
expect(browser.startPoller.callCount).toBe(1);
405-
});
406-
407-
it('should auto start poller only when first fn registered', function() {
408-
browser.startPoller.andCallThrough();
409-
browser.addPollFn(function() {});
410-
browser.addPollFn(function() {});
411-
browser.addPollFn(function() {});
412-
413-
expect(browser.startPoller.callCount).toBe(1);
414-
});
415397
});
416398

417399

@@ -421,7 +403,8 @@ describe('browser', function(){
421403

422404
fakeWindow = {
423405
location: {href:"http://server"},
424-
document: {}
406+
document: {},
407+
setTimeout: fakeSetTimeout
425408
};
426409

427410
browser = new Browser(fakeWindow, {}, {});
@@ -435,12 +418,12 @@ describe('browser', function(){
435418

436419
fakeWindow.location.href = "http://server/#newHash";
437420
expect(events).toEqual([]);
438-
browser.poll();
421+
fakeSetTimeout.flush();
439422
expect(events).toEqual(['x']);
440423

441424
//don't do anything if url hasn't changed
442425
events = [];
443-
browser.poll();
426+
fakeSetTimeout.flush();
444427
expect(events).toEqual([]);
445428
});
446429

@@ -493,7 +476,7 @@ describe('browser', function(){
493476
browser.onHashChange(callback);
494477

495478
window.location.hash = 'new-hash';
496-
browser.startPoller(100, setTimeout);
479+
browser.addPollFn(function() {});
497480

498481
waitsFor(function() {
499482
return callback.callCount;

‎test/scenario/ApplicationSpec.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,6 @@ describe('angular.scenario.Application', function() {
121121
};
122122
testWindow.angular.service.$browser = function() {
123123
return {
124-
poll: function() {
125-
polled = true;
126-
},
127124
notifyWhenNoOutstandingRequests: function(fn) {
128125
handlers.push(fn);
129126
}
@@ -137,7 +134,6 @@ describe('angular.scenario.Application', function() {
137134
expect($document).toBeDefined();
138135
expect($document[0].className).toEqual('test-foo');
139136
});
140-
expect(polled).toBeTruthy();
141137
expect(handlers.length).toEqual(1);
142138
handlers[0]();
143139
});

0 commit comments

Comments
 (0)