test: do not export common.leakedGlobals()

common.leakedGlobals() was exposed only to test its logic. The logic can instead be tested by running a fixture file that leaks a global and seeing if `common` causes an AssertionError on exit. This way, the entire functionality of leak detection is tested rather than just the leakedGlobals() function. It also reduces API surface area for the common monolith by one function. PR-URL: #22965 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Sep 23, 2018 · 43e3cf9 · 43e3cf9
1 parent d3bc862
commit 43e3cf9
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 12 deletions.
diff --git a/test/common/README.md b/test/common/README.md
@@ -219,11 +219,6 @@ Platform check for Windows.
 
 Platform check for Windows 32-bit on Windows 64-bit.
 
-### leakedGlobals()
-* return [&lt;Array>]
-
-Indicates whether any globals are not on the `knownGlobals` list.
-
 ### localhostIPv4
 * [&lt;string>]
 

diff --git a/test/common/index.js b/test/common/index.js
@@ -723,7 +723,6 @@ module.exports = {
   isSunOS,
   isWindows,
   isWOW64,
-  leakedGlobals,
   localIPv6Hosts,
   mustCall,
   mustCallAsync,

diff --git a/test/common/index.mjs b/test/common/index.mjs
@@ -25,7 +25,6 @@ const {
   ddCommand,
   platformTimeout,
   allowGlobals,
-  leakedGlobals,
   mustCall,
   mustCallAtLeast,
   mustCallAsync,
@@ -75,7 +74,6 @@ export {
   ddCommand,
   platformTimeout,
   allowGlobals,
-  leakedGlobals,
   mustCall,
   mustCallAtLeast,
   mustCallAsync,

diff --git a/test/fixtures/leakedGlobal.js b/test/fixtures/leakedGlobal.js
@@ -0,0 +1,5 @@
+'use strict';
+
+require('../common');
+
+global.gc = 42; // intentionally leak a global
diff --git a/test/parallel/test-common.js b/test/parallel/test-common.js
@@ -27,10 +27,13 @@ const assert = require('assert');
 const { execFile } = require('child_process');
 
 // test for leaked global detection
-global.gc = 42;  // Not a valid global unless --expose_gc is set.
-assert.deepStrictEqual(common.leakedGlobals(), ['gc']);
-delete global.gc;
-
+{
+  const p = fixtures.path('leakedGlobal.js');
+  execFile(process.argv[0], [p], common.mustCall((ex, stdout, stderr) => {
+    assert.notStrictEqual(ex.code, 0);
+    assert.ok(/\bAssertionError\b.*\bUnexpected global\b.*\bgc\b/.test(stderr));
+  }));
+}
 
 // common.mustCall() tests
 assert.throws(function() {