Skip to content

Commit

Permalink
fix test for #121
Browse files Browse the repository at this point in the history
dropping support for node v6.x as it is not working correctly with the installed SIGINT handlers
  • Loading branch information
silkentrance committed Mar 16, 2019
1 parent 0c5bc5c commit 88e2ee1
Show file tree
Hide file tree
Showing 14 changed files with 222 additions and 87 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ module.exports = {
"semi": [
"error",
"always"
]
],
"no-extra-boolean-cast": 0
}
};
1 change: 0 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ node_js:
- "10"
- "9"
- "8"
- "6"
sudo: false
cache:
directories:
Expand Down
107 changes: 68 additions & 39 deletions lib/tmp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand Down Expand Up @@ -101,7 +103,7 @@ function _isUndefined(obj) {
*/
function _parseArguments(options, callback) {
/* istanbul ignore else */
if (typeof options == 'function') {
if (typeof options === 'function') {
return [{}, options];
}

Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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.
Expand Down
32 changes: 18 additions & 14 deletions test/child-process.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
}
Expand All @@ -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) {
Expand All @@ -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;
}, []));
}

49 changes: 40 additions & 9 deletions test/issue121-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
}
34 changes: 34 additions & 0 deletions test/issue129-sigint-test.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
});
});
23 changes: 11 additions & 12 deletions test/outband/issue121.js
Original file line number Diff line number Diff line change
@@ -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);
};
Loading

0 comments on commit 88e2ee1

Please sign in to comment.