HTML Reporter: Decouple from sessionStorage reordering logic

Closes gh-1042
Fixes gh-964

reporter/html.js

@@ -1,5 +1,5 @@
 import QUnit from "../src/core";
-import { window, sessionStorage, navigator } from "../src/globals";
+import { window, navigator } from "../src/globals";
 
 // Escape text for attribute or text content.
 export function escapeText( s ) {
@@ -45,18 +45,6 @@ var config = QUnit.config,
 	hasOwn = Object.prototype.hasOwnProperty,
 	unfilteredUrl = setUrl( { filter: undefined, module: undefined,
 		moduleId: undefined, testId: undefined } ),
-	defined = {
-		sessionStorage: ( function() {
-			var x = "qunit-test-string";
-			try {
-				sessionStorage.setItem( x, x );
-				sessionStorage.removeItem( x );
-				return true;
-			} catch ( e ) {
-				return false;
-			}
-		}() )
-	},
 	modulesList = [];
 
 function addEvent( elem, type, fn ) {
@@ -609,8 +597,7 @@ QUnit.begin( function( details ) {
 } );
 
 QUnit.done( function( details ) {
-	var i, key,
-		banner = id( "qunit-banner" ),
+	var banner = id( "qunit-banner" ),
 		tests = id( "qunit-tests" ),
 		html = [
 			"Tests completed in ",
@@ -643,16 +630,6 @@ QUnit.done( function( details ) {
 		].join( " " );
 	}
 
-	// Clear own sessionStorage items if all tests passed
-	if ( config.reorder && defined.sessionStorage && details.failed === 0 ) {
-		for ( i = 0; i < sessionStorage.length; i++ ) {
-			key = sessionStorage.key( i++ );
-			if ( key.indexOf( "qunit-test-" ) === 0 ) {
-				sessionStorage.removeItem( key );
-			}
-		}
-	}
-
 	// Scroll back to top to show results
 	if ( config.scrolltop && window.scrollTo ) {
 		window.scrollTo( 0, 0 );
@@ -685,8 +662,7 @@ QUnit.testStart( function( details ) {
 
 	running = id( "qunit-testresult" );
 	if ( running ) {
-		bad = QUnit.config.reorder && defined.sessionStorage &&
-			+sessionStorage.getItem( "qunit-test-" + details.module + "-" + details.name );
+		bad = QUnit.config.reorder && details.previousFailure;
 
 		running.innerHTML = ( bad ?
 			"Rerunning previously failed test: <br />" :
@@ -803,15 +779,6 @@ QUnit.testDone( function( details ) {
 	good = details.passed;
 	bad = details.failed;
 
-	// Store result when possible
-	if ( config.reorder && defined.sessionStorage ) {
-		if ( bad ) {
-			sessionStorage.setItem( "qunit-test-" + details.module + "-" + details.name, bad );
-		} else {
-			sessionStorage.removeItem( "qunit-test-" + details.module + "-" + details.name );
-		}
-	}
-
 	if ( bad === 0 ) {
 
 		// Collapse the passing tests

src/core.js

@@ -1,4 +1,4 @@
-import { window, setTimeout, console } from "./globals";
+import { window, setTimeout, console, sessionStorage } from "./globals";
 
 import equiv from "./equiv";
 import dump from "./dump";
@@ -268,7 +268,7 @@ export function process( last ) {
 }
 
 function done() {
-	var runtime, passed;
+	var runtime, passed, i, key;
 
 	internalState.autorun = true;
 
@@ -281,6 +281,16 @@ function done() {
 		total: config.stats.all,
 		runtime: runtime
 	} );
+
+	// Clear own sessionStorage items if all tests passed
+	if ( config.reorder && defined.sessionStorage && config.stats.bad === 0 ) {
+		for ( i = 0; i < sessionStorage.length; i++ ) {
+			key = sessionStorage.key( i++ );
+			if ( key.indexOf( "qunit-test-" ) === 0 ) {
+				sessionStorage.removeItem( key );
+			}
+		}
+	}
 }
 
 function setHook( module, hookName ) {

src/test.js

@@ -94,7 +94,8 @@ Test.prototype = {
 		runLoggingCallbacks( "testStart", {
 			name: this.testName,
 			module: module.name,
-			testId: this.testId
+			testId: this.testId,
+			previousFailure: this.previousFailure
 		} );
 
 		if ( !config.pollution ) {
@@ -210,6 +211,8 @@ Test.prototype = {
 
 		var i,
 			module = this.module,
+			moduleName = module.name,
+			testName = this.testName,
 			skipped = !!this.skip,
 			bad = 0;
 
@@ -227,9 +230,19 @@ Test.prototype = {
 		}
 
 		notifyTestsRan( module );
+
+		// Store result when possible
+		if ( config.reorder && defined.sessionStorage ) {
+			if ( bad ) {
+				sessionStorage.setItem( "qunit-test-" + moduleName + "-" + testName, bad );
+			} else {
+				sessionStorage.removeItem( "qunit-test-" + moduleName + "-" + testName );
+			}
+		}
+
 		runLoggingCallbacks( "testDone", {
-			name: this.testName,
-			module: module.name,
+			name: testName,
+			module: moduleName,
 			skipped: skipped,
 			failed: bad,
 			passed: this.assertions.length - bad,
@@ -310,6 +323,8 @@ Test.prototype = {
 		priority = config.reorder && defined.sessionStorage &&
 				+sessionStorage.getItem( "qunit-test-" + this.module.name + "-" + this.testName );
 
+		this.previousFailure = !!priority;
+
 		return synchronize( run, priority, config.seed );
 	},
 

test/logs.js

@@ -152,7 +152,8 @@ QUnit.test( module1Test1.name, function( assert ) {
 	assert.deepEqual( testContext, {
 		module: module1Context.name,
 		name: module1Test1.name,
-		testId: module1Test1.testId
+		testId: module1Test1.testId,
+		previousFailure: false
 	}, "test context" );
 
 	assert.strictEqual( moduleDoneContext, undefined, "moduleDone context" );
@@ -199,7 +200,8 @@ QUnit.test( module1Test2.name, function( assert ) {
 	assert.deepEqual( testContext, {
 		module: module1Context.name,
 		name: module1Test2.name,
-		testId: module1Test2.testId
+		testId: module1Test2.testId,
+		previousFailure: false
 	}, "test context" );
 
 	assert.strictEqual( moduleDoneContext, undefined, "moduleDone context" );
@@ -220,7 +222,8 @@ QUnit.test( module2Test1.name, function( assert ) {
 	assert.deepEqual( testContext, {
 		module: module2Context.name,
 		name: module2Test1.name,
-		testId: module2Test1.testId
+		testId: module2Test1.testId,
+		previousFailure: false
 	}, "test context" );
 
 	assert.equal(
@@ -253,7 +256,8 @@ QUnit.test( module2Test2.name, function( assert ) {
 	assert.deepEqual( testContext, {
 		module: module2Context.name,
 		name: module2Test2.name,
-		testId: module2Test2.testId
+		testId: module2Test2.testId,
+		previousFailure: false
 	}, "test context" );
 	assert.deepEqual( moduleContext, module2Context, "module context" );