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

Commit 24a7f28

Browse files
committed
fix(ngMock): don't break if $rootScope.$destroy() is not a function
Previously, `angular-mocks` was calling `$rootScope.$destroy()` after each test as part of it's cleaning up, assuming that it was always available. This could break if `$rootScope` was mocked and the mocked version didn't provide the `$destroy()` method. This commit prevents the error by first checking that `$rootScope.$destroy` is present. Fixes #14106 Closes #14107
1 parent 146f9c1 commit 24a7f28

File tree

2 files changed

+130
-93
lines changed

2 files changed

+130
-93
lines changed

src/ngMock/angular-mocks.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -2330,7 +2330,10 @@ if (window.jasmine || window.mocha) {
23302330
}
23312331
angular.element.cleanData(cleanUpNodes);
23322332

2333-
injector.get('$rootScope').$destroy();
2333+
// Ensure `$destroy()` is available, before calling it
2334+
// (a mocked `$rootScope` might not implement it (or not even be an object at all))
2335+
var $rootScope = injector.get('$rootScope');
2336+
if ($rootScope && $rootScope.$destroy) $rootScope.$destroy();
23342337
}
23352338

23362339
// clean up jquery's fragment cache

test/ngMock/angular-mocksSpec.js

+126-92
Original file line numberDiff line numberDiff line change
@@ -1603,25 +1603,6 @@ describe('ngMock', function() {
16031603
});
16041604

16051605

