From f55400ec98aa39b2497c5f0e20d86334720d50f3 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Fri, 13 Nov 2015 14:35:58 -0400 Subject: [PATCH] Use Bluebird instead of native promises Native promises showed strange behaviour from time to time. --- .jshintrc | 2 +- lib/src/dialog.js | 1 + lib/src/drives.js | 36 +--------- lib/src/writer.js | 34 ++------- package.json | 1 + tests/src/dialog.spec.js | 8 +-- tests/src/drives.spec.js | 147 +++++++++------------------------------ tests/src/writer.spec.js | 45 ------------ 8 files changed, 45 insertions(+), 229 deletions(-) diff --git a/.jshintrc b/.jshintrc index 5878e739..582c1bb2 100644 --- a/.jshintrc +++ b/.jshintrc @@ -84,7 +84,7 @@ "couch" : false, // Enable globals exposed by CouchDB. "devel" : false, // Allow development statements e.g. `console.log();`. "dojo" : false, // Enable globals exposed by Dojo Toolkit. - "esnext" : true, // Enable globals exposed by ES6. + "esnext" : false, // Enable globals exposed by ES6. "mocha" : true, // Enable globals exposed by Mocha. "jquery" : false, // Enable globals exposed by jQuery JavaScript library. "mootools" : false, // Enable globals exposed by MooTools JavaScript framework. diff --git a/lib/src/dialog.js b/lib/src/dialog.js index a822d54c..796c9426 100644 --- a/lib/src/dialog.js +++ b/lib/src/dialog.js @@ -22,6 +22,7 @@ */ var dialog = require('dialog'); +var Promise = require('bluebird'); var _ = require('lodash'); /** diff --git a/lib/src/drives.js b/lib/src/drives.js index ab7f80a0..72e1bb6c 100644 --- a/lib/src/drives.js +++ b/lib/src/drives.js @@ -21,38 +21,8 @@ * THE SOFTWARE. */ -var drivelist = require('drivelist'); - -/** - * @summary List all available drives - * @function - * @public - * - * @description - * See https://github.com/resin-io/drivelist - * - * @fulfil {Object[]} - available drives - * @returns {Promise} - * - * @example - * drives.list().then(function(drives) { - * drives.forEach(function(drive) { - * console.log(drive.device); - * }); - * }); - */ -exports.list = function() { - 'use strict'; - - return new Promise(function(resolve, reject) { - drivelist.list(function(error, drives) { - if (error) { - return reject(error); - } - return resolve(drives); - }); - }); -}; +var Promise = require('bluebird'); +var drivelist = Promise.promisifyAll(require('drivelist')); /** * @summary List all available removable drives @@ -72,7 +42,7 @@ exports.list = function() { exports.listRemovable = function() { 'use strict'; - return exports.list().then(function(drives) { + return drivelist.listAsync().then(function(drives) { return drives.filter(function(drive) { return !drive.system; }); diff --git a/lib/src/writer.js b/lib/src/writer.js index 68526d9c..119642be 100644 --- a/lib/src/writer.js +++ b/lib/src/writer.js @@ -22,7 +22,8 @@ */ var imageWrite = require('resin-image-write'); -var umount = require('umount'); +var Promise = require('bluebird'); +var umount = Promise.promisifyAll(require('umount')); var fs = require('fs'); /** @@ -48,33 +49,6 @@ exports.getImageStream = function(image) { return stream; }; -/** - * @summary Unmount a drive - * @function - * @private - * - * @param {String} disk - disk device - * @returns {Promise} - * - * @example - * writer.unmountDisk('/dev/disk2').then(function() { - * console.log('Disk unmounted!'); - * }); - */ -exports.unmountDisk = function(disk) { - 'use strict'; - - return new Promise(function(resolve, reject) { - umount.umount(disk, function(error) { - if (error) { - return reject(error); - } - - return resolve(); - }); - }); -}; - /** * @summary Write an image to a disk drive * @function @@ -100,7 +74,7 @@ exports.unmountDisk = function(disk) { exports.writeImage = function(image, drive, onProgress) { 'use strict'; - return exports.unmountDisk(drive).then(function() { + return umount.umountAsync(drive).then(function() { var stream = exports.getImageStream(image); return imageWrite.write(drive, stream); }).then(function(writer) { @@ -110,6 +84,6 @@ exports.writeImage = function(image, drive, onProgress) { writer.on('done', resolve); }); }).then(function() { - return exports.unmountDisk(drive); + return umount.umountAsync(drive); }); }; diff --git a/package.json b/package.json index abf5de85..887db92a 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "dependencies": { "angular": "^1.4.6", "angular-ui-bootstrap": "^0.14.2", + "bluebird": "^3.0.5", "bootstrap-sass": "^3.3.5", "drivelist": "^2.0.0", "electron-window": "^0.6.0", diff --git a/tests/src/dialog.spec.js b/tests/src/dialog.spec.js index 0a002b03..0389538c 100644 --- a/tests/src/dialog.spec.js +++ b/tests/src/dialog.spec.js @@ -18,11 +18,9 @@ describe('Dialog:', function() { this.showOpenDialogStub.restore(); }); - it('should eventually equal the file', function(done) { - dialog.selectImage().then(function(image) { - m.chai.expect(image).to.equal('foo/bar'); - done(); - }).catch(done); + it('should eventually equal the file', function() { + var promise = dialog.selectImage(); + m.chai.expect(promise).to.eventually.equal('foo/bar'); }); }); diff --git a/tests/src/drives.spec.js b/tests/src/drives.spec.js index 1715767b..f089758f 100644 --- a/tests/src/drives.spec.js +++ b/tests/src/drives.spec.js @@ -1,91 +1,17 @@ var m = require('mochainon'); +var Promise = require('bluebird'); var drivelist = require('drivelist'); var drives = require('../../lib/src/drives'); describe('Drives:', function() { 'use strict'; - describe('.list()', function() { - - describe('given no available drives', function() { - - beforeEach(function() { - this.drivelistListStub = m.sinon.stub(drivelist, 'list'); - this.drivelistListStub.yields(null, []); - }); - - afterEach(function() { - this.drivelistListStub.restore(); - }); - - it('should eventually equal an empty array', function(done) { - drives.list().then(function(drives) { - m.chai.expect(drives).to.deep.equal([]); - done(); - }).catch(done); - }); - - }); - - describe('given available drives', function() { - - beforeEach(function() { - this.drives = [ - { - device: '/dev/sda', - description: 'WDC WD10JPVX-75J', - size: '931.5G', - mountpoint: '/', - system: true - } - ]; - - this.drivelistListStub = m.sinon.stub(drivelist, 'list'); - this.drivelistListStub.yields(null, this.drives); - }); - - afterEach(function() { - this.drivelistListStub.restore(); - }); - - it('should eventually equal the drives', function(done) { - drives.list().then(function(drives) { - m.chai.expect(drives).to.deep.equal(this.drives); - done(); - }.bind(this)).catch(done); - }); - - }); - - describe('given an error when listing the drives', function() { - - beforeEach(function() { - this.drivelistListStub = m.sinon.stub(drivelist, 'list'); - this.drivelistListStub.yields(new Error('scan error')); - }); - - afterEach(function() { - this.drivelistListStub.restore(); - }); - - it('should be rejected with the error', function(done) { - drives.list().catch(function(error) { - m.chai.expect(error).to.be.an.instanceof(Error); - m.chai.expect(error.message).to.equal('scan error'); - return done(); - }).catch(done); - }); - - }); - - }); - describe('.listRemovable()', function() { describe('given no available drives', function() { beforeEach(function() { - this.drivesListStub = m.sinon.stub(drives, 'list'); + this.drivesListStub = m.sinon.stub(drivelist, 'listAsync'); this.drivesListStub.returns(Promise.resolve([])); }); @@ -93,11 +19,9 @@ describe('Drives:', function() { this.drivesListStub.restore(); }); - it('should eventually equal an empty array', function(done) { - drives.listRemovable().then(function(drives) { - m.chai.expect(drives).to.deep.equal([]); - done(); - }).catch(done); + it('should eventually equal an empty array', function() { + var promise = drives.listRemovable(); + m.chai.expect(promise).to.eventually.become([]); }); }); @@ -115,7 +39,7 @@ describe('Drives:', function() { } ]; - this.drivesListStub = m.sinon.stub(drives, 'list'); + this.drivesListStub = m.sinon.stub(drivelist, 'listAsync'); this.drivesListStub.returns(Promise.resolve(this.drives)); }); @@ -123,11 +47,9 @@ describe('Drives:', function() { this.drivesListStub.restore(); }); - it('should eventually equal an empty array', function(done) { - drives.listRemovable().then(function(drives) { - m.chai.expect(drives).to.deep.equal([]); - done(); - }).catch(done); + it('should eventually equal an empty array', function() { + var promise = drives.listRemovable(); + m.chai.expect(promise).to.eventually.become([]); }); }); @@ -159,7 +81,7 @@ describe('Drives:', function() { } ]; - this.drivesListStub = m.sinon.stub(drives, 'list'); + this.drivesListStub = m.sinon.stub(drivelist, 'listAsync'); this.drivesListStub.returns(Promise.resolve(this.drives)); }); @@ -167,26 +89,24 @@ describe('Drives:', function() { this.drivesListStub.restore(); }); - it('should eventually become the removable drives', function(done) { - drives.listRemovable().then(function(drives) { - m.chai.expect(drives).to.deep.equal([ - { - device: '/dev/sdb', - description: 'Foo', - size: '14G', - mountpoint: '/mnt/foo', - system: false - }, - { - device: '/dev/sdc', - description: 'Bar', - size: '14G', - mountpoint: '/mnt/bar', - system: false - } - ]); - done(); - }).catch(done); + it('should eventually become the removable drives', function() { + var promise = drives.listRemovable(); + m.chai.expect(promise).to.eventually.become([ + { + device: '/dev/sdb', + description: 'Foo', + size: '14G', + mountpoint: '/mnt/foo', + system: false + }, + { + device: '/dev/sdc', + description: 'Bar', + size: '14G', + mountpoint: '/mnt/bar', + system: false + } + ]); }); }); @@ -194,7 +114,7 @@ describe('Drives:', function() { describe('given an error when listing the drives', function() { beforeEach(function() { - this.drivesListStub = m.sinon.stub(drives, 'list'); + this.drivesListStub = m.sinon.stub(drivelist, 'listAsync'); this.drivesListStub.returns(Promise.reject(new Error('scan error'))); }); @@ -202,12 +122,9 @@ describe('Drives:', function() { this.drivesListStub.restore(); }); - it('should be rejected with the error', function(done) { - drives.listRemovable().catch(function(error) { - m.chai.expect(error).to.be.an.instanceof(Error); - m.chai.expect(error.message).to.equal('scan error'); - return done(); - }).catch(done); + it('should be rejected with the error', function() { + var promise = drives.listRemovable(); + m.chai.expect(promise).to.be.rejectedWith('scan error'); }); }); diff --git a/tests/src/writer.spec.js b/tests/src/writer.spec.js index c5dc5a24..1b351bd4 100644 --- a/tests/src/writer.spec.js +++ b/tests/src/writer.spec.js @@ -29,49 +29,4 @@ describe('Writer:', function() { }); - describe('.unmountDisk()', function() { - - describe('given a successful unmount', function() { - - beforeEach(function() { - this.umountStub = m.sinon.stub(umount, 'umount'); - this.umountStub.yields(null); - }); - - afterEach(function() { - this.umountStub.restore(); - }); - - it('should eventually resolve undefined', function(done) { - writer.unmountDisk('/dev/disk2').then(function() { - m.chai.expect(arguments[0]).to.be.undefined; - done(); - }).catch(done); - }); - - }); - - describe('given an unsuccessful unmount', function() { - - beforeEach(function() { - this.umountStub = m.sinon.stub(umount, 'umount'); - this.umountStub.yields(new Error('unmount error')); - }); - - afterEach(function() { - this.umountStub.restore(); - }); - - it('should be rejected with the error', function(done) { - writer.unmountDisk('/dev/disk2').catch(function(error) { - m.chai.expect(error).to.be.an.instanceof(Error); - m.chai.expect(error.message).to.equal('unmount error'); - done(); - }).catch(done); - }); - - }); - - }); - });