X Tutup
Skip to content

Commit 871bebf

Browse files
gkalpakgkalpak
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 angular#14106 Closes angular#14107
1 parent 7e112c1 commit 871bebf

File tree

2 files changed

+130
-93
lines changed

2 files changed

+130
-93
lines changed

‎src/ngMock/angular-mocks.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2612,7 +2612,10 @@ if (window.jasmine || window.mocha) {
26122612
}
26132613
angular.element.cleanData(cleanUpNodes);
26142614

2615-
injector.get('$rootScope').$destroy();
2615+
// Ensure `$destroy()` is available, before calling it
2616+
// (a mocked `$rootScope` might not implement it (or not even be an object at all))
2617+
var $rootScope = injector.get('$rootScope');
2618+
if ($rootScope && $rootScope.$destroy) $rootScope.$destroy();
26162619
}
26172620

26182621
// clean up jquery's fragment cache

‎test/ngMock/angular-mocksSpec.js‎

Lines changed: 126 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1674,25 +1674,6 @@ describe('ngMock', function() {
16741674
});
16751675

16761676

1677-
describe('$rootScope', function() {
1678-
var destroyed = false;
1679-
var oldRootScope;
1680-
1681-
it('should destroy $rootScope after each test', inject(function($rootScope) {
1682-
$rootScope.$on('$destroy', function() {
1683-
destroyed = true;
1684-
});
1685-
oldRootScope = $rootScope;
1686-
}));
1687-
1688-
it('should have destroyed the $rootScope from the previous test', inject(function($rootScope) {
1689-
expect(destroyed).toBe(true);
1690-
expect($rootScope).not.toBe(oldRootScope);
1691-
expect(oldRootScope.$$destroyed).toBe(true);
1692-
}));
1693-
});
1694-
1695-
16961677
describe('$rootScopeDecorator', function() {
16971678

16981679
describe('$countChildScopes', function() {
@@ -2418,112 +2399,165 @@ describe('make sure that we can create an injector outside of tests', function()
24182399

24192400

24202401
describe('`afterEach` clean-up', function() {
2421-
describe('undecorated `$rootElement`', function() {
2422-
var prevRootElement;
2423-
var prevCleanDataSpy;
2402+
describe('`$rootElement`', function() {
2403+
describe('undecorated', function() {
2404+
var prevRootElement;
2405+
var prevCleanDataSpy;
24242406

24252407

2426-
it('should set up spies so the next test can verify `$rootElement` was cleaned up', function() {
2427-
module(function($provide) {
2428-
$provide.decorator('$rootElement', function($delegate) {
2429-
prevRootElement = $delegate;
2408+
it('should set up spies for the next test to verify that `$rootElement` was cleaned up',
2409+
function() {
2410+
module(function($provide) {
2411+
$provide.decorator('$rootElement', function($delegate) {
2412+
prevRootElement = $delegate;
24302413

2431-
// Spy on `angular.element.cleanData()`, so the next test can verify
2432-
// that it has been called as necessary
2433-
prevCleanDataSpy = spyOn(angular.element, 'cleanData').andCallThrough();
2414+
// Spy on `angular.element.cleanData()`, so the next test can verify
2415+
// that it has been called as necessary
2416+
prevCleanDataSpy = spyOn(angular.element, 'cleanData').andCallThrough();
2417+
2418+
return $delegate;
2419+
});
2420+
});
2421+
2422+
// Inject the `$rootElement` to ensure it has been created
2423+
inject(function($rootElement) {
2424+
expect($rootElement.injector()).toBeDefined();
2425+
});
2426+
}
2427+
);
24342428

2435-
return $delegate;
2436-
});
2437-
});
24382429

2439-
// Inject the `$rootElement` to ensure it has been created
2440-
inject(function($rootElement) {
2441-
expect($rootElement.injector()).toBeDefined();
2430+
it('should clean up `$rootElement` after each test', function() {
2431+
// One call is made by `testabilityPatch`'s `dealoc()`
2432+
// We want to verify the subsequent call, made by `angular-mocks`
2433+
expect(prevCleanDataSpy.callCount).toBe(2);
2434+
2435+
var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
2436+
expect(cleanUpNodes.length).toBe(1);
2437+
expect(cleanUpNodes[0]).toBe(prevRootElement[0]);
24422438
});
24432439
});
24442440

24452441

2446-
it('should clean up `$rootElement` after each test', function() {
2447-
// One call is made by `testabilityPatch`'s `dealoc()`
2448-
// We want to verify the subsequent call, made by `angular-mocks`
2449-
expect(prevCleanDataSpy.callCount).toBe(2);
2442+
describe('decorated', function() {
2443+
var prevOriginalRootElement;
2444+
var prevRootElement;
2445+
var prevCleanDataSpy;
24502446

2451-
var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
2452-
expect(cleanUpNodes.length).toBe(1);
2453-
expect(cleanUpNodes[0]).toBe(prevRootElement[0]);
2454-
});
2455-
});
24562447

2448+
it('should set up spies for the next text to verify that `$rootElement` was cleaned up',
2449+
function() {
2450+
module(function($provide) {
2451+
$provide.decorator('$rootElement', function($delegate) {
2452+
prevOriginalRootElement = $delegate;
24572453

2458-
describe('decorated `$rootElement`', function() {
2459-
var prevOriginalRootElement;
2460-
var prevRootElement;
2461-
var prevCleanDataSpy;
2454+
// Mock `$rootElement` to be able to verify that the correct object is cleaned up
2455+
prevRootElement = angular.element('<div></div>');
24622456

2457+
// Spy on `angular.element.cleanData()`, so the next test can verify
2458+
// that it has been called as necessary
2459+
prevCleanDataSpy = spyOn(angular.element, 'cleanData').andCallThrough();
24632460

2464-
it('should set up spies so the next text can verify `$rootElement` was cleaned up', function() {
2465-
module(function($provide) {
2466-
$provide.decorator('$rootElement', function($delegate) {
2467-
prevOriginalRootElement = $delegate;
2461+
return prevRootElement;
2462+
});
2463+
});
24682464

2469-
// Mock `$rootElement` to be able to verify that the correct object is cleaned up
2470-
prevRootElement = angular.element('<div></div>');
2465+
// Inject the `$rootElement` to ensure it has been created
2466+
inject(function($rootElement) {
2467+
expect($rootElement).toBe(prevRootElement);
2468+
expect(prevOriginalRootElement.injector()).toBeDefined();
2469+
expect(prevRootElement.injector()).toBeUndefined();
2470+
2471+
// If we don't clean up `prevOriginalRootElement`-related data now, `testabilityPatch` will
2472+
// complain about a memory leak, because it doesn't clean up after the original
2473+
// `$rootElement`
2474+
// This is a false alarm, because `angular-mocks` would have cleaned up in a subsequent
2475+
// `afterEach` block
2476+
prevOriginalRootElement.removeData();
2477+
});
2478+
}
2479+
);
24712480

2472-
// Spy on `angular.element.cleanData()`, so the next test can verify
2473-
// that it has been called as necessary
2474-
prevCleanDataSpy = spyOn(angular.element, 'cleanData').andCallThrough();
24752481

2476-
return prevRootElement;
2477-
});
2478-
});
2482+
it('should clean up `$rootElement` (both original and decorated) after each test',
2483+
function() {
2484+
// One call is made by `testabilityPatch`'s `dealoc()`
2485+
// We want to verify the subsequent call, made by `angular-mocks`
2486+
expect(prevCleanDataSpy.callCount).toBe(2);
2487+
2488+
var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
2489+
expect(cleanUpNodes.length).toBe(2);
2490+
expect(cleanUpNodes[0]).toBe(prevOriginalRootElement[0]);
2491+
expect(cleanUpNodes[1]).toBe(prevRootElement[0]);
2492+
}
2493+
);
2494+
});
24792495

2480-
// Inject the `$rootElement` to ensure it has been created
2481-
inject(function($rootElement) {
2482-
expect($rootElement).toBe(prevRootElement);
2483-
expect(prevOriginalRootElement.injector()).toBeDefined();
2484-
expect(prevRootElement.injector()).toBeUndefined();
24852496

2486-
// If we don't clean up `prevOriginalRootElement`-related data now, `testabilityPatch` will
2487-
// complain about a memory leak, because it doesn't clean up after the original
2488-
// `$rootElement`
2489-
// This is a false alarm, because `angular-mocks` would have cleaned up in a subsequent
2490-
// `afterEach` block
2491-
prevOriginalRootElement.removeData();
2497+
describe('uninstantiated or falsy', function() {
2498+
it('should not break if `$rootElement` was never instantiated', function() {
2499+
// Just an empty test to verify that `angular-mocks` doesn't break,
2500+
// when trying to clean up `$rootElement`, if `$rootElement` was never injected in the test
2501+
// (and thus never instantiated/created)
2502+
2503+
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2504+
inject(function() {});
24922505
});
2493-
});
24942506

24952507

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

2501-
var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
2502-
expect(cleanUpNodes.length).toBe(2);
2503-
expect(cleanUpNodes[0]).toBe(prevOriginalRootElement[0]);
2504-
expect(cleanUpNodes[1]).toBe(prevRootElement[0]);
2511+
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2512+
inject(function() {});
2513+
});
25052514
});
25062515
});
25072516

25082517

2509-
describe('uninstantiated or falsy `$rootElement`', function() {
2510-
it('should not break if `$rootElement` was never instantiated', function() {
2511-
// Just an empty test to verify that `angular-mocks` doesn't break,
2512-
// when trying to clean up `$rootElement`, if `$rootElement` was never injected in the test
2513-
// (and thus never instantiated/created)
2518+
describe('`$rootScope`', function() {
2519+
describe('undecorated', function() {
2520+
var prevRootScope;
2521+
var prevDestroySpy;
25142522

2515-
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2516-
inject(function() {});
2523+
2524+
it('should set up spies for the next test to verify that `$rootScope` was cleaned up',
2525+
inject(function($rootScope) {
2526+
prevRootScope = $rootScope;
2527+
prevDestroySpy = spyOn($rootScope, '$destroy').andCallThrough();
2528+
})
2529+
);
2530+
2531+
2532+
it('should clean up `$rootScope` after each test', inject(function($rootScope) {
2533+
expect($rootScope).not.toBe(prevRootScope);
2534+
expect(prevDestroySpy).toHaveBeenCalledOnce();
2535+
expect(prevRootScope.$$destroyed).toBe(true);
2536+
}));
25172537
});
25182538

25192539

2520-
it('should not break if the decorated `$rootElement` is falsy (e.g. `null`)', function() {
2521-
module(function($provide) {
2522-
$provide.value('$rootElement', null);
2540+
describe('falsy or without `$destroy()` method', function() {
2541+
it('should not break if `$rootScope` is falsy (e.g. `null`)', function() {
2542+
// Just an empty test to verify that `angular-mocks` doesn't break,
2543+
// when trying to clean up a mocked `$rootScope` set to `null`
2544+
2545+
module({$rootScope: null});
2546+
2547+
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2548+
inject(function() {});
25232549
});
25242550

2525-
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2526-
inject(function() {});
2551+
2552+
it('should not break if `$rootScope.$destroy` is not a function', function() {
2553+
// Just an empty test to verify that `angular-mocks` doesn't break,
2554+
// when trying to clean up a mocked `$rootScope` without a `$destroy()` method
2555+
2556+
module({$rootScope: {}});
2557+
2558+
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2559+
inject(function() {});
2560+
});
25272561
});
25282562
});
25292563
});

0 commit comments

Comments
 (0)
X Tutup