From 36c9ecb0f50aec9063725b0c45e0cca98ef87084 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Wed, 17 Oct 2012 04:33:47 -0400
Subject: [PATCH] Implement expectation test instead of using _removeData.
 Close gh-997.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

* Removed inline usage of QUnit.reset() because it is messing with the
  expectation model as reset does .empty() which does a recursive cleanData
  on everything in #qunit-fixture, so any expectJqData above .reset() would
  fail negatively.

  Instead of calling reset inline, either updated the following assertions to
  take previous assertions' state into account, or broke the test() up into
  2 tests at the point where it would call QUnit.reset.

* After introducing the new memory leak discovery a whole bunch of tests were
  failing as they didn't clean up everything. However I didn't (yet) add
  QUnit.expectJqData calls all over the place because in most if not all of
  these cases it is valid data storage. For example in test "data()", there
  will be an internal data key for "parsedAttrs". This particular test isn't
  intending to test for memory leaks, so therefor I made the new discovery
  system only push failures when the test contains at least 1 call to
  QUnit.expectJqData.

  When not, we'll assume that whatever data is being stored is acceptable
  because the relevant elements still exist in the DOM anyway (QUnit.reset
  will remove the elements and clean up the data automatically).

  I did add a "Always check jQuery.data" mode in the test suite that will
  trigger it everywhere. Maybe one day we'll include a call to everywhere,
  but for now I'm keeping the status quo: Only consider data left in storage
  to be a problem if the test says so ("opt-in").

