Skip to content

Commit

Permalink
Fixes #115
Browse files Browse the repository at this point in the history
Add node 7 to travis build matrix
  • Loading branch information
silkentrance committed Feb 25, 2017
1 parent 42cd5f7 commit 01728fb
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 2 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
node_modules/
.idea/
.*.swp
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ node_js:
- "5.11"
- "6.0"
- "6.1"
- "7"
- "node"
sudo: false
cache:
Expand Down
40 changes: 39 additions & 1 deletion lib/tmp.js
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ function _prepareTmpFileRemoveCallback(name, fd, opts) {
// under some node/windows related circumstances, a temporary file
// may have not be created as expected or the file was already closed
// by the user, in which case we will simply ignore the error
if (e.errno != -EBADF && e.errno != -ENOENT) {
if (!isEBADF(e) && !isENOENT(e)) {
// reraise any unanticipated error
throw e;
}
Expand Down Expand Up @@ -456,6 +456,44 @@ function _garbageCollector() {
}
}

/**
* Helper for testing against EBADF to compensate changes made to Node 7.x under Windows.
*/
function isEBADF(error) {
return isExpectedError(error, -EBADF, 'EBADF');
}

/**
* Helper for testing against ENOENT to compensate changes made to Node 7.x under Windows.
*/
function isENOENT(error) {
return isExpectedError(error, -EBADF, 'EBADF');
}

/**
* Helper to determine whether the expected error code and errno match the actual code and errno,
* which will differ between the supported node versions.
*
* - Node >= 7.0:
* error.code {String}
* error.errno {String|Number} any numerical value will be negated
*
* - Node >= 6.0 < 7.0:
* error.code {String}
* error.errno {Number} negated
*
* - Node >= 4.0 < 6.0: introduces SystemError
* error.code {String}
* error.errno {Number} negated
*
* - Node >= 0.10 < 4.0:
* error.code {Number} negated
* error.errno n/a
*/
function isExpectedError(error, code, errno) {

This comment has been minimized.

Copy link
@skrysmanski

skrysmanski Feb 26, 2017

There are two errors here:

  1. The second condition must check error.errno instead of error.code.
  2. The parameters code and errno must be swapped (see calls to this method).

The correct implementation would thus look like this:

function isExpectedError(error, errno, code) {
  return error.code == code || error.errno == errno;
}
return error.code == code || error.code == errno;
}

/**
* Sets the graceful cleanup.
*
Expand Down
10 changes: 10 additions & 0 deletions test/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,14 @@ function _testIssue62Sync(cb) {
_spawnTestWithoutError('issue62-sync.js', [], cb);
}

function _testIssue115File(cb) {
_spawnTestWithError('issue115-file.js', [], cb);
}

function _testIssue115FileSync(cb) {
_spawnTestWithError('issue115-file-sync.js', [], cb);
}

module.exports.testStat = _testStat;
module.exports.testPrefix = _testPrefix;
module.exports.testPrefixSync = _testPrefixSync;
Expand All @@ -208,5 +216,7 @@ module.exports.testName = _testName;
module.exports.testNameSync = _testNameSync;
module.exports.testUnsafeCleanup = _testUnsafeCleanup;
module.exports.testIssue62 = _testIssue62;
module.exports.testIssue115File = _testIssue115File;
module.exports.testIssue115FileSync = _testIssue115FileSync;
module.exports.testUnsafeCleanupSync = _testUnsafeCleanupSync;
module.exports.testIssue62Sync = _testIssue62Sync;
26 changes: 25 additions & 1 deletion test/file-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,30 @@ vows.describe('File creation').addBatch({
removeCallback();
assert.ok(!existsSync(name), 'File should be removed');
}
}
},

'issue115 async: user deleted tmp file prior to cleanup': {
topic: function () {
Test.testIssue115File(this.callback);
},

'should not return with an error': assert.isNull,
'should return with a name': Test.assertName,
'should not exist': function (err, name) {
assert.ok(!existsSync(name), 'File should be removed');
}
},

'issue115 sync: user deleted tmp file prior to cleanup': {
topic: function () {
Test.testIssue115FileSync(this.callback);
},

'should not return with an error': assert.isNull,
'should return with a name': Test.assertName,
'should not exist': function (err, name) {
assert.ok(!existsSync(name), 'File should be removed');
}
},

}).exportTo(module);
25 changes: 25 additions & 0 deletions test/issue115-file-sync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
var
fs = require('fs'),
join = require('path').join,
tmp = require('../lib/tmp'),
spawn = require('./spawn');

spawn.tmpFunction({ unsafeCleanup: true }, function (err, name) {
if (err) {
spawn.err(err, spawn.exit);
return;
}

try {
// creates a tmp file and then deletes it as per issue 115
// /~https://github.com/raszi/node-tmp/issues/115

tmpobj = tmp.fileSync();
fs.closeSync(tmpobj.fd);
fs.unlinkSync(tmpobj.name);
spawn.out(tmpobj.name, spawn.exit);
} catch (e) {
spawn.err(e.toString(), spawn.exit);
}
});

26 changes: 26 additions & 0 deletions test/issue115-file.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
var
fs = require('fs'),
join = require('path').join,
tmp = require('../lib/tmp'),
spawn = require('./spawn');

spawn.tmpFunction({ unsafeCleanup: true }, function (err, name) {
if (err) {
spawn.err(err, spawn.exit);
return;
}

try {
// creates a tmp file and then deletes it as per issue 115
// /~https://github.com/raszi/node-tmp/issues/115

tmp.file(function (err, name, fd, callback) {
fs.closeSync(fd);
fs.unlinkSync(name);
spawn.out(name, spawn.exit);
});
} catch (e) {
spawn.err(e.toString(), spawn.exit);
}
});

0 comments on commit 01728fb

Please sign in to comment.