From 46fd1defbabef63dbb002ccdb2883eff45ce43d9 Mon Sep 17 00:00:00 2001 From: Wraithan McCarroll Date: Tue, 29 Oct 2019 17:54:38 -0700 Subject: [PATCH 1/3] add timeout to command to catch if device isn't listening --- src/cmd/serial.js | 40 +++++++++++++++++++++++------------ src/cmd/serial.test.js | 24 ++++++++++++++++++++- test/__mocks__/serial.mock.js | 2 +- 3 files changed, 50 insertions(+), 16 deletions(-) diff --git a/src/cmd/serial.js b/src/cmd/serial.js index f0a3aa87a..7f3ce515d 100644 --- a/src/cmd/serial.js +++ b/src/cmd/serial.js @@ -23,7 +23,6 @@ const ensureError = require('../lib/utilities').ensureError; const cmd = path.basename(process.argv[1]); const arrow = chalk.green('>'); const alert = chalk.yellow('!'); -const timeoutError = 'Serial timed out'; function protip(){ const args = Array.prototype.slice.call(arguments); @@ -31,6 +30,8 @@ function protip(){ console.log.apply(null, args); } +const TIMEOUT_ERROR_MESSAGE = 'Serial timed out'; + // An LTE device may take up to 18 seconds to power up the modem const MODULE_INFO_COMMAND_TIMEOUT = 20000; const IDENTIFY_COMMAND_TIMEOUT = 20000; @@ -616,7 +617,7 @@ module.exports = class SerialCommand { return !!matches; }) .catch((err) => { - if (err !== timeoutError){ + if (err !== TIMEOUT_ERROR_MESSAGE){ throw err; } return false; @@ -897,7 +898,7 @@ module.exports = class SerialCommand { } function startTimeout(to){ - self._serialTimeout = setTimeout(() => reject('Serial timed out'), to); + self._serialTimeout = setTimeout(() => reject(TIMEOUT_ERROR_MESSAGE), to); } function resetTimeout(){ @@ -919,8 +920,22 @@ module.exports = class SerialCommand { return promise.finally(cleanUpFn); } + /** + * This is a wrapper function created so _serialWifiConfig can return the + * true promise state, but the wrapper can still act like it handled any + * failures gracefully as it acted before testing was attempted. + */ + serialWifiConfig(...args) { + return this._serialWifiConfig.apply(this, args) + .then(() => { + console.log('Done! Your device should now restart.'); + }, (err) => { + log.error('Something went wrong:', err); + }); + } + /* eslint-disable max-statements */ - serialWifiConfig(device, opts = {}){ + _serialWifiConfig(device, opts = {}){ if (!device){ return Promise.reject('No serial port available'); } @@ -1204,15 +1219,18 @@ module.exports = class SerialCommand { serialTrigger.start(true); serialPort.write('w'); serialPort.drain(); + + // In case device is not in listening mode. + startTimeout(5000, 'Serial timed out while initially listening to device, please ensure device is in listening mode with particle usb start-listening'); }); function serialClosedEarly(){ reject('Serial port closed early'); } - function startTimeout(to){ + function startTimeout(to, message = TIMEOUT_ERROR_MESSAGE){ self._serialTimeout = setTimeout(() => { - reject('Serial timed out'); + reject(message); }, to); } @@ -1275,13 +1293,7 @@ module.exports = class SerialCommand { } }); - return promise - .then(() => { - console.log('Done! Your device should now restart.'); - }, (err) => { - log.error('Something went wrong:', err); - }) - .finally(cleanUpFn); + return promise.finally(cleanUpFn); } /* eslint-enable max-statements */ @@ -1307,7 +1319,7 @@ module.exports = class SerialCommand { serialPort.pipe(parser); const failTimer = setTimeout(() => { - reject(timeoutError); + reject(TIMEOUT_ERROR_MESSAGE); }, failDelay); parser.on('data', (data) => { diff --git a/src/cmd/serial.test.js b/src/cmd/serial.test.js index 2ae8ae568..3a428c191 100644 --- a/src/cmd/serial.test.js +++ b/src/cmd/serial.test.js @@ -1,15 +1,23 @@ const MockSerial = require('../../test/__mocks__/serial.mock'); -const { expect } = require('../../test/setup'); +const { expect, sinon } = require('../../test/setup'); const SerialCommand = require('./serial'); describe('Serial Command', () => { let serial; + let clock; beforeEach(() => { serial = new SerialCommand({ params: {} }); }); + afterEach(() => { + if (clock !== undefined) { + clock.restore(); + clock = undefined; + } + }); + describe('supportsClaimCode', () => { it('can check if a device supports claiming', () => { var device = { port: 'vintage' }; @@ -53,5 +61,19 @@ describe('Serial Command', () => { }); }); }); + + describe('serialWifiConfig', () => { + it('can reject with timeout after 5000ms if not getting any serial data', () => { + clock = sinon.useFakeTimers(); + const device = { port: 'baltimore' }; + const mockSerial = new MockSerial(); + serial.serialPort = mockSerial; + const wifiPromise = serial._serialWifiConfig(device); + process.nextTick(() => { + clock.tick(5010); + }); + return expect(wifiPromise).to.be.rejected; + }); + }); }); diff --git a/test/__mocks__/serial.mock.js b/test/__mocks__/serial.mock.js index 380da1385..f7343af1d 100644 --- a/test/__mocks__/serial.mock.js +++ b/test/__mocks__/serial.mock.js @@ -15,7 +15,7 @@ MockSerial.prototype.write = function write(chunk) { this.data += chunk; }; -MockSerial.prototype.drain = function drain(cb) { +MockSerial.prototype.drain = function drain(cb = () => {}) { this.emit('drain'); process.nextTick(cb); }; From 189393452456b2f0a76427d80672ca96a8d4963b Mon Sep 17 00:00:00 2001 From: Wraithan McCarroll Date: Tue, 29 Oct 2019 18:13:32 -0700 Subject: [PATCH 2/3] reverting a style change I made and better comments in tests --- src/cmd/serial.js | 10 +++++----- src/cmd/serial.test.js | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/cmd/serial.js b/src/cmd/serial.js index 7f3ce515d..06211c656 100644 --- a/src/cmd/serial.js +++ b/src/cmd/serial.js @@ -23,6 +23,7 @@ const ensureError = require('../lib/utilities').ensureError; const cmd = path.basename(process.argv[1]); const arrow = chalk.green('>'); const alert = chalk.yellow('!'); +const timeoutError = 'Serial timed out'; function protip(){ const args = Array.prototype.slice.call(arguments); @@ -30,7 +31,6 @@ function protip(){ console.log.apply(null, args); } -const TIMEOUT_ERROR_MESSAGE = 'Serial timed out'; // An LTE device may take up to 18 seconds to power up the modem const MODULE_INFO_COMMAND_TIMEOUT = 20000; @@ -617,7 +617,7 @@ module.exports = class SerialCommand { return !!matches; }) .catch((err) => { - if (err !== TIMEOUT_ERROR_MESSAGE){ + if (err !== timeoutError){ throw err; } return false; @@ -898,7 +898,7 @@ module.exports = class SerialCommand { } function startTimeout(to){ - self._serialTimeout = setTimeout(() => reject(TIMEOUT_ERROR_MESSAGE), to); + self._serialTimeout = setTimeout(() => reject(timeoutError), to); } function resetTimeout(){ @@ -1228,7 +1228,7 @@ module.exports = class SerialCommand { reject('Serial port closed early'); } - function startTimeout(to, message = TIMEOUT_ERROR_MESSAGE){ + function startTimeout(to, message = timeoutError){ self._serialTimeout = setTimeout(() => { reject(message); }, to); @@ -1319,7 +1319,7 @@ module.exports = class SerialCommand { serialPort.pipe(parser); const failTimer = setTimeout(() => { - reject(TIMEOUT_ERROR_MESSAGE); + reject(timeoutError); }, failDelay); parser.on('data', (data) => { diff --git a/src/cmd/serial.test.js b/src/cmd/serial.test.js index 3a428c191..898dce4fa 100644 --- a/src/cmd/serial.test.js +++ b/src/cmd/serial.test.js @@ -69,6 +69,8 @@ describe('Serial Command', () => { const mockSerial = new MockSerial(); serial.serialPort = mockSerial; const wifiPromise = serial._serialWifiConfig(device); + // This allows _serialWifiConfig's internal promises to run and try to + // connect to the serial device before moving time forward. process.nextTick(() => { clock.tick(5010); }); From dffdb09cb76f340afb74e43936de8105405f31a8 Mon Sep 17 00:00:00 2001 From: Wraithan McCarroll Date: Mon, 4 Nov 2019 09:05:20 -0800 Subject: [PATCH 3/3] move to recommended async test style, minor tweaks --- src/cmd/serial.js | 13 ++++++------- src/cmd/serial.test.js | 33 ++++++++++++++++++++++++--------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/cmd/serial.js b/src/cmd/serial.js index 06211c656..a29e6bf8c 100644 --- a/src/cmd/serial.js +++ b/src/cmd/serial.js @@ -921,12 +921,11 @@ module.exports = class SerialCommand { } /** - * This is a wrapper function created so _serialWifiConfig can return the - * true promise state, but the wrapper can still act like it handled any - * failures gracefully as it acted before testing was attempted. + * This is a wrapper function so _serialWifiConfig can return the + * true promise state for testing. */ serialWifiConfig(...args) { - return this._serialWifiConfig.apply(this, args) + return this._serialWifiConfig(...args) .then(() => { console.log('Done! Your device should now restart.'); }, (err) => { @@ -1221,16 +1220,16 @@ module.exports = class SerialCommand { serialPort.drain(); // In case device is not in listening mode. - startTimeout(5000, 'Serial timed out while initially listening to device, please ensure device is in listening mode with particle usb start-listening'); + startTimeout(5000, 'Serial timed out while initially listening to device, please ensure device is in listening mode with particle usb start-listening', 'InitialTimeoutError'); }); function serialClosedEarly(){ reject('Serial port closed early'); } - function startTimeout(to, message = timeoutError){ + function startTimeout(to, message = timeoutError, name = 'TimeoutError'){ self._serialTimeout = setTimeout(() => { - reject(message); + reject(VError({name}, message)); }, to); } diff --git a/src/cmd/serial.test.js b/src/cmd/serial.test.js index 898dce4fa..e0505d9f9 100644 --- a/src/cmd/serial.test.js +++ b/src/cmd/serial.test.js @@ -62,19 +62,34 @@ describe('Serial Command', () => { }); }); - describe('serialWifiConfig', () => { - it('can reject with timeout after 5000ms if not getting any serial data', () => { + describe('serialWifiConfig', async () => { + it('can reject with timeout after 5000ms if not getting any serial data', async () => { clock = sinon.useFakeTimers(); const device = { port: 'baltimore' }; const mockSerial = new MockSerial(); + + mockSerial.write = function write(data) { + if (data === 'w') { + // This next tick allows _serialWifiConfig to set the timeout before we move the clock foward. + process.nextTick(() => { + clock.tick(5010); + }); + } + }; + serial.serialPort = mockSerial; - const wifiPromise = serial._serialWifiConfig(device); - // This allows _serialWifiConfig's internal promises to run and try to - // connect to the serial device before moving time forward. - process.nextTick(() => { - clock.tick(5010); - }); - return expect(wifiPromise).to.be.rejected; + + + let error; + + try { + await serial._serialWifiConfig(device); + } catch (e) { + error = e; + } + + expect(error).to.be.an.instanceOf(Error); + expect(error).to.have.property('name', 'InitialTimeoutError'); }); }); });