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
add appveyor build for node 11
  • Loading branch information
silkentrance committed Mar 16, 2019
1 parent 0c5bc5c commit dc55725
Show file tree
Hide file tree
Showing 15 changed files with 223 additions and 88 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.js
Expand Up @@ -23,6 +23,7 @@ module.exports = {
"semi": [
"error",
"always"
]
],
"no-extra-boolean-cast": 0
}
};
1 change: 0 additions & 1 deletion .travis.yml
Expand Up @@ -4,7 +4,6 @@ node_js:
- "10"
- "9"
- "8"
- "6"
sudo: false
cache:
directories:
Expand Down
2 changes: 1 addition & 1 deletion appveyor.yml
Expand Up @@ -2,11 +2,11 @@

environment:
matrix:
- nodejs_version: "6"
- nodejs_version: "7"
- nodejs_version: "8"
- nodejs_version: "9"
- nodejs_version: "10"
- nodejs_version: "11"

install:
- ps: Install-Product node $env:nodejs_version
Expand Down
107 changes: 68 additions & 39 deletions lib/tmp.js
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
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
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
@@ -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();
});
});
});
});

0 comments on commit dc55725

Please sign in to comment.