test: make common.noop a getter

Make `common.noop` a getter that returns a new function object each time
the propery is read, not the same one. The old behavior could possibly
lead to subtle and hard to catch bugs in some cases.

Refs: nodejs#12711 (comment)

test/README.md

@@ -348,7 +348,10 @@ Returns `true` if the exit code `exitCode` and/or signal name `signal` represent
 
 ### noop
 
-A non-op `Function` that can be used for a variety of scenarios.
+A getter that returns a non-op `Function` which can be used for a variety of
+scenarios. A new function object is always created, so the result of
+`common.noop` must be saved in a variable if it is intended to be used in
+assertions.
 
 For instance,
 

test/common.js

@@ -35,9 +35,11 @@ const execSync = require('child_process').execSync;
 const testRoot = process.env.NODE_TEST_DIR ?
                    fs.realpathSync(process.env.NODE_TEST_DIR) : __dirname;
 
-const noop = () => {};
+Object.defineProperty(exports, 'noop', {
+  enumerable: true,
+  get: () => () => {}
+});
 
-exports.noop = noop;
 exports.fixturesDir = path.join(__dirname, 'fixtures');
 exports.tmpDirName = 'tmp';
 // PORT should match the definition in test/testpy/__init__.py.
@@ -434,9 +436,9 @@ function runCallChecks(exitCode) {
 exports.mustCall = function(fn, expected) {
   if (typeof fn === 'number') {
     expected = fn;
-    fn = noop;
+    fn = exports.noop;
   } else if (fn === undefined) {
-    fn = noop;
+    fn = exports.noop;
   }
 
   if (expected === undefined)
@@ -530,9 +532,9 @@ util.inherits(ArrayStream, stream.Stream);
 exports.ArrayStream = ArrayStream;
 ArrayStream.prototype.readable = true;
 ArrayStream.prototype.writable = true;
-ArrayStream.prototype.pause = noop;
-ArrayStream.prototype.resume = noop;
-ArrayStream.prototype.write = noop;
+ArrayStream.prototype.pause = exports.noop;
+ArrayStream.prototype.resume = exports.noop;
+ArrayStream.prototype.write = exports.noop;
 
 // Returns true if the exit code "exitCode" and/or signal name "signal"
 // represent the exit code and/or signal name of a node process that aborted,

test/parallel/test-common.js

@@ -40,6 +40,11 @@ assert.throws(function() {
 }, /^TypeError: Invalid expected value: \/foo\/$/);
 
 
+// common.noop() tests
+assert.strictEqual(common.noop(), undefined);
+assert.notStrictEqual(common.noop, common.noop);
+
+
 // assert.fail() tests
 assert.throws(
   () => { assert.fail('fhqwhgads'); },

test/parallel/test-event-emitter-remove-all-listeners.js

@@ -24,7 +24,6 @@ const common = require('../common');
 const assert = require('assert');
 const events = require('events');
 
-
 function expect(expected) {
   const actual = [];
   process.on('exit', function() {
@@ -38,24 +37,28 @@ function expect(expected) {
 
 {
   const ee = new events.EventEmitter();
-  ee.on('foo', common.noop);
-  ee.on('bar', common.noop);
-  ee.on('baz', common.noop);
-  ee.on('baz', common.noop);
+  const fooListener = common.noop;
+  const barListener = common.noop;
+  const bazListener1 = common.noop;
+  const bazListener2 = common.noop;
+  ee.on('foo', fooListener);
+  ee.on('bar', barListener);
+  ee.on('baz', bazListener1);
+  ee.on('baz', bazListener2);
   const fooListeners = ee.listeners('foo');
   const barListeners = ee.listeners('bar');
   const bazListeners = ee.listeners('baz');
   ee.on('removeListener', expect(['bar', 'baz', 'baz']));
   ee.removeAllListeners('bar');
   ee.removeAllListeners('baz');
-  assert.deepStrictEqual(ee.listeners('foo'), [common.noop]);
+  assert.deepStrictEqual(ee.listeners('foo'), [fooListener]);
   assert.deepStrictEqual(ee.listeners('bar'), []);
   assert.deepStrictEqual(ee.listeners('baz'), []);
   // After calling removeAllListeners(),
   // the old listeners array should stay unchanged.
-  assert.deepStrictEqual(fooListeners, [common.noop]);
-  assert.deepStrictEqual(barListeners, [common.noop]);
-  assert.deepStrictEqual(bazListeners, [common.noop, common.noop]);
+  assert.deepStrictEqual(fooListeners, [fooListener]);
+  assert.deepStrictEqual(barListeners, [barListener]);
+  assert.deepStrictEqual(bazListeners, [bazListener1, bazListener2]);
   // After calling removeAllListeners(),
   // new listeners arrays is different from the old.
   assert.notStrictEqual(ee.listeners('bar'), barListeners);