Duplicate resolution of promises within util
Opened this issue · 2 comments
matmar10 commented
Several areas of the lib/utils.js
have duplicate resolution of promises - where an error calls BOTH reject
AND resolve
:
Example:
return new Promise((resolve, reject) => {
fs.readFile(file, 'utf8', function(err, data) {
if (err) {
reject(err);
}
resolve(data);
});
});
This will BOTH reject and resolve the error.
Also, the test cases are ambiguously written. It's not clear what behavior is actually expected:
it('should handle glob error', function(done) {
var files = new Error('custom: glob error');
utils.readFileGlobs(files)
.then(function() {
done('readFileGlobs did not throw error');
})
.catch(function(err) {
expect(err.message).to.equal('custom: glob error');
done();
});
});
Because the done()
method accepts an error. Fro the Mocha docs:
This callback accepts both an Error instance (or subclass thereof) or a falsy value; anything else is invalid usage and throws an error (usually causing a failed test).
matmar10 commented
straker commented
Ya for the longest time I just saw any string passed to done()
threw the error with the string as the message, so never bothered to turn it into an actual Error. So any time a done()
is called with a string that should be a failed test case and should technically be done(new Error(msg))
.
For the code, it should probably be return reject(data)
and I forgot the return.