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

Commit 9e62160

Browse files
committed
fixup 2
- Unify th clean-up paths for test with/without jQuery, using `jqLite.cleanData()`. - Remove redundant calls to `.off().removeData()`. - Fix bug in `testabilityPatch.js`, where `cleanData()` was called with an array of jqLite/jQuery elements (instead of "raw" nodes).
1 parent b972a12 commit 9e62160

File tree

3 files changed

+26
-70
lines changed

3 files changed

+26
-70
lines changed

src/ngMock/angular-mocks.js

+4-10
Original file line numberDiff line numberDiff line change
@@ -2602,16 +2602,10 @@ if (window.jasmine || window.mocha) {
26022602
currentSpec = null;
26032603

26042604
if (injector) {
2605-
var cleanUpElems = [originalRootElement];
2606-
var $rootElement = injector.get('$rootElement');
2607-
if ($rootElement !== originalRootElement) cleanUpElems.push($rootElement);
2608-
cleanUpElems.forEach(function(elem) {
2609-
// The `$rootElement` might not have been created or
2610-
// a mocked `$rootElement` might not have the standard methods
2611-
if (elem && elem.off) elem.off();
2612-
if (elem && elem.removeData) elem.removeData();
2613-
});
2614-
if (window.jQuery) window.jQuery.cleanData(cleanUpElems);
2605+
var cleanUpElems = [originalRootElement[0]];
2606+
var rootNode = injector.get('$rootElement')[0];
2607+
if (rootNode && (rootNode !== originalRootElement[0])) cleanUpElems.push(rootNode);
2608+
jqLite.cleanData(cleanUpElems);
26152609

26162610
injector.get('$rootScope').$destroy();
26172611
}

test/helpers/testabilityPatch.js

+2-6
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,8 @@ function dealoc(obj) {
139139
}
140140

141141
function cleanup(element) {
142-
element.off().removeData();
143-
if (window.jQuery) {
144-
// jQuery 2.x doesn't expose the cache storage; ensure all element data
145-
// is removed during its cleanup.
146-
jQuery.cleanData([element]);
147-
}
142+
jqLite.cleanData(element);
143+
148144
// Note: We aren't using element.contents() here. Under jQuery, element.contents() can fail
149145
// for IFRAME elements. jQuery explicitly uses (element.contentDocument ||
150146
// element.contentWindow.document) and both properties are null for IFRAMES that aren't attached

test/ngMock/angular-mocksSpec.js

+20-54
Original file line numberDiff line numberDiff line change
@@ -2421,21 +2421,17 @@ describe('`afterEach` clean-up', function() {
24212421
describe('undecorated `$rootElement`', function() {
24222422
var jQuery = window.jQuery;
24232423
var prevRootElement;
2424-
var prevRemoveDataSpy;
24252424
var prevCleanDataSpy;
24262425

24272426

24282427
it('should set up spies so the next test can verify `$rootElement` was cleaned up', function() {
24292428
module(function($provide) {
24302429
$provide.decorator('$rootElement', function($delegate) {
2431-
// Spy on `$rootElement.removeData()` and `jQuery.cleanData()`,
2432-
// so the next test can verify that they have been called as necessary
24332430
prevRootElement = $delegate;
2434-
prevRemoveDataSpy = spyOn($delegate, 'removeData').andCallThrough();
24352431

2436-
if (jQuery) {
2437-
prevCleanDataSpy = spyOn(jQuery, 'cleanData').andCallThrough();
2438-
}
2432+
// Spy on `jqLite.cleanData()`, so the next test can verify
2433+
// that it has been called as necessary
2434+
prevCleanDataSpy = spyOn(jqLite, 'cleanData').andCallThrough();
24392435

24402436
return $delegate;
24412437
});
@@ -2451,51 +2447,35 @@ describe('`afterEach` clean-up', function() {
24512447
it('should clean up `$rootElement` after each test', function() {
24522448
// One call is made by `testabilityPatch`'s `dealoc()`
24532449
// We want to verify the subsequent call, made by `angular-mocks`
2454-
// (Since `testabilityPatch` re-wrapps `$rootElement` and because jQuery returns a different
2455-
// object when re-wrapping, we don't capture the 1st call when using jQuery)
2456-
expect(prevRemoveDataSpy.callCount).toBe(jQuery ? 1 : 2);
2457-
2458-
if (jQuery) {
2459-
// One call is made by `testabilityPatch`'s `dealoc()`
2460-
// We want to verify the subsequent call, made by `angular-mocks`
2461-
expect(prevCleanDataSpy.callCount).toBe(2);
2462-
2463-
var cleanUpElems = prevCleanDataSpy.calls[1].args[0];
2464-
expect(cleanUpElems.length).toBe(1);
2465-
expect(cleanUpElems[0][0]).toBe(prevRootElement[0]);
2466-
}
2450+
expect(prevCleanDataSpy.callCount).toBe(2);
2451+
2452+
var cleanUpElems = prevCleanDataSpy.calls[1].args[0];
2453+
expect(cleanUpElems.length).toBe(1);
2454+
expect(cleanUpElems[0]).toBe(prevRootElement[0]);
24672455
});
24682456
});
24692457

24702458

24712459
describe('decorated `$rootElement`', function() {
24722460
var jQuery = window.jQuery;
24732461
var prevOriginalRootElement;
2474-
var prevOriginalRemoveDataSpy;
24752462
var prevRootElement;
2476-
var prevRemoveDataSpy;
24772463
var prevCleanDataSpy;
24782464

24792465

24802466
it('should set up spies so the next text can verify `$rootElement` was cleaned up', function() {
24812467
module(function($provide) {
24822468
$provide.decorator('$rootElement', function($delegate) {
2483-
// Mock `$rootElement` to be able to verify that the correct object is cleaned up
2484-
var mockRootElement = angular.element('<div></div>');
2485-
2486-
// Spy on `$rootElement.removeData()` and `jQuery.cleanData()`,
2487-
// so the next test can verify that they have been called as necessary
24882469
prevOriginalRootElement = $delegate;
2489-
prevOriginalRemoveDataSpy = spyOn($delegate, 'removeData').andCallThrough();
24902470

2491-
prevRootElement = mockRootElement;
2492-
prevRemoveDataSpy = spyOn(mockRootElement, 'removeData').andCallThrough();
2471+
// Mock `$rootElement` to be able to verify that the correct object is cleaned up
2472+
prevRootElement = angular.element('<div></div>');
24932473

2494-
if (jQuery) {
2495-
prevCleanDataSpy = spyOn(jQuery, 'cleanData').andCallThrough();
2496-
}
2474+
// Spy on `jqLite.cleanData()`, so the next test can verify
2475+
// that it has been called as necessary
2476+
prevCleanDataSpy = spyOn(jqLite, 'cleanData').andCallThrough();
24972477

2498-
return mockRootElement;
2478+
return prevRootElement;
24992479
});
25002480
});
25012481

@@ -2511,33 +2491,19 @@ describe('`afterEach` clean-up', function() {
25112491
// This is a false alarm, because `angular-mocks` would have cleaned up in a subsequent
25122492
// `afterEach` block
25132493
prevOriginalRootElement.removeData();
2514-
prevOriginalRemoveDataSpy.reset();
2515-
2516-
expect(prevOriginalRemoveDataSpy.callCount).toBe(0);
25172494
});
25182495
});
25192496

25202497

25212498
it('should clean up `$rootElement` (both original and decorated) after each test', function() {
2522-
// Only `angular-mocks` cleans up after the original `$rootElement`, not `testabilityPatch`
2523-
expect(prevOriginalRemoveDataSpy.callCount).toBe(1);
2524-
25252499
// One call is made by `testabilityPatch`'s `dealoc()`
25262500
// We want to verify the subsequent call, made by `angular-mocks`
2527-
// (Since `testabilityPatch` re-wrapps `$rootElement` and because jQuery returns a different
2528-
// object when re-wrapping, we don't capture the 1st call when using jQuery)
2529-
expect(prevRemoveDataSpy.callCount).toBe(jQuery ? 1 : 2);
2530-
2531-
if (jQuery) {
2532-
// One call is made by `testabilityPatch`'s `dealoc()`
2533-
// We want to verify the subsequent call, made by `angular-mocks`
2534-
expect(prevCleanDataSpy.callCount).toBe(2);
2535-
2536-
var cleanUpElems = prevCleanDataSpy.calls[1].args[0];
2537-
expect(cleanUpElems.length).toBe(2);
2538-
expect(cleanUpElems[0][0]).toBe(prevOriginalRootElement[0]);
2539-
expect(cleanUpElems[1][0]).toBe(prevRootElement[0]);
2540-
}
2501+
expect(prevCleanDataSpy.callCount).toBe(2);
2502+
2503+
var cleanUpElems = prevCleanDataSpy.calls[1].args[0];
2504+
expect(cleanUpElems.length).toBe(2);
2505+
expect(cleanUpElems[0]).toBe(prevOriginalRootElement[0]);
2506+
expect(cleanUpElems[1]).toBe(prevRootElement[0]);
25412507
});
25422508
});
25432509
});

0 commit comments

Comments
 (0)