* Had to move #fx-tests inside the fixture because ".remove()" test would
  otherwise remove stuff permanently and cause random other tests to fail
  as "#hide div" would yield an empty collection.
  (Why wasn't this in the fixture in the first place?)

  As a result moving fx-tests into the fixture a whole bunch of tests failed
  that relied on arbitrary stuff about the document-wide or fixture-wide
  state (e.g. number of divs etc.). So I had to adjust various tests to
  limit their sample data to not be so variable and unlimited...

* Moved out tests for expando cleanup into a separate test.

* Fixed implied global variable 'pass' in effects.js that was causing
  "TypeError: boolean is not a function" in *UNRELATED* dimensions.js that
  uses a global variable "pass = function () {};" ...

* Removed spurious calls to _removeData. The new test exposed various failures
  e.g. where div[0] isn't being assigned any data anyway.
  (queue.js and attributes.js toggleClass).

* Removed spurious clean up at the bottom of test() functions that are
  already covered by the teardown (calling QUnit.reset or removeClass to
  supposedly undo any changes).

* Documented the parentheses-less magic line in toggleClass. It appeared that
  it would always keep the current class name if there was any (since the
  assignment started with "this.className || ...".

  Adding parentheses + spacing is 8 bytes (though only 1 in gzip apparently).
  Only added the comment for now, though I prefer clarity with logical
  operators, I'd rather not face the yayMinPD[1] in this test-related commit.

* Updated QUnit urlConfig to the new format (raw string is deprecated).

* Clean up odd htmlentities in test titles, QUnit escapes this.
  (^\s+test\(.*)(&gt\;) → $1>
  (^\s+test\(.*)(&lt\;) → $1<

[1] jQuery MinJsGz Release Police Department (do the same, download less)
---
 src/attributes.js         |   6 +-
 test/data/testinit.js     |  46 ------------
 test/data/testrunner.js   | 147 +++++++++++++++++++++++++++++++++++++-
 test/index.html           |  50 +++++++------
 test/unit/attributes.js   |  14 ++--
 test/unit/data.js         | 124 +++++++++++++-------------------
 test/unit/dimensions.js   |  16 +++--
 test/unit/effects.js      | 105 +++++++++++++--------------
 test/unit/manipulation.js |  83 ++++++++++++---------
 test/unit/queue.js        |   1 -
 test/unit/traversing.js   |   2 +-
 11 files changed, 345 insertions(+), 249 deletions(-)

diff --git a/src/attributes.js b/src/attributes.js
index 1bd3fac24..52098ae34 100644
--- a/src/attributes.js
+++ b/src/attributes.js
@@ -127,13 +127,17 @@ jQuery.fn.extend({
 					self[ state ? "addClass" : "removeClass" ]( className );
 				}
 
+			// Toggle whole class name
 			} else if ( type === "undefined" || type === "boolean" ) {
 				if ( this.className ) {
 					// store className if set
 					jQuery._data( this, "__className__", this.className );
 				}
 
-				// toggle whole className
+				// If the element has a class name or if we're passed "false",
+				// then remove the whole classname (if there was one, the above saved it).
+				// Otherwise bring back whatever was previously saved (if anything),
+				// falling back to the empty string if nothing was stored.
 				this.className = this.className || value === false ? "" : jQuery._data( this, "__className__" ) || "";
 			}
 		});
diff --git a/test/data/testinit.js b/test/data/testinit.js
index 6f9fd46e6..3bbee9658 100644
--- a/test/data/testinit.js
+++ b/test/data/testinit.js
@@ -145,52 +145,6 @@ function url( value ) {
 }
 
 (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 ) {
-			equal( cacheLength, oldCacheLength, "No unit tests leak memory in jQuery.cache" );
-			oldCacheLength = cacheLength;
-		}
-		if ( fragmentsLength !== oldFragmentsLength ) {
-			equal( fragmentsLength, oldFragmentsLength, "No unit tests leak memory in jQuery.fragments" );
-			oldFragmentsLength = fragmentsLength;
-		}
-		if ( jQuery.timers && jQuery.timers.length !== oldTimersLength ) {
-			equal( jQuery.timers.length, oldTimersLength, "No timers are still running" );
-			oldTimersLength = jQuery.timers.length;
-		}
-		if ( jQuery.active !== undefined && jQuery.active !== oldActive ) {
-			equal( jQuery.active, 0, "No AJAX requests are still active" );
-			oldActive = jQuery.active;
-		}
-	};
 
 	this.testIframe = function( fileName, name, fn ) {
 
diff --git a/test/data/testrunner.js b/test/data/testrunner.js
index 9b77ac39f..bc5cbf005 100644
--- a/test/data/testrunner.js
+++ b/test/data/testrunner.js
@@ -10,6 +10,7 @@ var Sizzle = Sizzle || jQuery.find;
 // Allow subprojects to test against their own fixtures
 var qunitModule = QUnit.module,
 	qunitTest = QUnit.test;
+
 function testSubproject( label, url, risTests ) {
 	var sub, fixture, fixtureHTML,
 		fixtureReplaced = false;
@@ -131,10 +132,152 @@ function testSubproject( label, url, risTests ) {
  * QUnit hooks
  */
 (function() {
-	// jQuery-specific QUnit.reset
-	var reset = QUnit.reset,
+	// 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,
+
+		expectedDataKeys = {},
+
+		reset = QUnit.reset,
 		ajaxSettings = jQuery.ajaxSettings;
 
+	function keys(o) {
+		var ret, key;
+		if ( Object.keys ) {
+			ret = Object.keys( o );
+		} else {
+			ret = [];
+			for ( key in o ) {
+				ret.push( key );
+			}
+		}
+		ret.sort();
+		return ret;
+	}
+
+	/**
+	 * @param {jQuery|HTMLElement|Object|Array} elems Target (or array of targets) for jQuery.data.
+	 * @param {string} key
+	 */
+	QUnit.expectJqData = function( elems, key ) {
+		var i, elem, expando;
+		QUnit.current_testEnvironment.checkJqData = true;
+
+		if ( elems.jquery && elems.toArray ) {
+			elems = elems.toArray();
+		}
+		if ( !jQuery.isArray( elems ) ) {
+			elems = [ elems ];
+		}
+
+		for ( i = 0; i < elems.length; i++ ) {
+			elem = elems[i];
+
+			// jQuery.data only stores data for nodes in jQuery.cache,
+			// for other data targets the data is stored in the object itself,
+			// in that case we can't test that target for memory leaks.
+			// But we don't have to since in that case the data will/must will
+			// be available as long as the object is not garbage collected by
+			// the js engine, and when it is, the data will be removed with it.
+			if ( !elem.nodeType ) {
+				// Fixes false positives for dataTests(window), dataTests({}).
+				continue;
+			}
+
+			expando = elem[ jQuery.expando ];
+
+			if ( expando === undefined ) {
+				// In this case the element exists fine, but
+				// jQuery.data (or internal data) was never (in)directly
+				// called.
+				// Since this method was called it means some data was
+				// expected to be found, but since there is nothing, fail early
+				// (instead of in teardown).
+				notStrictEqual( expando, undefined, 'Target for expectJqData must have an expando, for else there can be no data to expect.' );
+			} else {
+				if ( expectedDataKeys[expando] ) {
+					expectedDataKeys[expando].push( key );
+				} else {
+					expectedDataKeys[expando] = [ key ];
+				}
+			}
+		}
+	};
+	QUnit.config.urlConfig.push( {
+		id: 'jqdata',
+		label: 'Always check jQuery.data',
+		tooltip: 'Trigger "QUnit.expectJqData" detection for all tests instead of just the ones that call it'
+	} );
+
+	/**
+	 * 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,
+			expectedKeys, actualKeys,
+			fragmentsLength = 0,
+			cacheLength = 0;
+
+		// Only look for jQuery data problems if this test actually
+		// provided some information to compare against.
+		if ( QUnit.urlParams.jqdata || this.checkJqData ) {
+			for ( i in jQuery.cache ) {
+				expectedKeys = expectedDataKeys[i];
+				actualKeys = jQuery.cache[i] ? keys( jQuery.cache[i] ) : jQuery.cache[i];
+				if ( !QUnit.equiv( expectedKeys, actualKeys ) ) {
+					deepEqual( actualKeys, expectedKeys, 'Expected keys exist in jQuery.cache' );
+				}
+				delete jQuery.cache[i];
+				delete expectedDataKeys[i];
+			}
+			// In case it was removed from cache before (or never there in the first place)
+			for ( i in expectedDataKeys ) {
+				deepEqual( expectedDataKeys[i], undefined, 'No unexpected keys were left in jQuery.cache (#' + i + ')' );
+				delete expectedDataKeys[i];
+			}
+		}
+
+		// Reset data register
+		expectedDataKeys = {};
+
+		// 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 ) {
+			equal( cacheLength, oldCacheLength, "No unit tests leak memory in jQuery.cache" );
+			oldCacheLength = cacheLength;
+		}
+		if ( fragmentsLength !== oldFragmentsLength ) {
+			equal( fragmentsLength, oldFragmentsLength, "No unit tests leak memory in jQuery.fragments" );
+			oldFragmentsLength = fragmentsLength;
+		}
+		if ( jQuery.timers && jQuery.timers.length !== oldTimersLength ) {
+			equal( jQuery.timers.length, oldTimersLength, "No timers are still running" );
+			oldTimersLength = jQuery.timers.length;
+		}
+		if ( jQuery.active !== undefined && jQuery.active !== oldActive ) {
+			equal( jQuery.active, 0, "No AJAX requests are still active" );
+			oldActive = jQuery.active;
+		}
+	};
+
+	// jQuery-specific QUnit.reset
 	QUnit.reset = function() {
 
 		// Ensure jQuery events and data on the fixture are properly removed
diff --git a/test/index.html b/test/index.html
index b8e6ff799..53dec94c0 100644
--- a/test/index.html
+++ b/test/index.html
@@ -17,12 +17,20 @@
 		(function() {
 			var src = "../dist/jquery.js";
 
-			QUnit.config.urlConfig.push( "min" );
+			QUnit.config.urlConfig.push( {
+				id: "min",
+				label: "Load minified",
+				tooltip: "Load the minified version of the jQuery build"
+			} );
 			if ( QUnit.urlParams.min ) {
 				src = "../dist/jquery.min.js";
 			}
 
-			QUnit.config.urlConfig.push( "noqsa" );
+			QUnit.config.urlConfig.push( {
+				id: "noqsa",
+				label: "Disable querySelectorAll",
+				tooltip: "Disable the native document.querySelectorAll"
+			} );
 			if ( QUnit.urlParams.noqsa ) {
 				document.querySelectorAll = null;
 			}
@@ -281,34 +289,34 @@ Z</textarea>
 			</em>
 			<span id="siblingspan"></span>
 		</div>
-	</div>
-	</dl>
-	<div id="fx-test-group" style="position:absolute;width:1px;height:1px;overflow:hidden;">
-		<div id="fx-queue" name="test">
-			<div id="fadein" class='chain test' name='div'>fadeIn<div>fadeIn</div></div>
-			<div id="fadeout" class='chain test out'>fadeOut<div>fadeOut</div></div>
+		<div id="fx-test-group" style="position:absolute;width:1px;height:1px;overflow:hidden;">
+			<div id="fx-queue" name="test">
+				<div id="fadein" class='chain test' name='div'>fadeIn<div>fadeIn</div></div>
+				<div id="fadeout" class='chain test out'>fadeOut<div>fadeOut</div></div>
 
-			<div id="show" class='chain test'>show<div>show</div></div>
-			<div id="hide" class='chain test out'>hide<div>hide</div></div>
+				<div id="show" class='chain test'>show<div>show</div></div>
+				<div id="hide" class='chain test out'>hide<div>hide</div></div>
 
-			<div id="togglein" class='chain test'>togglein<div>togglein</div></div>
-			<div id="toggleout" class='chain test out'>toggleout<div>toggleout</div></div>
+				<div id="togglein" class='chain test'>togglein<div>togglein</div></div>
+				<div id="toggleout" class='chain test out'>toggleout<div>toggleout</div></div>
 
 
-			<div id="slideup" class='chain test'>slideUp<div>slideUp</div></div>
-			<div id="slidedown" class='chain test out'>slideDown<div>slideDown</div></div>
+				<div id="slideup" class='chain test'>slideUp<div>slideUp</div></div>
+				<div id="slidedown" class='chain test out'>slideDown<div>slideDown</div></div>
 
-			<div id="slidetogglein" class='chain test'>slideToggleIn<div>slideToggleIn</div></div>
-			<div id="slidetoggleout" class='chain test out'>slideToggleOut<div>slideToggleOut</div></div>
+				<div id="slidetogglein" class='chain test'>slideToggleIn<div>slideToggleIn</div></div>
+				<div id="slidetoggleout" class='chain test out'>slideToggleOut<div>slideToggleOut</div></div>
 
-			<div id="fadetogglein" class='chain test'>fadeToggleIn<div>fadeToggleIn</div></div>
-			<div id="fadetoggleout" class='chain test out'>fadeToggleOut<div>fadeToggleOut</div></div>
+				<div id="fadetogglein" class='chain test'>fadeToggleIn<div>fadeToggleIn</div></div>
+				<div id="fadetoggleout" class='chain test out'>fadeToggleOut<div>fadeToggleOut</div></div>
 
-			<div id="fadeto" class='chain test'>fadeTo<div>fadeTo</div></div>
-		</div>
+				<div id="fadeto" class='chain test'>fadeTo<div>fadeTo</div></div>
+			</div>
 
-		<div id="fx-tests"></div>
+			<div id="fx-tests"></div>
+		</div>
 	</div>
+	</dl>
 	<map name="imgmap" id="imgmap">
 		<area shape="rect" coords="0,0,200,50">
 	</map>
diff --git a/test/unit/attributes.js b/test/unit/attributes.js
index 0e51ee137..59b34a7a4 100644
--- a/test/unit/attributes.js
+++ b/test/unit/attributes.js
@@ -1231,7 +1231,7 @@ var testToggleClass = function(valueObj) {
 
 	// multiple class names
 	e.addClass("testA testB");
-	ok( (e.is(".testA.testB")), "Assert 2 different classes present" );
+	ok( e.is(".testA.testB"), "Assert 2 different classes present" );
 	e.toggleClass( valueObj("testB testC") );
 	ok( (e.is(".testA.testC") && !e.is(".testB")), "Assert 1 class added, 1 class removed, and 1 class kept" );
 	e.toggleClass( valueObj("testA testC") );
@@ -1258,7 +1258,7 @@ var testToggleClass = function(valueObj) {
 
 	// Cleanup
 	e.removeClass("testD");
-	jQuery._removeData( e[ 0 ], "__className__" );
+	QUnit.expectJqData( e[ 0 ], "__className__" );
 };
 
 test( "toggleClass(String|boolean|undefined[, boolean])", function() {
@@ -1269,10 +1269,12 @@ test( "toggleClass(Function[, boolean])", function() {
 	testToggleClass( functionReturningObj );
 });
 
-test( "toggleClass(Fucntion[, boolean]) with incoming value", function() {
+test( "toggleClass(Function[, boolean]) with incoming value", function() {
 	expect( 14 );
 
-	var e = jQuery("#firstp"), old = e.attr("class") || "";
+	var e = jQuery("#firstp"),
+		old = e.attr("class") || "";
+
 	ok( !e.is(".test"), "Assert class not present" );
 
 	e.toggleClass(function( i, val ) {
@@ -1316,10 +1318,6 @@ test( "toggleClass(Fucntion[, boolean]) with incoming value", function() {
 		return "test";
 	}, false );
 	ok( !e.is(".test"), "Assert class not present" );
-
-	// Cleanup
-	e.removeClass("test");
-	jQuery._removeData( e[ 0 ], "__className__" );
 });
 
 test( "addClass, removeClass, hasClass", function() {
diff --git a/test/unit/data.js b/test/unit/data.js
index 8e4da305e..378750108 100644
--- a/test/unit/data.js
+++ b/test/unit/data.js
@@ -7,17 +7,6 @@ test("expando", function(){
 });
 
 function dataTests (elem) {
-	// expect(31)
-
-	function getCacheLength() {
-		var cacheLength = 0;
-		for (var i in jQuery.cache) {
-			++cacheLength;
-		}
-
-		return cacheLength;
-	}
-
 	var oldCacheLength, dataObj, internalDataObj, expected, actual;
 
 	equal( jQuery.data(elem, "foo"), undefined, "No data exists initially" );
@@ -66,51 +55,12 @@ function dataTests (elem) {
 	jQuery.removeData(elem);
 	strictEqual( jQuery._data(elem), internalDataObj, "jQuery.removeData does not remove internal data if it exists" );
 
-	jQuery._removeData(elem);
-
-	strictEqual( jQuery._data(elem, jQuery.expando), undefined, "jQuery.removeData on internal data works" );
-	strictEqual( jQuery.hasData(elem), false, "jQuery.hasData agrees all data has been removed from object" );
-
-	jQuery._data(elem, "foo", "foo2");
-	strictEqual( jQuery.hasData(elem), true, "jQuery.hasData shows data exists even if it is only internal data" );
-
-	jQuery.data(elem, "foo", "foo1");
-	equal( jQuery._data(elem, "foo"), "foo2", "Setting user data does not override internal data" );
-
-	// delete the last private data key so we can test removing public data
-	// will destroy the cache
-	jQuery._removeData( elem, "foo" );
-
-	if (elem.nodeType) {
-		oldCacheLength = getCacheLength();
-		jQuery.removeData(elem, "foo");
-
-		equal( getCacheLength(), oldCacheLength - 1, "Removing the last item in the data object destroys it" );
-	}
-	else {
-		jQuery.removeData(elem, "foo");
-
-
-		if (jQuery.support.deleteExpando) {
-			expected = false;
-			actual = jQuery.expando in elem;
-		}
-		else {
-			expected = null;
-			actual = elem[ jQuery.expando ];
-		}
-
-		equal( actual, expected, "Removing the last item in the data object destroys it" );
-	}
-
 	jQuery.data(elem, "foo", "foo1");
 	jQuery._data(elem, "foo", "foo2");
 
 	equal( jQuery.data(elem, "foo"), "foo1", "(sanity check) Ensure data is set in user data object" );
 	equal( jQuery._data(elem, "foo"), "foo2", "(sanity check) Ensure data is set in internal data object" );
 
-	jQuery._removeData(elem, "foo");
-
 	strictEqual( jQuery._data(elem, jQuery.expando), undefined, "Removing the last item in internal data destroys the internal data object" );
 
 	jQuery._data(elem, "foo", "foo2");
@@ -118,44 +68,68 @@ function dataTests (elem) {
 
 	jQuery.removeData(elem, "foo");
 	equal( jQuery._data(elem, "foo"), "foo2", "(sanity check) jQuery.removeData for user data does not remove internal data" );
-
-	if ( elem.nodeType ) {
-		oldCacheLength = getCacheLength();
-		jQuery._removeData(elem, "foo");
-		equal( getCacheLength(), oldCacheLength - 1, "Removing the last item in the internal data object also destroys the user data object when it is empty" );
-	}
-	else {
-		jQuery._removeData(elem, "foo");
-
-		if (jQuery.support.deleteExpando) {
-			expected = false;
-			actual = jQuery.expando in elem;
-		}
-		else {
-			expected = null;
-			actual = elem[ jQuery.expando ];
-		}
-
-		equal( actual, expected, "Removing the last item in the internal data object also destroys the user data object when it is empty" );
-	}
 }
 
-test("jQuery.data", function() {
-	expect(124);
-
+test("jQuery.data(div)", 25, function() {
 	var div = document.createElement("div");
 
 	dataTests(div);
+
+	// We stored one key in the private data
+	// assert that nothing else was put in there, and that that
+	// one stayed there.
+	QUnit.expectJqData(div, "foo");
+});
+
+test("jQuery.data({})", 25, function() {
 	dataTests({});
+});
 
+test("jQuery.data(window)", 25, function() {
 	// remove bound handlers from window object to stop potential false positives caused by fix for #5280 in
 	// transports/xhr.js
 	jQuery(window).unbind("unload");
 
 	dataTests(window);
+});
+
+test("jQuery.data(document)", 25, function() {
 	dataTests(document);
 
-	// clean up unattached element
+	QUnit.expectJqData(document, "foo");
+});
+
+test("Expando cleanup", 4, function() {
+	var expected, actual,
+		div = document.createElement("div");
+
+	function assertExpandoAbsent(message) {
+		if (jQuery.support.deleteExpando) {
+			expected = false;
+			actual = jQuery.expando in div;
+		} else {
+			expected = null;
+			actual = div[ jQuery.expando ];
+		}
+		equal( actual, expected, message );
+	}
+
+	assertExpandoAbsent("There is no expando on new elements");
+
+	jQuery.data(div, "foo", 100);
+	jQuery.data(div, "bar", 200);
+
+	ok(jQuery.expando in div, "There is an expando on the element after using $.data()");
+
+	jQuery.removeData(div, "foo");
+
+	ok(jQuery.expando in div, "There is still an expando on the element after removing (some) of the data");
+
+	jQuery.removeData(div, "bar");
+
+	assertExpandoAbsent("Removing the last item in the data store deletes the expando");
+
+	// Clean up unattached element
 	jQuery(div).remove();
 });
 
@@ -186,7 +160,7 @@ test(".data()", function() {
 
 	var dataObj = div.data();
 
-	deepEqual( dataObj, {test: "success"}, "data() get the entire data object" );
+	deepEqual( dataObj, {test: "success"}, "data() returns entire data object with expected properties" );
 	strictEqual( div.data("foo"), undefined, "Make sure that missing result is still undefined" );
 
 	var nodiv = jQuery("#unfound");
diff --git a/test/unit/dimensions.js b/test/unit/dimensions.js
index f0e7974c9..680b08320 100644
--- a/test/unit/dimensions.js
+++ b/test/unit/dimensions.js
@@ -7,7 +7,9 @@ var pass = function( val ) {
 };
 
 var fn = function( val ) {
-	return function(){ return val; };
+	return function() {
+		return val;
+	};
 };
 
 /*
@@ -50,7 +52,7 @@ var testWidth = function( val ) {
 
 	equal( jQuery(window).width(), document.documentElement.clientWidth, "Window width is equal to width reported by window/document." );
 
-	jQuery._removeData( $div[0], "olddisplay" );
+	QUnit.expectJqData( $div[0], "olddisplay" );
 };
 
 test("width()", function() {
@@ -101,7 +103,7 @@ var testHeight = function( val ) {
 
 	equal( jQuery(window).height(), document.documentElement.clientHeight, "Window width is equal to width reported by window/document." );
 
-	jQuery._removeData( $div[0], "olddisplay" );
+	QUnit.expectJqData( $div[0], "olddisplay" );
 };
 
 test("height()", function() {
@@ -156,7 +158,7 @@ test("innerWidth()", function() {
 	equal( div.innerWidth(), 0, "Make sure that disconnected nodes are handled." );
 
 	div.remove();
-	jQuery._removeData( $div[0], "olddisplay" );
+	QUnit.expectJqData( $div[0], "olddisplay" );
 });
 
 test("innerHeight()", function() {
@@ -191,7 +193,7 @@ test("innerHeight()", function() {
 	equal( div.innerHeight(), 0, "Make sure that disconnected nodes are handled." );
 
 	div.remove();
-	jQuery._removeData( $div[0], "olddisplay" );
+	QUnit.expectJqData( $div[0], "olddisplay" );
 });
 
 test("outerWidth()", function() {
@@ -229,7 +231,7 @@ test("outerWidth()", function() {
 	equal( div.outerWidth(), 0, "Make sure that disconnected nodes are handled." );
 
 	div.remove();
-	jQuery._removeData( $div[0], "olddisplay" );
+	QUnit.expectJqData( $div[0], "olddisplay" );
 });
 
 test("child of a hidden elem (or unconnected node) has accurate inner/outer/Width()/Height()  see #9441 #9300", function() {
@@ -375,7 +377,7 @@ test("outerHeight()", function() {
 	equal( div.outerHeight(), 0, "Make sure that disconnected nodes are handled." );
 
 	div.remove();
-	jQuery._removeData( $div[0], "olddisplay" );
+	QUnit.expectJqData( $div[0], "olddisplay" );
 });
 
 test("passing undefined is a setter #5571", function() {
diff --git a/test/unit/effects.js b/test/unit/effects.js
index 814f94ad0..ca6f2698e 100644
--- a/test/unit/effects.js
+++ b/test/unit/effects.js
@@ -7,22 +7,27 @@ test("sanity check", function() {
 	ok( jQuery("#dl:visible, #qunit-fixture:visible, #foo:visible").length === 3, "QUnit state is correct for testing effects" );
 });
 
-test("show()", function() {
-	expect(26);
-
-	var hiddendiv = jQuery("div.hidden");
+test("show() basic", 2, function() {
+	var div,
+		hiddendiv = jQuery("div.hidden");
 
 	hiddendiv.hide().show();
 
 	equal( hiddendiv.css("display"), "block", "Make sure a pre-hidden div is visible." );
 
-	var div = jQuery("<div>").hide().appendTo("#qunit-fixture").show();
+	div = jQuery("<div>").hide().appendTo("#qunit-fixture").show();
 
 	equal( div.css("display"), "block", "Make sure pre-hidden divs show" );
 
-	QUnit.reset();
+	// Clean up the detached node
+	div.remove();
 
-	hiddendiv = jQuery("div.hidden");
+	QUnit.expectJqData(hiddendiv, "olddisplay");
+});
+
+test("show()", 27, function () {
+	var div,
+		hiddendiv = jQuery("div.hidden");
 
 	equal(jQuery.css( hiddendiv[0], "display"), "none", "hiddendiv is display: none");
 
@@ -34,14 +39,12 @@ test("show()", function() {
 
 	hiddendiv.css("display","");
 
-	var pass = true;
-	div = jQuery("#qunit-fixture div");
-	div.show().each(function(){
-		if ( this.style.display == "none" ) {
-			pass = false;
-		}
+	var displaysActual = [],
+		displaysExpected = [];
+	div = jQuery("#fx-queue div").slice(0, 4);
+	div.show().each(function() {
+		notEqual(this.style.display, "none", "don't change any <div> with display block");
 	});
-	ok( pass, "Show" );
 
 	var speeds = {
 		"null speed": null,
@@ -50,7 +53,7 @@ test("show()", function() {
 	};
 
 	jQuery.each(speeds, function(name, speed) {
-		pass = true;
+		var pass = true;
 		div.hide().show(speed).each(function() {
 			if ( this.style.display == "none" ) {
 				pass = false;
@@ -60,13 +63,16 @@ test("show()", function() {
 	});
 
 	jQuery.each(speeds, function(name, speed) {
-	pass = true;
-	div.hide().show(speed, function() {
+		var pass = true;
+		div.hide().show(speed, function() {
 			pass = false;
 		});
 		ok( pass, "Show with " + name + " does not call animate callback" );
 	});
 
+	// Tolerate data from show()/hide()
+	QUnit.expectJqData(div, "olddisplay");
+
 	// #show-tests * is set display: none in CSS
 	jQuery("#qunit-fixture").append("<div id='show-tests'><div><p><a href='#'></a></p><code></code><pre></pre><span></span></div><table><thead><tr><th></th></tr></thead><tbody><tr><td></td></tr></tbody></table><ul><li></li></ul></div><table id='test-table'></table>");
 
@@ -143,8 +149,6 @@ test("show(Number) - other displays", function() {
 	jQuery("#show-tests").remove();
 });
 
-
-
 // Supports #7397
 test("Persist correct display value", function() {
 	expect(3);
@@ -174,6 +178,8 @@ test("Persist correct display value", function() {
 			});
 		});
 	});
+
+	QUnit.expectJqData($span, "olddisplay");
 });
 
 test("animate(Hash, Object, Function)", function() {
@@ -996,25 +1002,28 @@ jQuery.fn.saveState = function( hiddenOverflow ) {
 	expect(check.length);
 
 	stop();
-	return this.each(function(){
-		var self = this;
-		self.save = {};
+	return this.each(function() {
+		var el = this;
+		el.save = {};
 		jQuery.each(check, function( i, c ) {
-			self.save[ c ] = c === "overflow" && hiddenOverflow ? "hidden" : self.style[ c ] || jQuery.css( self, c );
+			el.save[ c ] = c === "overflow" && hiddenOverflow ? "hidden" : el.style[ c ] || jQuery.css( el, c );
 		});
 	});
 };
 
-/** @expose */
+/**
+ * @expose
+ * @context {HTMLElement}
+ */
 jQuery.checkState = function() {
-	var self = this;
-	jQuery.each(this.save, function( c, v ) {
-		var cur = self.style[ c ] || jQuery.css( self, c );
+	var el = this;
+	jQuery.each(el.save, function( c, v ) {
+		var cur = el.style[ c ] || jQuery.css( el, c );
 		equal( cur, v, "Make sure that " + c + " is reset (Old: " + v + " Cur: " + cur + ")");
 	});
 
-	// manually clean data on modified element
-	jQuery._removeData( this, "olddisplay" );
+	// Clean up
+	jQuery(el).remove();
 
 	start();
 };
@@ -1147,8 +1156,8 @@ function( method, defProp ) {
 						$elem[ method ](animTime, function() {
 							equal( defProp( $elem ), startVal, "After doing .stop() halfway through show, check that state has been saved for returning to original property value." );
 
-							// Remove olddisplay data from .hide() call
-							jQuery._removeData( this, "olddisplay" );
+							// Tolerate olddisplay data from .hide() call
+							QUnit.expectJqData( this, "olddisplay" );
 							start();
 						});
 					}, animTime / 2);
@@ -1478,28 +1487,20 @@ test( "animate should set display for disconnected nodes", function() {
 			show: [ 1 ],
 			animate: [{ width: "show" }]
 		},
-		elems = [
+		$divTest = jQuery("<div>test</div>"),
+		// parentNode = null
+		$divEmpty = jQuery("<div/>"),
+		$divNone = jQuery("<div style='display: none;'/>"),
+		$divInline = jQuery("<div style='display: inline;'/>");
 
-			// parentNode = document fragment
-			jQuery("<div>test</div>"),
-
-			// parentNode = null
-			jQuery("<div/>"),
+	strictEqual( $divTest.show()[ 0 ].style.display, "block", "set display with show() for element with parentNode = document fragment" );
+	strictEqual( $divEmpty.show()[ 0 ].style.display, "block", "set display with show() for element with parentNode = null" );
+	strictEqual( $divNone.show()[ 0 ].style.display, "block", "show() should change display if it already set to none" );
+	strictEqual( $divInline.show()[ 0 ].style.display, "inline", "show() should not change display if it already set" );
 
-			jQuery("<div style='display:inline'/>"),
-
-			jQuery("<div style='display:none'/>")
-		];
-
-	strictEqual( elems[ 0 ].show()[ 0 ].style.display, "block", "set display with show() for element with parentNode = document fragment" );
-	strictEqual( elems[ 1 ].show()[ 0 ].style.display, "block", "set display with show() for element with parentNode = null" );
-	strictEqual( elems[ 2 ].show()[ 0 ].style.display, "inline", "show() should not change display if it already set" );
-	strictEqual( elems[ 3 ].show()[ 0 ].style.display, "block", "show() should change display if it already set to none" );
-
-	// cleanup
-	jQuery.each( elems, function() {
-		jQuery._removeData( this[ 0 ], "olddisplay" );
-	});
+	QUnit.expectJqData( $divTest[ 0 ], "olddisplay" );
+	QUnit.expectJqData( $divEmpty[ 0 ], "olddisplay" );
+	QUnit.expectJqData( $divNone[ 0 ], "olddisplay" );
 
 	stop();
 	jQuery.each( methods, function( name, opt ) {
@@ -1515,7 +1516,7 @@ test( "animate should set display for disconnected nodes", function() {
 			var callback = [function () {
 					strictEqual( this.style.display, "block", "set display to block with " + name );
 
-					jQuery._removeData( this, "olddisplay" );
+					QUnit.expectJqData( this, "olddisplay" );
 
 					if ( ++i === 14 ) {
 						start();
diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js
index 06e8b3050..714e8f4bc 100644
--- a/test/unit/manipulation.js
+++ b/test/unit/manipulation.js
@@ -80,8 +80,6 @@ test("text(Function) with incoming value", function() {
 	});
 
 	equal( jQuery("#sap").text(), "foobar", "Check for merged text of more then one element." );
-
-	QUnit.reset();
 });
 
 var testWrap = function(val) {
@@ -461,7 +459,7 @@ var testAppend = function(valueObj) {
 	QUnit.reset();
 };
 
-test("append(String|Element|Array&lt;Element&gt;|jQuery)", function() {
+test("append(String|Element|Array<Element>|jQuery)", function() {
 	testAppend(manipulationBareObj);
 });
 
@@ -718,7 +716,7 @@ test("append(xml)", function() {
 
 });
 
-test("appendTo(String|Element|Array&lt;Element&gt;|jQuery)", function() {
+test("appendTo(String|Element|Array<Element>|jQuery)", function() {
 	expect( 16 + ( jQuery.getScript ? 1 : 0 ) );
 
 	var defaultText = "Try them out:";
@@ -831,7 +829,7 @@ var testPrepend = function(val) {
 	equal( jQuery("#sap").text(), expected, "Check for prepending of array of jQuery objects" );
 };
 
-test("prepend(String|Element|Array&lt;Element&gt;|jQuery)", function() {
+test("prepend(String|Element|Array<Element&gt;|jQuery)", function() {
 	testPrepend(manipulationBareObj);
 });
 
@@ -890,7 +888,7 @@ test("prepend(Function) with incoming value", function() {
 	equal( jQuery("#sap").text(), expected, "Check for prepending of jQuery object" );
 });
 
-test("prependTo(String|Element|Array&lt;Element&gt;|jQuery)", function() {
+test("prependTo(String|Element|Array<Element&gt;|jQuery)", function() {
 	expect(6);
 	var defaultText = "Try them out:";
 	jQuery("<b>buga</b>").prependTo("#first");
@@ -950,7 +948,7 @@ var testBefore = function(val) {
 	equal( set.length, 1, "Insert the element before the disconnected node. should be a no-op" );
 };
 
-test("before(String|Element|Array&lt;Element&gt;|jQuery)", function() {
+test("before(String|Element|Array<Element&gt;|jQuery)", function() {
 	testBefore(manipulationBareObj);
 });
 
@@ -990,7 +988,7 @@ test("before and after on disconnected node (#10517)", function() {
 	equal( jQuery("#en").text(), expectedAfter, "Insert String after with disconnected node first" );
 });
 
-test("insertBefore(String|Element|Array&lt;Element&gt;|jQuery)", function() {
+test("insertBefore(String|Element|Array<Element>|jQuery)", function() {
 	expect(4);
 	var expected = "This is a normal link: bugaYahoo";
 	jQuery("<b>buga</b>").insertBefore("#yahoo");
@@ -1043,7 +1041,7 @@ var testAfter = function(val) {
 	equal( set.length, 1, "Insert the element after the disconnected node should be a no-op" );
 };
 
-test("after(String|Element|Array&lt;Element&gt;|jQuery)", function() {
+test("after(String|Element|Array<Element>|jQuery)", function() {
 	testAfter(manipulationBareObj);
 });
 
@@ -1051,7 +1049,7 @@ test("after(Function)", function() {
 	testAfter(manipulationFunctionReturningObj);
 });
 
-test("insertAfter(String|Element|Array&lt;Element&gt;|jQuery)", function() {
+test("insertAfter(String|Element|Array<Element>|jQuery)", function() {
 	expect(4);
 	var expected = "This is a normal link: Yahoobuga";
 	jQuery("<b>buga</b>").insertAfter("#yahoo");
@@ -1161,7 +1159,7 @@ var testReplaceWith = function(val) {
 	equal( jQuery("#qunit-fixture").find("div[id=replaceWith]").length, 1, "Make sure only one div exists." );
 };
 
-test("replaceWith(String|Element|Array&lt;Element&gt;|jQuery)", function() {
+test("replaceWith(String|Element|Array<Element>|jQuery)", function() {
 	testReplaceWith(manipulationBareObj);
 });
 
@@ -1224,7 +1222,7 @@ test("replaceWith(string) for collection with disconnected element", function(){
 	equal(newSet.filter("span").length, 1, "ensure the new element is in the new set");
 });
 
-test("replaceAll(String|Element|Array&lt;Element&gt;|jQuery)", function() {
+test("replaceAll(String|Element|Array<Element>|jQuery)", function() {
 	expect(10);
 	jQuery("<b id='replace'>buga</b>").replaceAll("#yahoo");
 	ok( jQuery("#replace")[0], "Replace element with string" );
@@ -1587,17 +1585,18 @@ test("html(Function)", function() {
 });
 
 test("html(Function) with incoming value", function() {
-	expect(20);
+	expect(18);
 
-	var div = jQuery("#qunit-fixture > div"), old = div.map(function(){ return jQuery(this).html(); });
+	var els = jQuery("#foo > p"),
+		actualhtml = els.map(function() { return jQuery(this).html(); });
 
-	div.html(function(i, val) {
-		equal( val, old[i], "Make sure the incoming value is correct." );
+	els.html(function(i, val) {
+		equal( val, actualhtml[i], "Make sure the incoming value is correct." );
 		return "<b>test</b>";
 	});
 
 	var pass = true;
-	div.each(function(){
+	els.each(function(){
 		if ( this.childNodes.length !== 1 ) {
 			pass = false;
 		}
@@ -1607,10 +1606,10 @@ test("html(Function) with incoming value", function() {
 	QUnit.reset();
 	// using contents will get comments regular, text, and comment nodes
 	var j = jQuery("#nonnodes").contents();
-	old = j.map(function(){ return jQuery(this).html(); });
+	actualhtml = j.map(function(){ return jQuery(this).html(); });
 
 	j.html(function(i, val) {
-		equal( val, old[i], "Make sure the incoming value is correct." );
+		equal( val, actualhtml[i], "Make sure the incoming value is correct." );
 		return "<b>bold</b>";
 	});
 
@@ -1652,10 +1651,7 @@ test("html(Function) with incoming value", function() {
 });
 
 var testRemove = function(method) {
-	expect(9);
-
-	var cleanUp, count,
-			first = jQuery("#ap").children(":first");
+	var first = jQuery("#ap").children(":first");
 
 	first.data("foo", "bar");
 
@@ -1663,7 +1659,7 @@ var testRemove = function(method) {
 	ok( jQuery("#ap").text().length > 10, "Check text is not removed" );
 	equal( jQuery("#ap").children().length, 0, "Check remove" );
 
-	equal( first.data("foo"), method == "remove" ? null : "bar" );
+	equal( first.data("foo"), method == "remove" ? null : "bar", "first data" );
 
 	QUnit.reset();
 	jQuery("#ap").children()[method]("a");
@@ -1683,29 +1679,46 @@ var testRemove = function(method) {
 	if (method === "detach") {
 		first.remove();
 	}
+};
 
-	QUnit.reset();
+test("remove()", 8, function() {
+	testRemove("remove");
+});
 
-	count = 0;
+test("remove() event cleaning ", 1, function() {
+	var count, first, cleanUp;
 
+	count = 0;
 	first = jQuery("#ap").children(":first");
+	cleanUp = first.click(function() {
+		count++;
+	}).remove().appendTo("#qunit-fixture").click();
 
-	cleanUp = first.click(function() { count++; })[method]().appendTo("#qunit-fixture").click();
-
-	equal( method == "remove" ? 0 : 1, count );
+	strictEqual( 0, count, "Event handler has been removed" );
 
-	// manually clean up detached elements
+	// Clean up detached data
 	cleanUp.remove();
-};
-
-test("remove()", function() {
-	testRemove("remove");
 });
 
-test("detach()", function() {
+test("detach()", 8, function() {
 	testRemove("detach");
 });
 
+test("detach() event cleaning ", 1, function() {
+	var count, first, cleanUp;
+
+	count = 0;
+	first = jQuery("#ap").children(":first");
+	cleanUp = first.click(function() {
+		count++;
+	}).detach().appendTo("#qunit-fixture").click();
+
+	strictEqual( 1, count, "Event handler has not been removed" );
+
+	// Clean up detached data
+	cleanUp.remove();
+});
+
 test("empty()", function() {
 	expect(3);
 	equal( jQuery("#ap").children().empty().text().length, 0, "Check text is removed" );
diff --git a/test/unit/queue.js b/test/unit/queue.js
index 692975469..0b17f6e1e 100644
--- a/test/unit/queue.js
+++ b/test/unit/queue.js
@@ -119,7 +119,6 @@ test("callbacks keep their place in the queue", function() {
 
 	div.promise("fx").done(function() {
 		equal(counter, 4, "Deferreds resolved");
-		jQuery._removeData( div[0], "olddisplay" );
 		start();
 	});
 });
diff --git a/test/unit/traversing.js b/test/unit/traversing.js
index 96b044977..0facd2e6e 100644
--- a/test/unit/traversing.js
+++ b/test/unit/traversing.js
@@ -8,7 +8,7 @@ test("find(String)", function() {
 	var j = jQuery("#nonnodes").contents();
 	equal( j.find("div").length, 0, "Check node,textnode,comment to find zero divs" );
 
-	deepEqual( jQuery("#qunit-fixture").find("> div").get(), q("foo", "moretests", "tabindex-tests", "liveHandlerOrder", "siblingTest"), "find child elements" );
+	deepEqual( jQuery("#qunit-fixture").find("> div").get(), q("foo", "moretests", "tabindex-tests", "liveHandlerOrder", "siblingTest", "fx-test-group"), "find child elements" );
 	deepEqual( jQuery("#qunit-fixture").find("> #foo, > #moretests").get(), q("foo", "moretests"), "find child elements" );
 	deepEqual( jQuery("#qunit-fixture").find("> #foo > p").get(), q("sndp", "en", "sap"), "find child elements" );
 });
-- 
2.39.5