From 43e3cf93e8cfad5a494d8ab9d927107f905f7fb8 Mon Sep 17 00:00:00 2001
From: Rich Trott <rtrott@gmail.com>
Date: Wed, 19 Sep 2018 16:33:18 -0700
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/22965
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 test/common/README.md         |  5 -----
 test/common/index.js          |  1 -
 test/common/index.mjs         |  2 --
 test/fixtures/leakedGlobal.js |  5 +++++
 test/parallel/test-common.js  | 11 +++++++----
 5 files changed, 12 insertions(+), 12 deletions(-)
 create mode 100644 test/fixtures/leakedGlobal.js

diff --git a/test/common/README.md b/test/common/README.md
index 009f8247879f69..cefacc756422a6 100644
--- 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
index 2efec135772c98..0ccab184d9d454 100644
--- 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
index e0842011c663c9..832f68a9ee0d3e 100644
--- 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
new file mode 100644
index 00000000000000..6f4b1b992e250b
--- /dev/null
+++ 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
index eafe4dd830bcb2..51e0302868cbe6 100644
--- 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() {