From 88e2ee1f64c210bf42f70fc8c417bfc25feea9d0 Mon Sep 17 00:00:00 2001
From: Carsten Klein <trancesilken@gmail.com>
Date: Wed, 12 Sep 2018 18:11:08 +0200
Subject: [PATCH] fix test for #121 dropping support for node v6.x as it is not
 working correctly with the installed SIGINT handlers

---
 .eslintrc.js                      |   3 +-
 .travis.yml                       |   1 -
 lib/tmp.js                        | 107 +++++++++++++++++++-----------
 test/child-process.js             |  32 +++++----
 test/issue121-test.js             |  49 +++++++++++---
 test/issue129-sigint-test.js      |  34 ++++++++++
 test/outband/issue121.js          |  23 +++----
 test/outband/issue121.json        |   1 +
 test/outband/issue129-sigint.js   |  38 +++++++++++
 test/outband/issue129-sigint.json |   3 +
 test/outband/issue129.js          |  12 ++--
 test/spawn-custom.js              |   1 -
 test/spawn-generic.js             |   2 +-
 test/spawn.js                     |   3 -
 14 files changed, 222 insertions(+), 87 deletions(-)
 create mode 100644 test/issue129-sigint-test.js
 create mode 100644 test/outband/issue129-sigint.js
 create mode 100644 test/outband/issue129-sigint.json

diff --git a/.eslintrc.js b/.eslintrc.js
index 45d1d61..43ed1a1 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -23,6 +23,7 @@ module.exports = {
     "semi": [
       "error",
       "always"
-    ]
+    ],
+    "no-extra-boolean-cast": 0
   }
 };
diff --git a/.travis.yml b/.travis.yml
index fb0b545..f3dde6f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -4,7 +4,6 @@ node_js:
   - "10"
   - "9"
   - "8"
-  - "6"
 sudo: false
 cache:
   directories:
diff --git a/lib/tmp.js b/lib/tmp.js
index 8fe48a0..33d59bd 100644
--- a/lib/tmp.js
+++ b/lib/tmp.js
@@ -43,7 +43,9 @@ const
   DIR_MODE = 448 /* 0o700 */,
   FILE_MODE = 384 /* 0o600 */,
 
-  EVENT = 'exit',
+  EXIT = 'exit',
+
+  SIGINT = 'SIGINT',
 
   // this will hold the objects need to be removed on exit
   _removeObjects = [];
