Skip to content

Commit 1467e15

Browse files
author
Matias Niemela
committed
revert: fix(ngMock): don't break if $rootScope.$destroy() is not a function
This reverts commit 24a7f28.
1 parent 389349e commit 1467e15

File tree

2 files changed

+93
-130
lines changed

2 files changed

+93
-130
lines changed

‎src/ngMock/angular-mocks.js‎

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

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();
2333+
injector.get('$rootScope').$destroy();
23372334
}
23382335

23392336
// clean up jquery's fragment cache

‎test/ngMock/angular-mocksSpec.js‎

Lines changed: 92 additions & 126 deletions
Original file line numberDiff line numberDiff line change
@@ -1603,6 +1603,25 @@ 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+
16061625
describe('$rootScopeDecorator', function() {
16071626

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

21282147

21292148
describe('`afterEach` clean-up', function() {
2130-
describe('`$rootElement`', function() {
2131-
describe('undecorated', function() {
2132-
var prevRootElement;
2133-
var prevCleanDataSpy;
2134-
2135-
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;
2141-
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();
2149+
describe('undecorated `$rootElement`', function() {
2150+
var prevRootElement;
2151+
var prevCleanDataSpy;
21452152

2146-
return $delegate;
2147-
});
2148-
});
21492153

2150-
// Inject the `$rootElement` to ensure it has been created
2151-
inject(function($rootElement) {
2152-
expect($rootElement.injector()).toBeDefined();
2153-
});
2154-
}
2155-
);
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;
21562158

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();
21572162

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);
2163+
return $delegate;
2164+
});
2165+
});
21622166

2163-
var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
2164-
expect(cleanUpNodes.length).toBe(1);
2165-
expect(cleanUpNodes[0]).toBe(prevRootElement[0]);
2167+
// Inject the `$rootElement` to ensure it has been created
2168+
inject(function($rootElement) {
2169+
expect($rootElement.injector()).toBeDefined();
21662170
});
21672171
});
21682172

21692173

2170-
describe('decorated', function() {
2171-
var prevOriginalRootElement;
2172-
var prevRootElement;
2173-
var prevCleanDataSpy;
2174-
2175-
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;
2181-
2182-
// Mock `$rootElement` to be able to verify that the correct object is cleaned up
2183-
prevRootElement = angular.element('<div></div>');
2184-
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();
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);
21882178

2189-
return prevRootElement;
2190-
});
2191-
});
2179+
var cleanUpNodes = prevCleanDataSpy.calls[1].args[0];
2180+
expect(cleanUpNodes.length).toBe(1);
2181+
expect(cleanUpNodes[0]).toBe(prevRootElement[0]);
2182+
});
2183+
});
21922184

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-
);
22082185

2186+
describe('decorated `$rootElement`', function() {
2187+
var prevOriginalRootElement;
2188+
var prevRootElement;
2189+
var prevCleanDataSpy;
22092190

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);
22152191

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-
});
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;
22232196

2197+
// Mock `$rootElement` to be able to verify that the correct object is cleaned up
2198+
prevRootElement = angular.element('<div></div>');
22242199

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)
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();
22302203

2231-
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2232-
inject(function() {});
2204+
return prevRootElement;
2205+
});
22332206
});
22342207

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();
22352213

2236-
it('should not break if the decorated `$rootElement` is falsy (e.g. `null`)', function() {
2237-
module({$rootElement: null});
2238-
2239-
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2240-
inject(function() {});
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();
22412220
});
22422221
});
2243-
});
22442222

22452223

2246-
describe('`$rootScope`', function() {
2247-
describe('undecorated', function() {
2248-
var prevRootScope;
2249-
var prevDestroySpy;
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);
22502228

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]);
2233+
});
2234+
});
22512235

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-
);
22582236

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)
22592242

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-
}));
2243+
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2244+
inject(function() {});
22652245
});
22662246

22672247

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() {});
2248+
it('should not break if the decorated `$rootElement` is falsy (e.g. `null`)', function() {
2249+
module(function($provide) {
2250+
$provide.value('$rootElement', null);
22772251
});
22782252

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-
});
2253+
// Ensure the `$injector` is created - if there is no `$injector`, no clean-up takes places
2254+
inject(function() {});
22892255
});
22902256
});
22912257
});

0 commit comments

Comments
 (0)