Skip to content

Commit 1262840

Browse files
silkentrancesilkentrance
committed
fix #155
fix existing async tests, finally
1 parent cf037ec commit 1262840

File tree

12 files changed

+309
-116
lines changed

12 files changed

+309
-116
lines changed

‎lib/tmp.js

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,16 @@ function fileSync(options) {
284284
};
285285
}
286286

287+
/**
288+
* Asynchronously removes files and folders in a directory recursively.
289+
*
290+
* @param {string} root
291+
* @private
292+
*/
293+
function _rmdirRecursive(root, next) {
294+
next(new Error('not implemented yet'));
295+
}
296+
287297
/**
288298
* Removes files and folders in a directory recursively.
289299
*
@@ -377,7 +387,26 @@ function dirSync(options) {
377387
* @private
378388
*/
379389
function _prepareTmpFileRemoveCallback(name, fd, opts) {
380-
const removeCallback = _prepareRemoveCallback(function _removeCallback(fdPath) {
390+
const removeFunction = function _removeFunction(fdPath, next) {
391+
const _handler = function (err) {
392+
if (err && !isENOENT(err)) {
393+
// reraise any unanticipated error
394+
if (next) {
395+
return next(err);
396+
}
397+
throw e;
398+
}
399+
next(null);
400+
}
401+
402+
if (0 <= fdPath[0])
403+
fs.close(fdPath[0], function (err) {
404+
fs.unlink(fdPath[1], _handler);
405+
});
406+
else fs.unlink(fdPath[1], _handler);
407+
}
408+
409+
const removeFunctionSync = function _removeFunctionSync(fdPath) {
381410
try {
382411
if (0 <= fdPath[0]) {
383412
fs.closeSync(fdPath[0]);
@@ -401,10 +430,13 @@ function _prepareTmpFileRemoveCallback(name, fd, opts) {
401430
throw e;
402431
}
403432
}
404-
}, [fd, name]);
433+
}
434+
435+
const removeCallbackSync = _prepareRemoveCallback(removeFunctionSync, [fd, name]);
436+
const removeCallback = _prepareRemoveCallback(removeFunction, [fd, name], removeCallbackSync);
405437

406438
if (!opts.keep) {
407-
_removeObjects.unshift(removeCallback);
439+
_removeObjects.unshift(removeCallbackSync);
408440
}
409441

410442
return removeCallback;
@@ -419,11 +451,13 @@ function _prepareTmpFileRemoveCallback(name, fd, opts) {
419451
* @private
420452
*/
421453
function _prepareTmpDirRemoveCallback(name, opts) {
422-
const removeFunction = opts.unsafeCleanup ? _rmdirRecursiveSync : fs.rmdirSync.bind(fs);
423-
const removeCallback = _prepareRemoveCallback(removeFunction, name);
454+
const removeFunction = opts.unsafeCleanup ? _rmdirRecursive : fs.rmdir.bind(fs);
455+
const removeFunctionSync = opts.unsafeCleanup ? _rmdirRecursiveSync : fs.rmdirSync.bind(fs);
456+
const removeCallbackSync = _prepareRemoveCallback(removeFunctionSync, name);
457+
const removeCallback = _prepareRemoveCallback(removeFunction, name, removeCallbackSync);
424458

425459
if (!opts.keep) {
426-
_removeObjects.unshift(removeCallback);
460+
_removeObjects.unshift(removeCallbackSync);
427461
}
428462

429463
return removeCallback;
@@ -437,21 +471,32 @@ function _prepareTmpDirRemoveCallback(name, opts) {
437471
* @returns {Function}
438472
* @private
439473
*/
440-
function _prepareRemoveCallback(removeFunction, arg) {
474+
function _prepareRemoveCallback(removeFunction, arg, cleanupCallbackSync) {
441475
var called = false;
442476

443477
return function _cleanupCallback(next) {
444478
if (!called) {
445-
const index = _removeObjects.indexOf(_cleanupCallback);
479+
const toRemove = cleanupCallbackSync || _cleanupCallback;
480+
const index = _removeObjects.indexOf(toRemove);
446481
if (index >= 0) {
447482
_removeObjects.splice(index, 1);
448483
}
449484

450485
called = true;
451-
removeFunction(arg);
452-
}
453-
454-
if (next) next(null);
486+
// sync?
487+
if (removeFunction.length == 1) {
488+
try {
489+
removeFunction(arg);
490+
if (next) return next(null);
491+
}
492+
catch (err) {
493+
// if no next is provided and since we are
494+
// in silent cleanup mode on process exit,
495+
// we will ignore the error
496+
if (next) return next(err);
497+
}
498+
} else return removeFunction(arg, next);
499+
} else if (next) return next(new Error('cleanup callback has already been called'));
455500
};
456501
}
457502

‎lib/wip.scratch

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
2+
if (opts.name) {
3+
cb(null, _generateTmpName(opts));
4+
} else {
5+
do {
6+
const name = _generateTmpName(opts);
7+
// check whether the path exists then retry if needed
8+
fs.stat(name, function (err) {
9+
if (err && tries == 0) {
10+
return cb(new Error('Could not get a unique tmp filename, max tries reached ' + name));
11+
} else {
12+
cb(null, name);
13+
}
14+
});
15+
} while (tries-- > 0)
16+
}
17+
18+

‎package-lock.json

Lines changed: 10 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@
2525
"os-tmpdir": "~1.0.2"
2626
},
2727
"devDependencies": {
28-
"mocha": "~3.4.2"
28+
"mocha": "~3.4.2",
29+
"rimraf": "^2.6.2"
2930
},
3031
"main": "lib/tmp.js",
3132
"files": [

‎test/dir-sync-test.js

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ var
88
inbandStandardTests = require('./inband-standard'),
99
childProcess = require('./child-process').genericChildProcess,
1010
assertions = require('./assertions'),
11+
rimraf = require('rimraf'),
1112
tmp = require('../lib/tmp');
1213

1314

@@ -56,68 +57,82 @@ describe('tmp', function () {
5657
it('on graceful cleanup', function (done) {
5758
childProcess('graceful-dir-sync.json', function (err, stderr, stdout) {
5859
if (err) return done(err);
59-
else if (!stderr) assert.fail('stderr expected');
60-
else assertions.assertDoesNotExist(stdout);
60+
else if (!stderr) return done(new Error('stderr expected'));
61+
try {
62+
assertions.assertDoesNotExist(stdout);
63+
} catch (err) {
64+
rimraf.sync(stdout);
65+
return done(err);
66+
}
6167
done();
6268
});
6369
});
6470
it('on non graceful cleanup', function (done) {
6571
childProcess('non-graceful-dir-sync.json', function (err, stderr, stdout) {
6672
if (err) return done(err);
67-
else if (!stderr) assert.fail('stderr expected');
68-
else {
73+
else if (!stderr) return done(new Error('stderr expected'));
74+
try {
6975
assertions.assertExists(stdout);
70-
fs.rmdirSync(stdout);
76+
rimraf.sync(stdout);
77+
} catch (err) {
78+
return done(err);
7179
}
7280
done();
7381
});
7482
});
7583
it('on keep', function (done) {
7684
childProcess('keep-dir-sync.json', function (err, stderr, stdout) {
7785
if (err) return done(err);
78-
else if (stderr) assert.fail(stderr);
79-
else {
86+
else if (stderr) return done(new Error(stderr));
87+
try {
8088
assertions.assertExists(stdout);
81-
fs.rmdirSync(stdout);
89+
rimraf.sync(stdout);
90+
} catch (err) {
91+
return done(err);
8292
}
8393
done();
8494
});
8595
});
8696
it('on unlink (keep == false)', function (done) {
8797
childProcess('unlink-dir-sync.json', function (err, stderr, stdout) {
8898
if (err) return done(err);
89-
else if (stderr) assert.fail(stderr);
90-
else assertions.assertDoesNotExist(stdout);
99+
else if (stderr) return done(new Error(stderr));
100+
try {
101+
assertions.assertDoesNotExist(stdout);
102+
} catch (err) {
103+
rimraf.sync(stdout);
104+
return done(err);
105+
}
91106
done();
92107
});
93108
});
94109
it('on unsafe cleanup', function (done) {
95110
childProcess('unsafe-sync.json', function (err, stderr, stdout) {
96111
if (err) return done(err);
97-
else if (stderr) assert.fail(stderr);
98-
else {
112+
else if (stderr) return done(new Error(stderr));
113+
try {
99114
assertions.assertDoesNotExist(stdout);
100-
var basepath = path.join(__dirname, 'outband', 'fixtures', 'symlinkme');
101-
assertions.assertExists(basepath);
102-
assertions.assertExists(path.join(basepath, 'file.js'), true);
115+
} catch (err) {
116+
rimraf.sync(stdout);
117+
return done(err);
103118
}
104119
done();
105120
});
106121
});
107122
it('on non unsafe cleanup', function (done) {
108123
childProcess('non-unsafe-sync.json', function (err, stderr, stdout) {
109124
if (err) return done(err);
110-
else if (stderr) assert.fail(stderr);
111-
else {
125+
else if (stderr) return done(new Error(stderr));
126+
try {
112127
assertions.assertExists(stdout);
113128
assertions.assertExists(path.join(stdout, 'should-be-removed.file'), true);
114129
if (process.platform == 'win32')
115130
assertions.assertExists(path.join(stdout, 'symlinkme-target'), true);
116131
else
117132
assertions.assertExists(path.join(stdout, 'symlinkme-target'));
118-
fs.unlinkSync(path.join(stdout, 'should-be-removed.file'));
119-
fs.unlinkSync(path.join(stdout, 'symlinkme-target'));
120-
fs.rmdirSync(stdout);
133+
rimraf.sync(stdout);
134+
} catch (err) {
135+
return done(err);
121136
}
122137
done();
123138
});
@@ -130,8 +145,13 @@ describe('tmp', function () {
130145
it('on issue #62', function (done) {
131146
childProcess('issue62-sync.json', function (err, stderr, stdout) {
132147
if (err) return done(err);
133-
else if (stderr) assert.fail(stderr);
134-
else assertions.assertDoesNotExist(stdout);
148+
else if (stderr) return done(new Error(stderr));
149+
try {
150+
assertions.assertDoesNotExist(stdout);
151+
} catch (err) {
152+
rimraf.sync(stdout);
153+
return done(err);
154+
}
135155
done();
136156
});
137157
});

0 commit comments

Comments
 (0)