From e2941d5a98e91c5f61b200b2763e5fa0eb339365 Mon Sep 17 00:00:00 2001 From: Colin Snover Date: Sun, 9 Jan 2011 15:58:47 -0600 Subject: [PATCH] Update unit tests with a leak detection mechanism for the various jQuery globals and fix all leaks in the tests. --- test/data/testinit.js | 49 +++++++++++++++++++++++++++++++++++ test/index.html | 2 +- test/unit/ajax.js | 2 +- test/unit/attributes.js | 2 +- test/unit/core.js | 5 +++- test/unit/css.js | 2 +- test/unit/data.js | 4 +++ test/unit/dimensions.js | 18 ++++++++++++- test/unit/effects.js | 31 ++++++++++++++-------- test/unit/event.js | 41 ++++++++++++++++++++++++++--- test/unit/manipulation.js | 63 +++++++++++++++++++++++++++++++++++---------- test/unit/offset.js | 2 +- test/unit/queue.js | 2 +- test/unit/selector.js | 2 +- test/unit/traversing.js | 2 +- 15 files changed, 189 insertions(+), 38 deletions(-) diff --git a/test/data/testinit.js b/test/data/testinit.js index a66f71d..c478390 100644 --- a/test/data/testinit.js +++ b/test/data/testinit.js @@ -45,3 +45,52 @@ function t(a,b,c) { function url(value) { return value + (/\?/.test(value) ? "&" : "?") + new Date().getTime() + "" + parseInt(Math.random()*100000); } + +(function () { + // Store the old counts so that we only assert on tests that have actually leaked, + // instead of asserting every time a test has leaked sometime in the past + var oldCacheLength = 0, + oldFragmentsLength = 0, + oldTimersLength = 0, + oldActive = 0; + + /** + * Ensures that tests have cleaned up properly after themselves. Should be passed as the + * teardown function on all modules' lifecycle object. + */ + this.moduleTeardown = function () { + var i, fragmentsLength = 0, cacheLength = 0; + + // Allow QUnit.reset to clean up any attached elements before checking for leaks + QUnit.reset(); + + for ( i in jQuery.cache ) { + ++cacheLength; + } + + jQuery.fragments = {}; + + for ( i in jQuery.fragments ) { + ++fragmentsLength; + } + + // Because QUnit doesn't have a mechanism for retrieving the number of expected assertions for a test, + // if we unconditionally assert any of these, the test will fail with too many assertions :| + if ( cacheLength !== oldCacheLength ) { + equals( cacheLength, oldCacheLength, "No unit tests leak memory in jQuery.cache" ); + oldCacheLength = cacheLength; + } + if ( fragmentsLength !== oldFragmentsLength ) { + equals( fragmentsLength, oldFragmentsLength, "No unit tests leak memory in jQuery.fragments" ); + oldFragmentsLength = fragmentsLength; + } + if ( jQuery.timers.length !== oldTimersLength ) { + equals( jQuery.timers.length, oldTimersLength, "No timers are still running" ); + oldTimersLength = jQuery.timers.length; + } + if ( jQuery.active !== oldActive ) { + equals( jQuery.active, 0, "No AJAX requests are still active" ); + oldActive = jQuery.active; + } + } +}()); \ No newline at end of file diff --git a/test/index.html b/test/index.html index bbeda63..fc5f667 100644 --- a/test/index.html +++ b/test/index.html @@ -264,7 +264,7 @@ Z
slideToggleOut
slideToggleOut
fadeToggleIn
fadeToggleIn
-
fadeToggleOut
fadeToggleOut
+
fadeToggleOut
fadeToggleOut
fadeTo
fadeTo
diff --git a/test/unit/ajax.js b/test/unit/ajax.js index 773088f..d29eddb 100644 --- a/test/unit/ajax.js +++ b/test/unit/ajax.js @@ -1,4 +1,4 @@ -module("ajax"); +module("ajax", { teardown: moduleTeardown }); // Safari 3 randomly crashes when running these tests, // but only in the full suite - you can run just the Ajax diff --git a/test/unit/attributes.js b/test/unit/attributes.js index 04f1684..c58111d 100644 --- a/test/unit/attributes.js +++ b/test/unit/attributes.js @@ -1,4 +1,4 @@ -module("attributes"); +module("attributes", { teardown: moduleTeardown }); var bareObj = function(value) { return value; }; var functionReturningObj = function(value) { return (function() { return value; }); }; diff --git a/test/unit/core.js b/test/unit/core.js index 8fd0605..6231b1d 100644 --- a/test/unit/core.js +++ b/test/unit/core.js @@ -1,4 +1,4 @@ -module("core"); +module("core", { teardown: moduleTeardown }); test("Basic requirements", function() { expect(7); @@ -84,6 +84,9 @@ test("jQuery()", function() { exec = true; elem.click(); + + // manually clean up detached elements + elem.remove(); }); test("selector state", function() { diff --git a/test/unit/css.js b/test/unit/css.js index fbbf937..0637071 100644 --- a/test/unit/css.js +++ b/test/unit/css.js @@ -1,4 +1,4 @@ -module("css"); +module("css", { teardown: moduleTeardown }); test("css(String|Hash)", function() { expect(41); diff --git a/test/unit/data.js b/test/unit/data.js index 28e19a3..889fc2d 100644 --- a/test/unit/data.js +++ b/test/unit/data.js @@ -301,6 +301,8 @@ test("data-* attributes", function() { div.data("attr", "internal").attr("data-attr", "external"); equals( div.data("attr"), "internal", "Check for .data('attr') precedence (internal > external data-* attribute)" ); + div.remove(); + child.appendTo('#main'); equals( child.data("myobj"), "old data", "Value accessed from data-* attribute"); @@ -312,6 +314,8 @@ test("data-* attributes", function() { var obj = child.data(), obj2 = dummy.data(), check = [ "myobj", "ignored", "other" ], num = 0, num2 = 0; + dummy.remove(); + for ( var i = 0, l = check.length; i < l; i++ ) { ok( obj[ check[i] ], "Make sure data- property exists when calling data-." ); ok( obj2[ check[i] ], "Make sure data- property exists when calling data-." ); diff --git a/test/unit/dimensions.js b/test/unit/dimensions.js index b38e73b..fa59a9f 100644 --- a/test/unit/dimensions.js +++ b/test/unit/dimensions.js @@ -1,4 +1,4 @@ -module("dimensions"); +module("dimensions", { teardown: moduleTeardown }); function pass( val ) { return val; @@ -33,6 +33,8 @@ function testWidth( val ) { var blah = jQuery("blah"); equals( blah.width( val(10) ), blah, "Make sure that setting a width on an empty set returns the set." ); equals( blah.width(), null, "Make sure 'null' is returned on an empty set"); + + jQuery.removeData($div[0], 'olddisplay', true); } test("width()", function() { @@ -80,6 +82,8 @@ function testHeight( val ) { var blah = jQuery("blah"); equals( blah.height( val(10) ), blah, "Make sure that setting a height on an empty set returns the set." ); equals( blah.height(), null, "Make sure 'null' is returned on an empty set"); + + jQuery.removeData($div[0], 'olddisplay', true); } test("height()", function() { @@ -126,6 +130,9 @@ test("innerWidth()", function() { // Temporarily require 0 for backwards compat - should be auto equals( div.innerWidth(), 0, "Make sure that disconnected nodes are handled." ); + + div.remove(); + jQuery.removeData($div[0], 'olddisplay', true); }); test("innerHeight()", function() { @@ -152,6 +159,9 @@ test("innerHeight()", function() { // Temporarily require 0 for backwards compat - should be auto equals( div.innerHeight(), 0, "Make sure that disconnected nodes are handled." ); + + div.remove(); + jQuery.removeData($div[0], 'olddisplay', true); }); test("outerWidth()", function() { @@ -179,6 +189,9 @@ test("outerWidth()", function() { // Temporarily require 0 for backwards compat - should be auto equals( div.outerWidth(), 0, "Make sure that disconnected nodes are handled." ); + + div.remove(); + jQuery.removeData($div[0], 'olddisplay', true); }); test("outerHeight()", function() { @@ -205,4 +218,7 @@ test("outerHeight()", function() { // Temporarily require 0 for backwards compat - should be auto equals( div.outerHeight(), 0, "Make sure that disconnected nodes are handled." ); + + div.remove(); + jQuery.removeData($div[0], 'olddisplay', true); }); diff --git a/test/unit/effects.js b/test/unit/effects.js index cf9d131..b1dd288 100644 --- a/test/unit/effects.js +++ b/test/unit/effects.js @@ -1,4 +1,4 @@ -module("effects"); +module("effects", { teardown: moduleTeardown }); test("sanity check", function() { expect(1); @@ -14,7 +14,7 @@ test("show()", function() { equals( hiddendiv.css("display"), "block", "Make sure a pre-hidden div is visible." ); - var div = jQuery("
").hide().appendTo("body").show(); + var div = jQuery("
").hide().appendTo("#main").show(); equal( div.css("display"), "block", "Make sure pre-hidden divs show" ); @@ -403,13 +403,16 @@ test("animate duration 0", function() { $elem.hide(0, function(){ ok(true, "Hide callback with no duration"); }); + + // manually clean up detached elements + $elem.remove(); }); test("animate hyphenated properties", function(){ expect(1); stop(); - jQuery("#nothiddendiv") + jQuery("#foo") .css("font-size", 10) .animate({"font-size": 20}, 200, function(){ equals( this.style.fontSize, "20px", "The font-size property was animated." ); @@ -433,7 +436,7 @@ test("stop()", function() { expect(3); stop(); - var $foo = jQuery("#nothiddendiv"); + var $foo = jQuery("#foo"); var w = 0; $foo.hide().width(200).width(); @@ -446,6 +449,8 @@ test("stop()", function() { nw = $foo.width(); notEqual( nw, w, "Stop didn't reset the animation " + nw + "px " + w + "px"); setTimeout(function(){ + $foo.removeData(); + $foo.removeData(undefined, true); equals( nw, $foo.width(), "The animation didn't continue" ); start(); }, 100); @@ -456,7 +461,7 @@ test("stop() - several in queue", function() { expect(3); stop(); - var $foo = jQuery("#nothiddendivchild"); + var $foo = jQuery("#foo"); var w = 0; $foo.hide().width(200).width(); @@ -481,7 +486,7 @@ test("stop(clearQueue)", function() { expect(4); stop(); - var $foo = jQuery("#nothiddendiv"); + var $foo = jQuery("#foo"); var w = 0; $foo.hide().width(200).width(); @@ -508,7 +513,7 @@ test("stop(clearQueue, gotoEnd)", function() { expect(1); stop(); - var $foo = jQuery("#nothiddendivchild"); + var $foo = jQuery("#foo"); var w = 0; $foo.hide().width(200).width(); @@ -536,7 +541,7 @@ test("stop(clearQueue, gotoEnd)", function() { test("toggle()", function() { expect(6); - var x = jQuery("#nothiddendiv"); + var x = jQuery("#foo"); ok( x.is(":visible"), "is visible" ); x.toggle(); ok( x.is(":hidden"), "is hidden" ); @@ -737,6 +742,9 @@ jQuery.each( { } } + // manually remove generated element + jQuery(this).remove(); + start(); }); }); @@ -763,6 +771,10 @@ jQuery.checkState = function(){ var cur = self.style[ c ] || jQuery.css(self, c); equals( cur, v, "Make sure that " + c + " is reset (Old: " + v + " Cur: " + cur + ")"); }); + + // manually clean data on modified element + jQuery.removeData(this, 'olddisplay', true); + start(); } @@ -829,9 +841,6 @@ jQuery.makeTest = function( text ){ jQuery("

") .text( text ) .appendTo("#fx-tests") - .click(function(){ - jQuery(this).next().toggle(); - }) .after( elem ); return elem; diff --git a/test/unit/event.js b/test/unit/event.js index cae1a83..a6d8cb6 100644 --- a/test/unit/event.js +++ b/test/unit/event.js @@ -1,4 +1,4 @@ -module("event"); +module("event", { teardown: moduleTeardown }); test("null or undefined handler", function() { expect(2); @@ -80,6 +80,9 @@ test("bind(), multiple events at once and namespaces", function() { cur = "focusin"; div.trigger("focusin.a"); + // manually clean up detached elements + div.remove(); + div = jQuery("
").bind("click mouseover", obj, function(e) { equals( e.type, cur, "Verify right multi event was fired." ); equals( e.data, obj, "Make sure the data came in correctly." ); @@ -91,6 +94,9 @@ test("bind(), multiple events at once and namespaces", function() { cur = "mouseover"; div.trigger("mouseover"); + // manually clean up detached elements + div.remove(); + div = jQuery("
").bind("focusin.a focusout.b", function(e) { equals( e.type, cur, "Verify right multi event was fired." ); }); @@ -100,6 +106,9 @@ test("bind(), multiple events at once and namespaces", function() { cur = "focusout"; div.trigger("focusout.b"); + + // manually clean up detached elements + div.remove(); }); test("bind(), namespace with special add", function() { @@ -525,6 +534,9 @@ test("bind(name, false), unbind(name, false)", function() { jQuery("#ap").unbind("click", false); jQuery("#ap").trigger("click"); equals( main, 1, "Verify that the trigger happened correctly." ); + + // manually clean up events from elements outside the fixture + jQuery("#main").unbind("click"); }); test("bind()/trigger()/unbind() on plain object", function() { @@ -663,13 +675,18 @@ test("hover()", function() { test("trigger() shortcuts", function() { expect(6); - jQuery('
  • Change location
  • ').prependTo('#firstUL').find('a').bind('click', function() { + + var elem = jQuery('
  • Change location
  • ').prependTo('#firstUL'); + elem.find('a').bind('click', function() { var close = jQuery('spanx', this); // same with jQuery(this).find('span'); equals( close.length, 0, "Context element does not exist, length must be zero" ); ok( !close[0], "Context element does not exist, direct access to element must return undefined" ); return false; }).click(); + // manually clean up detached elements + elem.remove(); + jQuery("#check1").click(function() { ok( true, "click event handler for checkbox gets fired twice, see #815" ); }).click(); @@ -688,9 +705,12 @@ test("trigger() shortcuts", function() { jQuery('#simon1').click(); equals( clickCounter, 1, "Check that click, triggers onclick event handler on an a tag also" ); - jQuery('').load(function(){ + elem = jQuery('').load(function(){ ok( true, "Trigger the load event, using the shortcut .load() (#2819)"); }).load(); + + // manually clean up detached elements + elem.remove(); }); test("trigger() bubbling", function() { @@ -725,6 +745,10 @@ test("trigger() bubbling", function() { equals( body, 2, "ap bubble" ); equals( main, 1, "ap bubble" ); equals( ap, 1, "ap bubble" ); + + // manually clean up events from elements outside the fixture + jQuery(document).unbind("click"); + jQuery("html, body, #main").unbind("click"); }); test("trigger(type, [data], [fn])", function() { @@ -768,7 +792,7 @@ test("trigger(type, [data], [fn])", function() { pass = true; try { - jQuery('table:first').bind('test:test', function(){}).trigger('test:test'); + jQuery('#main table:first').bind('test:test', function(){}).trigger('test:test'); } catch (e) { pass = false; } @@ -952,6 +976,9 @@ test("toggle(Function, Function, ...)", function() { var data = jQuery._data( $div[0], 'events' ); ok( !data, "Unbinding one function from toggle unbinds them all"); + // manually clean up detached elements + $div.remove(); + // Test Multi-Toggles var a = [], b = []; $div = jQuery("
    "); @@ -967,6 +994,9 @@ test("toggle(Function, Function, ...)", function() { $div.click(); same( a, [1,2,1], "Check that a click worked with a second toggle, second click." ); same( b, [1,2], "Check that a click worked with a second toggle, second click." ); + + // manually clean up detached elements + $div.remove(); }); test(".live()/.die()", function() { @@ -1277,6 +1307,9 @@ test("live with multiple events", function(){ div.trigger("submit"); equals( count, 2, "Make sure both the click and submit were triggered." ); + + // manually clean up events from elements outside the fixture + div.die(); }); test("live with namespaces", function(){ diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js index e273cf0..de84144 100644 --- a/test/unit/manipulation.js +++ b/test/unit/manipulation.js @@ -1,4 +1,4 @@ -module("manipulation"); +module("manipulation", { teardown: moduleTeardown }); // Ensure that an extended Array prototype doesn't break jQuery Array.prototype.arrayProtoFn = function(arg) { throw("arrayProtoFn should not be called"); }; @@ -115,12 +115,19 @@ var testWrap = function(val) { // Wrap an element with a jQuery set and event result = jQuery("
    ").click(function(){ ok(true, "Event triggered."); + + // Remove handlers on detached elements + result.unbind(); + jQuery(this).unbind(); }); j = jQuery("").wrap(result); equals( j[0].parentNode.nodeName.toLowerCase(), "div", "Wrapping works." ); j.parent().trigger("click"); + + // clean up attached elements + QUnit.reset(); } test("wrap(String|Element)", function() { @@ -408,8 +415,12 @@ test("append the same fragment with events (Bug #6997, 5566)", function () { ok(true, "Event exists on original after being unbound on clone"); jQuery(this).unbind('click'); }); - element.clone(true).unbind('click')[0].fireEvent('onclick'); + var clone = element.clone(true).unbind('click'); + clone[0].fireEvent('onclick'); element[0].fireEvent('onclick'); + + // manually clean up detached elements + clone.remove(); } element = jQuery("").click(function () { @@ -894,20 +905,36 @@ test("clone()", function() { ok( true, "Bound event still exists." ); }); - div = div.clone(true).clone(true); + clone = div.clone(true); + + // manually clean up detached elements + div.remove(); + + div = clone.clone(true); + + // manually clean up detached elements + clone.remove(); + equals( div.length, 1, "One element cloned" ); equals( div[0].nodeName.toUpperCase(), "DIV", "DIV element cloned" ); div.trigger("click"); + // manually clean up detached elements + div.remove(); + div = jQuery("
    ").append([ document.createElement("table"), document.createElement("table") ]); div.find("table").click(function(){ ok( true, "Bound event still exists." ); }); - div = div.clone(true); - equals( div.length, 1, "One element cloned" ); - equals( div[0].nodeName.toUpperCase(), "DIV", "DIV element cloned" ); - div.find("table:last").trigger("click"); + clone = div.clone(true); + equals( clone.length, 1, "One element cloned" ); + equals( clone[0].nodeName.toUpperCase(), "DIV", "DIV element cloned" ); + clone.find("table:last").trigger("click"); + + // manually clean up detached elements + div.remove(); + clone.remove(); // this is technically an invalid object, but because of the special // classid instantiation it is the only kind that IE has trouble with, @@ -928,12 +955,16 @@ test("clone()", function() { equals( clone[0].nodeName.toUpperCase(), "DIV", "DIV element cloned" ); div = jQuery("
    ").data({ a: true }); - var div2 = div.clone(true); - equals( div2.data("a"), true, "Data cloned." ); - div2.data("a", false); - equals( div2.data("a"), false, "Ensure cloned element data object was correctly modified" ); + clone = div.clone(true); + equals( clone.data("a"), true, "Data cloned." ); + clone.data("a", false); + equals( clone.data("a"), false, "Ensure cloned element data object was correctly modified" ); equals( div.data("a"), true, "Ensure cloned element data object is copied, not referenced" ); + // manually clean up detached elements + div.remove(); + clone.remove(); + var form = document.createElement("form"); form.action = "/test/"; var div = document.createElement("div"); @@ -1141,15 +1172,21 @@ var testRemove = function(method) { jQuery("#nonnodes").contents()[method](); equals( jQuery("#nonnodes").contents().length, 0, "Check node,textnode,comment remove works" ); + // manually clean up detached elements + if (method === "detach") { + first.remove(); + } + QUnit.reset(); var count = 0; var first = jQuery("#ap").children(":first"); - var cleanUp = first.click(function() { count++ })[method]().appendTo("body").click(); + var cleanUp = first.click(function() { count++ })[method]().appendTo("#main").click(); equals( method == "remove" ? 0 : 1, count ); - cleanUp.detach(); + // manually clean up detached elements + cleanUp.remove(); }; test("remove()", function() { diff --git a/test/unit/offset.js b/test/unit/offset.js index cfa1444..329d69f 100644 --- a/test/unit/offset.js +++ b/test/unit/offset.js @@ -1,4 +1,4 @@ -module("offset"); +module("offset", { teardown: moduleTeardown }); test("disconnected node", function() { expect(2); diff --git a/test/unit/queue.js b/test/unit/queue.js index eada0ee..31e587d 100644 --- a/test/unit/queue.js +++ b/test/unit/queue.js @@ -1,4 +1,4 @@ -module("queue"); +module("queue", { teardown: moduleTeardown }); test("queue() with other types",function() { expect(9); diff --git a/test/unit/selector.js b/test/unit/selector.js index 5b758c1..c530c1f 100644 --- a/test/unit/selector.js +++ b/test/unit/selector.js @@ -1,4 +1,4 @@ -module("selector"); +module("selector", { teardown: moduleTeardown }); test("element", function() { expect(21); diff --git a/test/unit/traversing.js b/test/unit/traversing.js index 31cffc2..f0471d7 100644 --- a/test/unit/traversing.js +++ b/test/unit/traversing.js @@ -1,4 +1,4 @@ -module("traversing"); +module("traversing", { teardown: moduleTeardown }); test("find(String)", function() { expect(5); -- 1.7.10.4