1606-
describe('$rootScope', function() {
1607-
var destroyed = false;
1608-
var oldRootScope;
1609-
1610-
it('should destroy $rootScope after each test', inject(function($rootScope) {
1611-
$rootScope.$on('$destroy', function() {
1612-
destroyed = true;
1613-
});
1614-
oldRootScope = $rootScope;
1615-
}));
1616-
1617-
it('should have destroyed the $rootScope from the previous test', inject(function($rootScope) {
1618-
expect(destroyed).toBe(true);
1619-
expect($rootScope).not.toBe(oldRootScope);
1620-
expect(oldRootScope.$$destroyed).toBe(true);
1621-
}));
1622-
});
1623-
1624-
16251606
describe('$rootScopeDecorator', function() {
16261607

16271608
describe('$countChildScopes', function() {
@@ -2146,112 +2127,165 @@ describe('make sure that we can create an injector outside of tests', function()
21462127

21472128

21482129
describe('`afterEach` clean-up', function() {
2149-
describe('undecorated `$rootElement`', function() {
2150-
var prevRootElement;
2151-
var prevCleanDataSpy;
2130+
describe('`$rootElement`', function() {
2131+
describe('undecorated', function() {
2132+
var prevRootElement;
2133+
var prevCleanDataSpy;
21522134

21532135

2154-
it('should set up spies so the next test can verify `$rootElement` was cleaned up', function() {
2155-
module(function($provide) {
2156-
$provide.decorator('$rootElement', function($delegate) {
2157-
prevRootElement = $delegate;
2136+
it('should set up spies for the next test to verify that `$rootElement` was cleaned up',
2137+
function() {
2138+
module(function($provide) {
2139+
$provide.decorator('$rootElement', function($delegate) {
2140+
prevRootElement = $delegate;
21582141

2159-
// Spy on `angular.element.cleanData()`, so the next test can verify
2160-
// that it has been called as necessary
2161-
prevCleanDataSpy = spyOn(angular.element, 'cleanData').andCallThrough();
2142+
// Spy on `angular.element.cleanData()`, so the next test can verify
2143+
// that it has been called as necessary
2144+
prevCleanDataSpy = spyOn(angular.element, 'cleanData').andCallThrough();
2145+
2146+
return $delegate;
2147+
});
2148+
});
2149+
2150+
// Inject the `$rootElement` to ensure it has been created
2151+
inject(function($rootElement) {
2152+
expect($rootElement.injector()).toBeDefined();
2153+
});
2154+
}
2155+
);
21622156

2163-
return $delegate;
2164-
});
2165-
});
21662157

2167-
// Inject the `$rootElement` to ensure it has been created
2168-
inject(function($rootElement) {
2169-
expect($rootElement.injector()).toBeDefined();
2158+
it('should clean up `$rootElement` after each test', function() {
2159+
// One call is made by `testabilityPatch`'s `dealoc()`
2160+
// We want to verify the subsequent call, made by `angular-mocks`
2161+
expect(prevCleanDataSpy.callCount).toBe(2);
2162+
2163+
var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
2164+
expect(cleanUpNodes.length).toBe(1);
2165+
expect(cleanUpNodes[0]).toBe(prevRootElement[0]);
21702166
});
21712167
});
21722168

21732169

2174-
it('should clean up `$rootElement` after each test', function() {
2175-
// One call is made by `testabilityPatch`'s `dealoc()`
2176-
// We want to verify the subsequent call, made by `angular-mocks`
2177-
expect(prevCleanDataSpy.callCount).toBe(2);
2170+
describe('decorated', function() {
2171+
var prevOriginalRootElement;
2172+
var prevRootElement;
2173+
var prevCleanDataSpy;
21782174

2179-
var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
2180-
expect(cleanUpNodes.length).toBe(1);
2181-
expect(cleanUpNodes[0]).toBe(prevRootElement[0]);
2182-
});
2183-
});
21842175

2176+
it('should set up spies for the next text to verify that `$rootElement` was cleaned up',
2177+
function() {
2178+
module(function($provide) {
2179+
$provide.decorator('$rootElement', function($delegate) {
2180+
prevOriginalRootElement = $delegate;
21852181

2186-
describe('decorated `$rootElement`', function() {
2187-
var prevOriginalRootElement;
2188-
var prevRootElement;
2189-
var prevCleanDataSpy;
2182+
// Mock `$rootElement` to be able to verify that the correct object is cleaned up
2183+
prevRootElement = angular.element('<div></div>');
21902184

2185+
// Spy on `angular.element.cleanData()`, so the next test can verify
2186+
// that it has been called as necessary
2187+
prevCleanDataSpy = spyOn(angular.element, 'cleanData').andCallThrough();
21912188

2192-
it('should set up spies so the next text can verify `$rootElement` was cleaned up', function() {
2193-
module(function($provide) {
2194-
$provide.decorator('$rootElement', function($delegate) {
2195-
prevOriginalRootElement = $delegate;
2189+
return prevRootElement;
2190+
});
2191+
});
21962192

2197-
// Mock `$rootElement` to be able to verify that the correct object is cleaned up
2198-
prevRootElement = angular.element('<div></div>');
2193+
// Inject the `$rootElement` to ensure it has been created
2194+
inject(function($rootElement) {
2195+
expect($rootElement).toBe(prevRootElement);
2196+
expect(prevOriginalRootElement.injector()).toBeDefined();
2197+
expect(prevRootElement.injector()).toBeUndefined();
2198+
2199+
// If we don't clean up `prevOriginalRootElement`-related data now, `testabilityPatch` will
2200+
// complain about a memory leak, because it doesn't clean up after the original
2201+
// `$rootElement`
2202+
// This is a false alarm, because `angular-mocks` would have cleaned up in a subsequent
2203+
// `afterEach` block
2204+
prevOriginalRootElement.removeData();
2205+
});
2206+
}
2207+
);
21992208

2200-
// Spy on `angular.element.cleanData()`, so the next test can verify
2201-
// that it has been called as necessary
2202-
prevCleanDataSpy = spyOn(angular.element, 'cleanData').andCallThrough();
22032209

2204-
return prevRootElement;
2205-
});
2206-
});
2210+
it('should clean up `$rootElement` (both original and decorated) after each test',
2211+
function() {
2212+
// One call is made by `testabilityPatch`'s `dealoc()`
2213+
// We want to verify the subsequent call, made by `angular-mocks`
2214+
expect(prevCleanDataSpy.callCount).toBe(2);
22072215

2208-
// Inject the `$rootElement` to ensure it has been created
2209-
inject(function($rootElement) {
2210-
expect($rootElement).toBe(prevRootElement);
2211-
expect(prevOriginalRootElement.injector()).toBeDefined();
2212-
expect(prevRootElement.injector()).toBeUndefined();
2216+
var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
2217+
expect(cleanUpNodes.length).toBe(2);
2218+
expect(cleanUpNodes[0]).toBe(prevOriginalRootElement[0]);
2219+
expect(cleanUpNodes[1]).toBe(prevRootElement[0]);
2220+
}
2221+
);
2222+
});
2223+
2224+
2225+
describe('uninstantiated or falsy', function() {
2226+
it('should not break if `$rootElement` was never instantiated', function() {
2227+
// Just an empty test to verify that `angular-mocks` doesn't break,
2228+
// when trying to clean up `$rootElement`, if `$rootElement` was never injected in the test
2229+
// (and thus never instantiated/created)
22132230

2214-
// If we don't clean up `prevOriginalRootElement`-related data now, `testabilityPatch` will
2215-
// complain about a memory leak, because it doesn't clean up after the original
2216-
// `$rootElement`
2217-
// This is a false alarm, because `angular-mocks` would have cleaned up in a subsequent
2218-
// `afterEach` block
2219-
prevOriginalRootElement.removeData();
2231+
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2232+
inject(function() {});
22202233
});
2221-
});
22222234

22232235

2224-
it('should clean up `$rootElement` (both original and decorated) after each test', function() {
2225-
// One call is made by `testabilityPatch`'s `dealoc()`
2226-
// We want to verify the subsequent call, made by `angular-mocks`
2227-
expect(prevCleanDataSpy.callCount).toBe(2);
2236+
it('should not break if the decorated `$rootElement` is falsy (e.g. `null`)', function() {
2237+
module({$rootElement: null});
22282238

2229-
var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
2230-
expect(cleanUpNodes.length).toBe(2);
2231-
expect(cleanUpNodes[0]).toBe(prevOriginalRootElement[0]);
2232-
expect(cleanUpNodes[1]).toBe(prevRootElement[0]);
2239+
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2240+
inject(function() {});
2241+
});
22332242
});
22342243
});
22352244

22362245

2237-
describe('uninstantiated or falsy `$rootElement`', function() {
2238-
it('should not break if `$rootElement` was never instantiated', function() {
2239-
// Just an empty test to verify that `angular-mocks` doesn't break,
2240-
// when trying to clean up `$rootElement`, if `$rootElement` was never injected in the test
2241-
// (and thus never instantiated/created)
2246+
describe('`$rootScope`', function() {
2247+
describe('undecorated', function() {
2248+
var prevRootScope;
2249+
var prevDestroySpy;
22422250

2243-
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2244-
inject(function() {});
2251+
2252+
it('should set up spies for the next test to verify that `$rootScope` was cleaned up',
2253+
inject(function($rootScope) {
2254+
prevRootScope = $rootScope;
2255+
prevDestroySpy = spyOn($rootScope, '$destroy').andCallThrough();
2256+
})
2257+
);
2258+
2259+
2260+
it('should clean up `$rootScope` after each test', inject(function($rootScope) {
2261+
expect($rootScope).not.toBe(prevRootScope);
2262+
expect(prevDestroySpy).toHaveBeenCalledOnce();
2263+
expect(prevRootScope.$$destroyed).toBe(true);
2264+
}));
22452265
});
22462266

22472267

2248-
it('should not break if the decorated `$rootElement` is falsy (e.g. `null`)', function() {
2249-
module(function($provide) {
2250-
$provide.value('$rootElement', null);
2268+
describe('falsy or without `$destroy()` method', function() {
2269+
it('should not break if `$rootScope` is falsy (e.g. `null`)', function() {
2270+
// Just an empty test to verify that `angular-mocks` doesn't break,
2271+
// when trying to clean up a mocked `$rootScope` set to `null`
2272+
2273+
module({$rootScope: null});
2274+
2275+
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2276+
inject(function() {});
22512277
});
22522278

2253-
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2254-
inject(function() {});
2279+
2280+
it('should not break if `$rootScope.$destroy` is not a function', function() {
2281+
// Just an empty test to verify that `angular-mocks` doesn't break,
2282+
// when trying to clean up a mocked `$rootScope` without a `$destroy()` method
2283+
2284+
module({$rootScope: {}});
2285+
2286+
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2287+
inject(function() {});
2288+
});
22552289
});
22562290
});
22572291
});

0 commit comments

Comments
 (0)