Skip to content

Commit 0727260

Browse files
petebacondarwinIgorMinar
petebacondarwin
authored and
IgorMinar
committed
fix(modules): stop leaking global variables in tests
The routeUtils.js file was declaring a number of functions that were leaking into other modules such as ngMocks causing tests to pass incorrectly. Closes angular#4360
1 parent b019a48 commit 0727260

8 files changed

Lines changed: 55 additions & 68 deletions

File tree

‎src/ngMock/angular-mocks.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,7 @@ angular.mock.animate = angular.module('mock.animate', ['ng'])
769769
}
770770
};
771771

772-
forEach(['enter','leave','move','addClass','removeClass'], function(method) {
772+
angular.forEach(['enter','leave','move','addClass','removeClass'], function(method) {
773773
animate[method] = function() {
774774
var params = arguments;
775775
animate.queue.push({

‎src/ngRoute/route.js‎

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ var ngRouteModule = angular.module('ngRoute', ['ng']).
2828
* Requires the {@link ngRoute `ngRoute`} module to be installed.
2929
*/
3030
function $RouteProvider(){
31+
function inherit(parent, extra) {
32+
return angular.extend(new (angular.extend(function() {}, {prototype:parent}))(), extra);
33+
}
34+
3135
var routes = {};
3236

3337
/**
@@ -126,7 +130,7 @@ function $RouteProvider(){
126130
* Adds a new route definition to the `$route` service.
127131
*/
128132
this.when = function(path, route) {
129-
routes[path] = extend(
133+
routes[path] = angular.extend(
130134
{reloadOnSearch: true},
131135
route,
132136
path && pathRegExp(path, route)
@@ -138,7 +142,7 @@ function $RouteProvider(){
138142
? path.substr(0, path.length-1)
139143
: path +'/';
140144

141-
routes[redirectPath] = extend(
145+
routes[redirectPath] = angular.extend(
142146
{redirectTo: path},
143147
pathRegExp(redirectPath, route)
144148
);
@@ -460,17 +464,17 @@ function $RouteProvider(){
460464
last = $route.current;
461465

462466
if (next && last && next.$$route === last.$$route
463-
&& equals(next.pathParams, last.pathParams) && !next.reloadOnSearch && !forceReload) {
467+
&& angular.equals(next.pathParams, last.pathParams) && !next.reloadOnSearch && !forceReload) {
464468
last.params = next.params;
465-
copy(last.params, $routeParams);
469+
angular.copy(last.params, $routeParams);
466470
$rootScope.$broadcast('$routeUpdate', last);
467471
} else if (next || last) {
468472
forceReload = false;
469473
$rootScope.$broadcast('$routeChangeStart', next, last);
470474
$route.current = next;
471475
if (next) {
472476
if (next.redirectTo) {
473-
if (isString(next.redirectTo)) {
477+
if (angular.isString(next.redirectTo)) {
474478
$location.path(interpolate(next.redirectTo, next.params)).search(next.params)
475479
.replace();
476480
} else {
@@ -483,29 +487,29 @@ function $RouteProvider(){
483487
$q.when(next).
484488
then(function() {
485489
if (next) {
486-
var locals = extend({}, next.resolve),
490+
var locals = angular.extend({}, next.resolve),
487491
template, templateUrl;
488492

489-
forEach(locals, function(value, key) {
490-
locals[key] = isString(value) ? $injector.get(value) : $injector.invoke(value);
493+
angular.forEach(locals, function(value, key) {
494+
locals[key] = angular.isString(value) ? $injector.get(value) : $injector.invoke(value);
491495
});
492496

493-
if (isDefined(template = next.template)) {
494-
if (isFunction(template)) {
497+
if (angular.isDefined(template = next.template)) {
498+
if (angular.isFunction(template)) {
495499
template = template(next.params);
496500
}
497-
} else if (isDefined(templateUrl = next.templateUrl)) {
498-
if (isFunction(templateUrl)) {
501+
} else if (angular.isDefined(templateUrl = next.templateUrl)) {
502+
if (angular.isFunction(templateUrl)) {
499503
templateUrl = templateUrl(next.params);
500504
}
501505
templateUrl = $sce.getTrustedResourceUrl(templateUrl);
502-
if (isDefined(templateUrl)) {
506+
if (angular.isDefined(templateUrl)) {
503507
next.loadedTemplateUrl = templateUrl;
504508
template = $http.get(templateUrl, {cache: $templateCache}).
505509
then(function(response) { return response.data; });
506510
}
507511
}
508-
if (isDefined(template)) {
512+
if (angular.isDefined(template)) {
509513
locals['$template'] = template;
510514
}
511515
return $q.all(locals);
@@ -516,7 +520,7 @@ function $RouteProvider(){
516520
if (next == $route.current) {
517521
if (next) {
518522
next.locals = locals;
519-
copy(next.params, $routeParams);
523+
angular.copy(next.params, $routeParams);
520524
}
521525
$rootScope.$broadcast('$routeChangeSuccess', next, last);
522526
}
@@ -535,10 +539,10 @@ function $RouteProvider(){
535539
function parseRoute() {
536540
// Match a route
537541
var params, match;
538-
forEach(routes, function(route, path) {
542+
angular.forEach(routes, function(route, path) {
539543
if (!match && (params = switchRouteMatcher($location.path(), route))) {
540544
match = inherit(route, {
541-
params: extend({}, $location.search(), params),
545+
params: angular.extend({}, $location.search(), params),
542546
pathParams: params});
543547
match.$$route = route;
544548
}
@@ -552,7 +556,7 @@ function $RouteProvider(){
552556
*/
553557
function interpolate(string, params) {
554558
var result = [];
555-
forEach((string||'').split(':'), function(segment, i) {
559+
angular.forEach((string||'').split(':'), function(segment, i) {
556560
if (i === 0) {
557561
result.push(segment);
558562
} else {
@@ -566,4 +570,4 @@ function $RouteProvider(){
566570
return result.join('');
567571
}
568572
}];
569-
}
573+
}

‎src/ngRoute/routeUtils.js‎

Lines changed: 0 additions & 17 deletions
This file was deleted.

‎test/helpers/matchers.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,11 @@ beforeEach(function() {
7474
this.message = function() {
7575
var expected;
7676
if (this.actual.message && this.actual.name == 'Error') {
77-
expected = toJson(this.actual.message);
77+
expected = angular.toJson(this.actual.message);
7878
} else {
79-
expected = toJson(this.actual);
79+
expected = angular.toJson(this.actual);
8080
}
81-
return "Expected " + expected + " to be an Error with message " + toJson(message);
81+
return "Expected " + expected + " to be an Error with message " + angular.toJson(message);
8282
};
8383
return this.actual.name == 'Error' && this.actual.message == message;
8484
},
@@ -187,9 +187,9 @@ beforeEach(function() {
187187
codeRegex = new RegExp('^\\[' + escapeRegexp(namespace) + ':' + escapeRegexp(code) + '\\]'),
188188
not = this.isNot ? "not " : "",
189189
regex = jasmine.isA_("RegExp", content) ? content :
190-
isDefined(content) ? new RegExp(escapeRegexp(content)) : undefined;
190+
angular.isDefined(content) ? new RegExp(escapeRegexp(content)) : undefined;
191191

192-
if(!isFunction(this.actual)) {
192+
if(!angular.isFunction(this.actual)) {
193193
throw new Error('Actual is not a function');
194194
}
195195

@@ -215,7 +215,7 @@ beforeEach(function() {
215215
return result;
216216
}
217217

218-
if (isDefined(regex)) {
218+
if (angular.isDefined(regex)) {
219219
return regex.test(exceptionMessage);
220220
}
221221
return result;

‎test/helpers/privateMocksSpec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ describe('private mocks', function() {
99
var stylesheet = createMockStyleSheet($document, $window);
1010
expect(doc.styleSheets.length).toBe(count + 1);
1111

12-
jqLite(doc.body).append($rootElement);
12+
angular.element(doc.body).append($rootElement);
1313

1414
var elm = $compile('<div class="padded">...</div>')($rootScope);
1515
$rootElement.append(elm);

‎test/ngMock/angular-mocksSpec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ describe('ngMock', function() {
160160

161161

162162
describe('$log', function() {
163-
forEach([true, false], function(debugEnabled) {
163+
angular.forEach([true, false], function(debugEnabled) {
164164
describe('debug ' + debugEnabled, function() {
165165
beforeEach(module(function($logProvider) {
166166
$logProvider.debugEnabled(debugEnabled);

‎test/ngRoute/directive/ngViewSpec.js‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ describe('ngView', function() {
279279

280280
it('should be async even if served from cache', function() {
281281
module(function($routeProvider) {
282-
$routeProvider.when('/foo', {controller: noop, templateUrl: 'myUrl1'});
282+
$routeProvider.when('/foo', {controller: angular.noop, templateUrl: 'myUrl1'});
283283
});
284284

285285
inject(function($route, $rootScope, $location, $templateCache) {
@@ -498,16 +498,16 @@ describe('ngView', function() {
498498
$location.url('/foo');
499499
$rootScope.$digest();
500500

501-
forEach(element.contents(), function(node) {
501+
angular.forEach(element.contents(), function(node) {
502502
if(node.nodeType == 3 /* text node */) {
503-
expect(jqLite(node).scope()).not.toBe($route.current.scope);
504-
expect(jqLite(node).controller()).not.toBeDefined();
503+
expect(angular.element(node).scope()).not.toBe($route.current.scope);
504+
expect(angular.element(node).controller()).not.toBeDefined();
505505
} else if(node.nodeType == 8 /* comment node */) {
506-
expect(jqLite(node).scope()).toBe(element.scope());
507-
expect(jqLite(node).controller()).toBe(element.controller());
506+
expect(angular.element(node).scope()).toBe(element.scope());
507+
expect(angular.element(node).controller()).toBe(element.controller());
508508
} else {
509-
expect(jqLite(node).scope()).toBe($route.current.scope);
510-
expect(jqLite(node).controller()).toBeDefined();
509+
expect(angular.element(node).scope()).toBe($route.current.scope);
510+
expect(angular.element(node).controller()).toBeDefined();
511511
}
512512
});
513513
});
@@ -530,7 +530,7 @@ describe('ngView animations', function() {
530530
// we need to run animation on attached elements;
531531
return function(_$rootElement_) {
532532
$rootElement = _$rootElement_;
533-
body = jqLite(document.body);
533+
body = angular.element(document.body);
534534
};
535535
}));
536536

@@ -541,8 +541,8 @@ describe('ngView animations', function() {
541541

542542

543543
beforeEach(module(function($provide, $routeProvider) {
544-
$routeProvider.when('/foo', {controller: noop, templateUrl: '/foo.html'});
545-
$routeProvider.when('/bar', {controller: noop, templateUrl: '/bar.html'});
544+
$routeProvider.when('/foo', {controller: angular.noop, templateUrl: '/foo.html'});
545+
$routeProvider.when('/bar', {controller: angular.noop, templateUrl: '/bar.html'});
546546
return function($templateCache) {
547547
$templateCache.put('/foo.html', [200, '<div>data</div>', {}]);
548548
$templateCache.put('/bar.html', [200, '<div>data2</div>', {}]);

‎test/ngRoute/routeSpec.js‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe('$route', function() {
2525

2626
module(function($routeProvider) {
2727
$routeProvider.when('/Book/:book/Chapter/:chapter',
28-
{controller: noop, templateUrl: 'Chapter.html'});
28+
{controller: angular.noop, templateUrl: 'Chapter.html'});
2929
$routeProvider.when('/Blank', {});
3030
});
3131
inject(function($route, $location, $rootScope) {
@@ -69,9 +69,9 @@ describe('$route', function() {
6969

7070
module(function($routeProvider) {
7171
$routeProvider.when('/Book1/:book/Chapter/:chapter/:highlight*/edit',
72-
{controller: noop, templateUrl: 'Chapter.html'});
72+
{controller: angular.noop, templateUrl: 'Chapter.html'});
7373
$routeProvider.when('/Book2/:book/:highlight*/Chapter/:chapter',
74-
{controller: noop, templateUrl: 'Chapter.html'});
74+
{controller: angular.noop, templateUrl: 'Chapter.html'});
7575
$routeProvider.when('/Blank', {});
7676
});
7777
inject(function($route, $location, $rootScope) {
@@ -128,9 +128,9 @@ describe('$route', function() {
128128

129129
module(function($routeProvider) {
130130
$routeProvider.when('/Book1/:book/Chapter/:chapter/:highlight*/edit',
131-
{controller: noop, templateUrl: 'Chapter.html', caseInsensitiveMatch: true});
131+
{controller: angular.noop, templateUrl: 'Chapter.html', caseInsensitiveMatch: true});
132132
$routeProvider.when('/Book2/:book/:highlight*/Chapter/:chapter',
133-
{controller: noop, templateUrl: 'Chapter.html'});
133+
{controller: angular.noop, templateUrl: 'Chapter.html'});
134134
$routeProvider.when('/Blank', {});
135135
});
136136
inject(function($route, $location, $rootScope) {
@@ -618,8 +618,8 @@ describe('$route', function() {
618618

619619
inject(function($route, $httpBackend, $location, $rootScope, $routeParams) {
620620
var log = '';
621-
$rootScope.$on('$routeChangeStart', function(e, next) { log += '$before' + toJson($routeParams) + ';'});
622-
$rootScope.$on('$routeChangeSuccess', function(e, next) { log += '$after' + toJson($routeParams) + ';'});
621+
$rootScope.$on('$routeChangeStart', function(e, next) { log += '$before' + angular.toJson($routeParams) + ';'});
622+
$rootScope.$on('$routeChangeSuccess', function(e, next) { log += '$after' + angular.toJson($routeParams) + ';'});
623623

624624
$httpBackend.whenGET('r1.html').respond('R1');
625625
$httpBackend.whenGET('r2.html').respond('R2');
@@ -876,7 +876,7 @@ describe('$route', function() {
876876
var reloaded = jasmine.createSpy('route reload');
877877

878878
module(function($routeProvider) {
879-
$routeProvider.when('/foo', {controller: noop});
879+
$routeProvider.when('/foo', {controller: angular.noop});
880880
});
881881

882882
inject(function($route, $location, $rootScope, $routeParams) {
@@ -901,7 +901,7 @@ describe('$route', function() {
901901
routeUpdate = jasmine.createSpy('route update');
902902

903903
module(function($routeProvider) {
904-
$routeProvider.when('/foo', {controller: noop, reloadOnSearch: false});
904+
$routeProvider.when('/foo', {controller: angular.noop, reloadOnSearch: false});
905905
});
906906

907907
inject(function($route, $location, $rootScope) {
@@ -931,7 +931,7 @@ describe('$route', function() {
931931
var routeChange = jasmine.createSpy('route change');
932932

933933
module(function($routeProvider) {
934-
$routeProvider.when('/foo/:fooId', {controller: noop, reloadOnSearch: false});
934+
$routeProvider.when('/foo/:fooId', {controller: angular.noop, reloadOnSearch: false});
935935
});
936936

937937
inject(function($route, $location, $rootScope) {
@@ -963,8 +963,8 @@ describe('$route', function() {
963963
var routeParamsWatcher = jasmine.createSpy('routeParamsWatcher');
964964

965965
module(function($routeProvider) {
966-
$routeProvider.when('/foo', {controller: noop});
967-
$routeProvider.when('/bar/:barId', {controller: noop, reloadOnSearch: false});
966+
$routeProvider.when('/foo', {controller: angular.noop});
967+
$routeProvider.when('/bar/:barId', {controller: angular.noop, reloadOnSearch: false});
968968
});
969969

970970
inject(function($route, $location, $rootScope, $routeParams) {

0 commit comments

Comments
 (0)