@@ -101,7 +103,7 @@ function _isUndefined(obj) {
  */
 function _parseArguments(options, callback) {
   /* istanbul ignore else */
-  if (typeof options == 'function') {
+  if (typeof options === 'function') {
     return [{}, options];
   }
 
@@ -132,7 +134,7 @@ function _generateTmpName(opts) {
     var template = opts.template;
     // make sure that we prepend the tmp path if none was given
     /* istanbul ignore else */
-    if (path.basename(template) == template)
+    if (path.basename(template) === template)
       template = path.join(opts.dir || tmpDir, template);
     return template.replace(TEMPLATE_PATTERN, _randomChars(6));
   }
@@ -479,7 +481,7 @@ function _prepareRemoveCallback(removeFunction, arg, cleanupCallbackSync) {
 
       called = true;
       // sync?
-      if (removeFunction.length == 1) {
+      if (removeFunction.length === 1) {
         try {
           removeFunction(arg);
           return next(null);
@@ -550,7 +552,7 @@ function isENOENT(error) {
  *   error.errno n/a
  */
 function isExpectedError(error, code, errno) {
-  return error.code == code || error.code == errno;
+  return error.code === code || error.code === errno;
 }
 
 /**
@@ -569,60 +571,87 @@ function setGracefulCleanup() {
  * @returns {Boolean} true whether listener is a legacy listener
  */
 function _is_legacy_listener(listener) {
-  return (listener.name == '_exit' || listener.name == '_uncaughtExceptionThrown')
+  return (listener.name === '_exit' || listener.name === '_uncaughtExceptionThrown')
     && listener.toString().indexOf('_garbageCollector();') > -1;
 }
 
 /**
- * Safely install process exit listeners.
- * 
+ * Safely install SIGINT listener.
+ *
+ * NOTE: this will only work on OSX and Linux.
+ *
  * @private
  */
-function _safely_install_listener() {
-  var listeners = process.listeners(EVENT);
+function _safely_install_sigint_listener() {
 
-  // collect any existing listeners
-  var existingListeners = [];
-  for (var i = 0, length = listeners.length; i < length; i++) {
-    var lstnr = listeners[i];
+  const listeners = process.listeners(SIGINT);
+  const existingListeners = [];
+  for (let i = 0, length = listeners.length; i < length; i++) {
+    const lstnr = listeners[i];
     /* istanbul ignore else */
-    if (lstnr.name == '_tmp$safe_listener' || _is_legacy_listener(lstnr)) {
-      // we must forget about the uncaughtException listener
-      if (lstnr.name != '_uncaughtExceptionThrown') existingListeners.push(lstnr);
-      process.removeListener(EVENT, lstnr);
+    if (lstnr.name === '_tmp$sigint_listener') {
+      existingListeners.push(lstnr);
+      process.removeListener(SIGINT, lstnr);
     }
   }
-
-  // windows does not support signals
-  // it'd never had won if it wasn't a major PITA
-  // with node v8.x and win 10 this is no longer an issue
-  if (process.platform == 'win32') {
-    var rl = require('readline').createInterface({
-      input: process.stdin,
-      output: process.stdout
-    });
-
-    rl.on('SIGINT', function () {
-      process.emit('SIGINT');
-    });
-  }
-
-  process.on('SIGINT', function () {
-    process.exit(0);
+  process.on(SIGINT, function _tmp$sigint_listener(doExit) {
+    for (let i = 0, length = existingListeners.length; i < length; i++) {
+      // let the existing listener do the garbage collection (e.g. jest sandbox)
+      try {
+        existingListeners[i](false);
+      } catch (err) {
+        // ignore
+      }
+    }
+    try {
+      // force the garbage collector even it is called again in the exit listener
+      _garbageCollector();
+    } finally {
+      if (!!doExit) {
+        process.exit(0);
+      }
+    }
   });
+}
+
+/**
+ * Safely install process exit listener.
+ *
+ * @private
+ */
+function _safely_install_exit_listener() {
+  const listeners = process.listeners(EXIT);
 
-  process.addListener(EVENT, function _tmp$safe_listener(data) {
+  // collect any existing listeners
+  const existingListeners = [];
+  for (let i = 0, length = listeners.length; i < length; i++) {
+    const lstnr = listeners[i];
     /* istanbul ignore else */
-    if (existingListeners.length) {
-      for (var i = 0, length = existingListeners.length; i < length; i++) {
+    // TODO: remove support for legacy listeners once release 1.0.0 is out
+    if (lstnr.name === '_tmp$safe_listener' || _is_legacy_listener(lstnr)) {
+      // we must forget about the uncaughtException listener, hopefully it is ours
+      if (lstnr.name !== '_uncaughtExceptionThrown') {
+        existingListeners.push(lstnr);
+      }
+      process.removeListener(EXIT, lstnr);
+    }
+  }
+  // TODO: what was the data parameter good for?
+  process.addListener(EXIT, function _tmp$safe_listener(data) {
+    for (let i = 0, length = existingListeners.length; i < length; i++) {
+      // let the existing listener do the garbage collection (e.g. jest sandbox)
+      try {
         existingListeners[i](data);
+      } catch (err) {
+        // ignore
       }
     }
     _garbageCollector();
   });
 }
 
-_safely_install_listener();
+_safely_install_exit_listener();
+_safely_install_sigint_listener();
 
 /**
  * Configuration options.
diff --git a/test/child-process.js b/test/child-process.js
index ce189e2..dd9fd5b 100644
--- a/test/child-process.js
+++ b/test/child-process.js
@@ -12,24 +12,25 @@ module.exports.genericChildProcess = _spawnProcess('spawn-generic.js');
 module.exports.childProcess = _spawnProcess('spawn-custom.js');
 
 function _spawnProcess(spawnFile) {
-  return function (testCase, configFile, cb) {
-    var
+  return function (testCase, configFile, cb, signal) {
+    const
       configFilePath = path.join(__dirname, 'outband', configFile),
       commandArgs = [path.join(__dirname, spawnFile), configFilePath];
 
     exists(configFilePath, function (configExists) {
-      if (configExists) return _doSpawn(commandArgs, cb);
+      if (configExists) return _doSpawn(commandArgs, cb, signal);
 
       cb(new Error('ENOENT: configFile ' + configFilePath + ' does not exist'));
     });
   };
 }
 
-function _doSpawn(commandArgs, cb) {
-  var
+function _doSpawn(commandArgs, cb, signal) {
+  const
     node_path = process.argv[0],
     stdoutBufs = [],
-    stderrBufs = [],
+    stderrBufs = [];
+  let
     child,
     done = false,
     stderrDone = false,
@@ -48,14 +49,11 @@ function _doSpawn(commandArgs, cb) {
   child = spawn(node_path, commandArgs);
   child.stdin.end();
 
-  // TODO we no longer support node 0.6
-  // Cannot use 'close' event because not on node-0.6.
   function _close() {
-    var
-      stderr = _bufferConcat(stderrBufs).toString(),
-      stdout = _bufferConcat(stdoutBufs).toString();
-
     if (stderrDone && stdoutDone && !done) {
+      const
+        stderr = _bufferConcat(stderrBufs).toString(),
+        stdout = _bufferConcat(stdoutBufs).toString();
       done = true;
       cb(null, stderr, stdout);
     }
@@ -81,6 +79,13 @@ function _doSpawn(commandArgs, cb) {
     stderrDone = true;
     _close();
   });
+
+  if (signal) {
+    setTimeout(function () {
+      // does not work on node 6
+      child.kill(signal);
+    }, 2000);
+  }
 }
 
 function _bufferConcat(buffers) {
@@ -89,10 +94,9 @@ function _bufferConcat(buffers) {
   }
 
   return new Buffer(buffers.reduce(function (acc, buf) {
-    for (var i = 0; i < buf.length; i++) {
+    for (let i = 0; i < buf.length; i++) {
       acc.push(buf[i]);
     }
     return acc;
   }, []));
 }
-
diff --git a/test/issue121-test.js b/test/issue121-test.js
index 69a24f3..16ff6ad 100644
--- a/test/issue121-test.js
+++ b/test/issue121-test.js
@@ -2,26 +2,57 @@
 // vim: expandtab:ts=2:sw=2
 
 const
-  assert = require('assert'),
   assertions = require('./assertions'),
   childProcess = require('./child-process').childProcess,
-  signals = ['SIGINT', 'SIGTERM'];
+  os = require('os'),
+  rimraf = require('rimraf'),
+  testCases = [
+    { signal: 'SIGINT', expectExists: false },
+    { signal: 'SIGTERM', expectExists: true }
+  ];
+
+const isWindows = os.platform() === 'win32';
+
+let tfunc = it;
+if (isWindows) {
+  // skip tests on win32
+  tfunc = xit;
+}
 
 describe('tmp', function () {
   describe('issue121 - clean up on terminating signals', function () {
-    for (var i=0; i < signals.length; i++) {
-      issue121Tests(signals[i]);
+    for (let tc of testCases) {
+      tfunc('for signal ' + tc.signal, function (done) {
+        // increase timeout so that the child process may fail
+        this.timeout(20000);
+        issue121Tests(tc.signal, tc.expectExists)(done);
+      });
     }
   });
 });
 
-function issue121Tests(signal) {
+function issue121Tests(signal, expectExists) {
   return function (done) {
-    childProcess('issue121.json', function (err, stderr, stdout) {
+    childProcess(this, 'issue121.json', function (err, stderr, stdout) {
       if (err) return done(err);
       else if (stderr) return done(new Error(stderr));
-      else assertions.assertDoesNotExist(stdout);
-      done();
-    }, true);
+
+      try {
+        if (expectExists) {
+          assertions.assertExists(stdout);
+        }
+        else {
+          assertions.assertDoesNotExist(stdout);
+        }
+        done();
+      } catch (err) {
+        done(err);
+      } finally {
+        // cleanup
+        if (expectExists) {
+          rimraf.sync(stdout);
+        }
+      }
+    }, signal);
   };
 }
diff --git a/test/issue129-sigint-test.js b/test/issue129-sigint-test.js
new file mode 100644
index 0000000..0ba9b82
--- /dev/null
+++ b/test/issue129-sigint-test.js
@@ -0,0 +1,34 @@
+/* eslint-disable no-octal */
+// vim: expandtab:ts=2:sw=2
+
+var
+  assert = require('assert'),
+  assertions = require('./assertions'),
+  childProcess = require('./child-process').childProcess;
+
+describe('tmp', function () {
+  describe('issue129: safely install sigint listeners', function () {
+    it('when simulating sandboxed behavior', function (done) {
+      childProcess(this, 'issue129-sigint.json', function (err, stderr, stdout) {
+        if (err) return done(err);
+        if (!stdout && !stderr) return done(new Error('stderr or stdout expected'));
+        if (stderr) {
+          try {
+            assertions.assertDoesNotStartWith(stderr, 'EEXISTS:MULTIPLE');
+            assertions.assertDoesNotStartWith(stderr, 'ENOAVAIL:EXISTING');
+          } catch (err) {
+            return done(err);
+          }
+        }
+        if (stdout) {
+          try {
+            assert.equal(stdout, 'EOK', 'existing listeners should have been removed and called');
+          } catch (err) {
+            return done(err);
+          }
+        }
+        done();
+      });
+    });
+  });
+});
diff --git a/test/outband/issue121.js b/test/outband/issue121.js
index 0dce377..deba395 100644
--- a/test/outband/issue121.js
+++ b/test/outband/issue121.js
@@ -1,20 +1,19 @@
 /* eslint-disable no-octal */
 // vim: expandtab:ts=2:sw=2
 
-var
-  fs = require('fs'),
-  tmp = require('../../lib/tmp'),
-  // we reuse the fixtures from issue62 here
-  fixture = require('./issue62');
-
-tmp.setGracefulCleanup();
+const
+  tmp = require('../../lib/tmp');
 
 // https://github.com/raszi/node-tmp/issues/121
-module.exports = function (signal) {
-  fixture.apply(this, [tmp.dirSync({ unsafeCleanup: true }), tmp]);
+module.exports = function () {
+
+  tmp.setGracefulCleanup();
+
+  const result = tmp.dirSync({ unsafeCleanup: true });
 
-  // make sure that the process keeps running
-  setTimeout(function () {}, 1000000);
+  this.out(result.name, function () { });
 
-  this.kill(signal);
+  setTimeout(function () {
+    throw new Error('ran into timeout');
+  }, 10000);
 };
diff --git a/test/outband/issue121.json b/test/outband/issue121.json
index 86429e8..2210135 100644
--- a/test/outband/issue121.json
+++ b/test/outband/issue121.json
@@ -1,3 +1,4 @@
 {
+  "graceful": true,
   "tc": "issue121"
 }
diff --git a/test/outband/issue129-sigint.js b/test/outband/issue129-sigint.js
new file mode 100644
index 0000000..0710536
--- /dev/null
+++ b/test/outband/issue129-sigint.js
@@ -0,0 +1,38 @@
+/* eslint-disable no-octal */
+// vim: expandtab:ts=2:sw=2
+
+// addendum to https://github.com/raszi/node-tmp/issues/129 so that with jest sandboxing we do not install our sigint
+// listener multiple times
+module.exports = function () {
+  var callState = {
+    existingListener : false,
+  };
+
+  // simulate an existing SIGINT listener
+  var listener1 = (function (callState) {
+    return function _tmp$sigint_listener(doExit) {
+      callState.existingListener = !doExit;
+    };
+  })(callState);
+
+  process.addListener('SIGINT', listener1);
+
+  // now let tmp install its listener safely
+  require('../../lib/tmp');
+
+  var sigintListeners = [];
+
+  var listeners = process.listeners('SIGINT');
+  for (var i = 0; i < listeners.length; i++) {
+    var listener = listeners[i];
+    if (listener.name === '_tmp$sigint_listener') {
+      sigintListeners.push(listener);
+    }
+  }
+
+  if (listeners.length > 1) this.fail('EEXISTS:MULTIPLE: existing SIGINT listener was not removed', this.exit);
+  listeners[0](false); // prevent listener from exiting the process
+  if (!callState.existingListener) this.fail('ENOAVAIL:EXISTING: existing listener was not called', this.exit);
+  this.out('EOK', this.exit);
+  process.exit(0);
+};
diff --git a/test/outband/issue129-sigint.json b/test/outband/issue129-sigint.json
new file mode 100644
index 0000000..967ff91
--- /dev/null
+++ b/test/outband/issue129-sigint.json
@@ -0,0 +1,3 @@
+{
+  "tc": "issue129-sigint"
+}
diff --git a/test/outband/issue129.js b/test/outband/issue129.js
index 0d65547..a36c56b 100644
--- a/test/outband/issue129.js
+++ b/test/outband/issue129.js
@@ -5,10 +5,10 @@
 module.exports = function () {
   // dup from lib/tmp.js
   function _is_legacy_listener(listener) {
-    return (listener.name == '_exit' || listener.name == '_uncaughtExceptionThrown')
-      && listener.toString().indexOf('_garbageCollector();') != -1;
+    return (listener.name === '_exit' || listener.name === '_uncaughtExceptionThrown')
+      && listener.toString().indexOf('_garbageCollector();') !== -1;
   }
-
+å
   function _garbageCollector() {}
 
   var callState = {
@@ -56,14 +56,14 @@ module.exports = function () {
   for (var i = 0; i < listeners.length; i++) {
     var listener = listeners[i];
     // the order here is important
-    if (listener.name == '_tmp$safe_listener') {
+    if (listener.name === '_tmp$safe_listener') {
       newStyleListeners.push(listener);
     }
     else if (_is_legacy_listener(listener)) {
-      if (listener.name == '_uncaughtExceptionThrown') legacyUncaughtListener = listener;
+      if (listener.name === '_uncaughtExceptionThrown') legacyUncaughtListener = listener;
       else legacyExitListener = listener;
     }
-  }
+  }å
 
   if (legacyExitListener) this.fail('EEXISTS:LEGACY:EXIT existing legacy exit listener was not removed', this.exit);
   if (legacyUncaughtListener) this.fail('EEXISTS:LEGACY:UNCAUGHT existing legacy uncaught exception thrown listener was not removed', this.exit);
diff --git a/test/spawn-custom.js b/test/spawn-custom.js
index bb1cc27..f833f07 100644
--- a/test/spawn-custom.js
+++ b/test/spawn-custom.js
@@ -11,4 +11,3 @@ spawn.graceful = !!config.graceful;
 // import the test case function and execute it
 var fn = require(path.join(__dirname, 'outband', config.tc));
 fn.apply(spawn);
-
diff --git a/test/spawn-generic.js b/test/spawn-generic.js
index 6b4cb5b..d557db0 100644
--- a/test/spawn-generic.js
+++ b/test/spawn-generic.js
@@ -23,7 +23,7 @@ if (config.async)
   fnUnderTest(config.options, function (err, name, fdOrCallback, cb) {
     if (err) spawn.err(err);
     else {
-      var result = null; 
+      var result = null;
       if (config.file) result = { name: name, fd: fdOrCallback, removeCallback: cb };
       else result = { name: name, removeCallback: fdOrCallback };
       fn.apply(spawn, [result, tmp]);
diff --git a/test/spawn.js b/test/spawn.js
index 5dfd87e..3f89510 100644
--- a/test/spawn.js
+++ b/test/spawn.js
@@ -28,9 +28,6 @@ module.exports = {
   },
   exit: function (code) {
     process.exit(code || 0);
-  },
-  kill: function (signal) {
-    process.kill(signal || 'SIGINT');
   }
 };