Core: Ensure skipped child module hooks don't leak memory

If a module is skipped, the hooks of chid modules should be deleted
as well from our global config so that references to memory held
by these closures may be released.

Fixes #1649.

src/test.js

@@ -398,8 +398,20 @@ Test.prototype = {
 		function logSuiteEnd( module ) {
 
 			// Reset `module.hooks` to ensure that anything referenced in these hooks
-			// has been released to be garbage collected.
-			module.hooks = {};
+			// has been released to be garbage collected. Descendant modules that were
+			// entirely skipped, e.g. due to filtering, will never have this method
+			// called for them, but might have hooks with references pinning data in
+			// memory (even if the hooks weren't actually executed), so we reset the
+			// hooks on all descendant modules here as well. This is safe because we
+			// will never call this as long as any descendant modules still have tests
+			// to run. This also means that in multi-tiered nesting scenarios we might
+			// reset the hooks multiple times on some modules, but that's harmless.
+			const modules = [ module ];
+			while ( modules.length ) {
+				const nextModule = modules.shift();
+				nextModule.hooks = {};
+				modules.push( ...nextModule.childModules );
+			}
 
 			emit( "suiteEnd", module.suiteReport.end( true ) );
 			return runLoggingCallbacks( "moduleDone", {

test/cli/cli-main.js

@@ -434,6 +434,15 @@ CALLBACK: done`;
 			assert.equal( execution.stderr, "" );
 			assert.equal( execution.stdout, expectedOutput[ command ] );
 		} );
+
+		QUnit.test( "callbacks and hooks from filtered-out modules are properly released for garbage collection", async assert => {
+			const command = "node --expose-gc ../../../bin/qunit.js --filter '!child' memory-leak/*.js";
+			const execution = await execute( command );
+
+			assert.equal( execution.code, 0 );
+			assert.equal( execution.stderr, "" );
+			assert.equal( execution.stdout, expectedOutput[ command ] );
+		} );
 	}
 
 	QUnit.module( "filter", () => {

test/cli/fixtures/expected/tap-outputs.js

@@ -267,6 +267,17 @@ ok 1 Single > has a test
 	"node --expose-gc ../../../bin/qunit.js memory-leak/*.js":
 `TAP version 13
 ok 1 some nested module > can call method on foo
+ok 2 some nested module > child module > child test
+ok 3 later thing > has released all foos
+1..3
+# pass 3
+# skip 0
+# todo 0
+# fail 0`,
+
+	"node --expose-gc ../../../bin/qunit.js --filter '!child' memory-leak/*.js":
+`TAP version 13
+ok 1 some nested module > can call method on foo
 ok 2 later thing > has released all foos
 1..2
 # pass 2

test/cli/fixtures/memory-leak/index.js

@@ -44,6 +44,17 @@ QUnit.module( "some nested module", function( hooks ) {
 		assert.equal( foo1.getId(), "FooNum" );
 	} );
 
+	QUnit.module( "child module", function( hooks ) {
+		let foo3;
+
+		hooks.beforeEach( function() {
+			foo3 = foo1;
+		} );
+
+		QUnit.test( "child test", function( assert ) {
+			assert.ok( foo3 );
+		} );
+	} );
 } );
 
 QUnit.module( "later thing", function() {
@@ -58,10 +69,10 @@ QUnit.module( "later thing", function() {
 
 		let snapshot = await streamToString( v8.getHeapSnapshot() );
 		let matches = snapshot.match( reHeap ) || [];
-		assert.strictEqual( matches.length, 2, "the before heap" );
+		assert.notEqual( matches.length, 0, "the before heap" );
 
 		snapshot = matches = null;
-		assert.strictEqual( foos.size, 2, "foos in Set" );
+		assert.notEqual( foos.size, 0, "foos in Set" );
 
 		// Comment out the below to test the failure mode
 		foos.clear();