Skip to content

Commit 2b29df7

Browse files
committed
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]>
1 parent 5942a34 commit 2b29df7

File tree

5 files changed

+12
-12
lines changed

5 files changed

+12
-12
lines changed

test/common/README.md

-5
Original file line numberDiff line numberDiff line change
@@ -219,11 +219,6 @@ Platform check for Windows.
219219

220220
Platform check for Windows 32-bit on Windows 64-bit.
221221

222-
### leakedGlobals()
223-
* return [&lt;Array>]
224-
225-
Indicates whether any globals are not on the `knownGlobals` list.
226-
227222
### localhostIPv4
228223
* [&lt;string>]
229224

test/common/index.js

-1
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,6 @@ module.exports = {
726726
isSunOS,
727727
isWindows,
728728
isWOW64,
729-
leakedGlobals,
730729
localIPv6Hosts,
731730
mustCall,
732731
mustCallAsync,

test/common/index.mjs

-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ const {
2525
ddCommand,
2626
platformTimeout,
2727
allowGlobals,
28-
leakedGlobals,
2928
mustCall,
3029
mustCallAtLeast,
3130
mustCallAsync,
@@ -75,7 +74,6 @@ export {
7574
ddCommand,
7675
platformTimeout,
7776
allowGlobals,
78-
leakedGlobals,
7977
mustCall,
8078
mustCallAtLeast,
8179
mustCallAsync,

test/fixtures/leakedGlobal.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
'use strict';
2+
3+
require('../common');
4+
5+
global.gc = 42; // intentionally leak a global

test/parallel/test-common.js

+7-4
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,13 @@ const assert = require('assert');
2727
const { execFile } = require('child_process');
2828

2929
// test for leaked global detection
30-
global.gc = 42; // Not a valid global unless --expose_gc is set.
31-
assert.deepStrictEqual(common.leakedGlobals(), ['gc']);
32-
delete global.gc;
33-
30+
{
31+
const p = fixtures.path('leakedGlobal.js');
32+
execFile(process.argv[0], [p], common.mustCall((ex, stdout, stderr) => {
33+
assert.notStrictEqual(ex.code, 0);
34+
assert.ok(/\bAssertionError\b.*\bUnexpected global\b.*\bgc\b/.test(stderr));
35+
}));
36+
}
3437

3538
// common.mustCall() tests
3639
assert.throws(function() {

0 commit comments

Comments
 (0)