From 33ffe6230366768fcddc56afea499791b634e3b4 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Tue, 21 Mar 2017 17:26:34 -0700 Subject: [PATCH 01/16] Create SetupChecker --- .../src/lib/kits/maker/ui/BoardSetupCheck.jsx | 25 +++++--------- apps/src/lib/kits/maker/util/SetupChecker.js | 33 +++++++++++++++++++ apps/src/sites/studio/pages/maker/setup.js | 7 +++- 3 files changed, 48 insertions(+), 17 deletions(-) create mode 100644 apps/src/lib/kits/maker/util/SetupChecker.js diff --git a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx index 8624738c8dba8..f80d0d6cc4d19 100644 --- a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx +++ b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx @@ -1,7 +1,8 @@ /** @file Maker Board setup checker */ -import React, {Component} from 'react'; +import React, {Component, PropTypes} from 'react'; import CircuitPlaygroundBoard from '../CircuitPlaygroundBoard'; import {ensureAppInstalled, findPortWithViableDevice} from '../portScanning'; +import SetupChecker from '../util/SetupChecker'; import SetupStep, { HIDDEN, WAITING, @@ -32,6 +33,10 @@ const initialState = { export default class BoardSetupCheck extends Component { state = {...initialState}; + static propTypes = { + setupChecker: PropTypes.instanceOf(SetupChecker) + }; + hide(selector) { this.setState({[selector]: HIDDEN}); } @@ -113,19 +118,11 @@ export default class BoardSetupCheck extends Component { } detectChromeVersion() { + const {setupChecker} = this.props; this.spin(STATUS_IS_CHROME); return promiseWaitFor(200) - .then(() => { - if (!isChrome()) { - return Promise.reject(new Error('Not using Chrome')); - } - - if (!gtChrome33()) { - return Promise.reject(new Error('Not using Chrome > v33')); - } - - this.succeed(STATUS_IS_CHROME); - }) + .then(() => setupChecker.detectChromeVersion()) + .then(() => this.succeed(STATUS_IS_CHROME)) .catch(error => { this.fail(STATUS_IS_CHROME); return Promise.reject(error); @@ -323,10 +320,6 @@ function getChromeVersion() { return raw ? parseInt(raw[2], 10) : false; } -function gtChrome33() { - return getChromeVersion() >= 33; -} - function isWindows() { return navigator.platform.indexOf('Win') > -1; } diff --git a/apps/src/lib/kits/maker/util/SetupChecker.js b/apps/src/lib/kits/maker/util/SetupChecker.js new file mode 100644 index 0000000000000..d29075b76c1f3 --- /dev/null +++ b/apps/src/lib/kits/maker/util/SetupChecker.js @@ -0,0 +1,33 @@ +/** @file Stubbable core setup check behavior for the setup page. */ + +export default class SetupChecker { + /** + * Resolve if using Chrome > 33 + * @returns {Promise} + */ + detectChromeVersion() { + return new Promise((resolve, reject) => { + if (!isChrome()) { + reject(new Error('Not using Chrome')); + } if (!gtChrome33()) { + reject(new Error('Not using Chrome > v33')); + } else { + resolve(); + } + }); + } +} + +// TODO deduplicate these helpers into yet another util file +function gtChrome33() { + return getChromeVersion() >= 33; +} + +function isChrome() { + return !!window.chrome; +} + +function getChromeVersion() { + const raw = navigator.userAgent.match(/Chrom(e|ium)\/([0-9]+)\./); + return raw ? parseInt(raw[2], 10) : false; +} diff --git a/apps/src/sites/studio/pages/maker/setup.js b/apps/src/sites/studio/pages/maker/setup.js index 2deefda0d4d23..a94d929e38714 100644 --- a/apps/src/sites/studio/pages/maker/setup.js +++ b/apps/src/sites/studio/pages/maker/setup.js @@ -1,8 +1,13 @@ import React from 'react'; import ReactDOM from 'react-dom'; import BoardSetupCheck from '@cdo/apps/lib/kits/maker/ui/BoardSetupCheck'; +import SetupChecker from '@cdo/apps/lib/kits/maker/util/SetupChecker'; $(function () { - ReactDOM.render(, document.getElementById('setup-status-mount')); + const setupChecker = new SetupChecker(); + ReactDOM.render( + , + document.getElementById('setup-status-mount') + ); $('.maker-setup a').attr('target', '_blank'); }); From dc90c2f2f4741ef876746edfba09ba3b17e91a2a Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Tue, 21 Mar 2017 17:32:01 -0700 Subject: [PATCH 02/16] Move detectChromeAppInstalled --- apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx | 5 +++-- apps/src/lib/kits/maker/util/SetupChecker.js | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx index f80d0d6cc4d19..624b0123916bc 100644 --- a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx +++ b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx @@ -1,7 +1,7 @@ /** @file Maker Board setup checker */ import React, {Component, PropTypes} from 'react'; import CircuitPlaygroundBoard from '../CircuitPlaygroundBoard'; -import {ensureAppInstalled, findPortWithViableDevice} from '../portScanning'; +import {findPortWithViableDevice} from '../portScanning'; import SetupChecker from '../util/SetupChecker'; import SetupStep, { HIDDEN, @@ -133,9 +133,10 @@ export default class BoardSetupCheck extends Component { * @return {Promise} */ detectChromeAppInstalled() { + const {setupChecker} = this.props; this.spin(STATUS_APP_INSTALLED); return promiseWaitFor(200) - .then(ensureAppInstalled) + .then(() => setupChecker.detectChromeAppInstalled()) .then(() => this.succeed(STATUS_APP_INSTALLED)) .catch(error => { this.fail(STATUS_APP_INSTALLED); diff --git a/apps/src/lib/kits/maker/util/SetupChecker.js b/apps/src/lib/kits/maker/util/SetupChecker.js index d29075b76c1f3..c6288ba795013 100644 --- a/apps/src/lib/kits/maker/util/SetupChecker.js +++ b/apps/src/lib/kits/maker/util/SetupChecker.js @@ -1,4 +1,5 @@ /** @file Stubbable core setup check behavior for the setup page. */ +import {ensureAppInstalled} from '../portScanning'; export default class SetupChecker { /** @@ -16,6 +17,13 @@ export default class SetupChecker { } }); } + + /** + * Resolve if the Chrome Connector App is installed. + */ + detectChromeAppInstalled() { + return ensureAppInstalled(); + } } // TODO deduplicate these helpers into yet another util file From e6984c7b40262a036755c05cf147912810a57c26 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Tue, 21 Mar 2017 17:35:21 -0700 Subject: [PATCH 03/16] Extract browser detection helpers --- apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx | 14 +------------- apps/src/lib/kits/maker/util/SetupChecker.js | 15 +-------------- apps/src/lib/kits/maker/util/browserChecks.js | 18 ++++++++++++++++++ 3 files changed, 20 insertions(+), 27 deletions(-) create mode 100644 apps/src/lib/kits/maker/util/browserChecks.js diff --git a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx index 624b0123916bc..c89d8f07cbfce 100644 --- a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx +++ b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx @@ -3,6 +3,7 @@ import React, {Component, PropTypes} from 'react'; import CircuitPlaygroundBoard from '../CircuitPlaygroundBoard'; import {findPortWithViableDevice} from '../portScanning'; import SetupChecker from '../util/SetupChecker'; +import {isWindows, isChrome, getChromeVersion} from '../util/browserChecks'; import SetupStep, { HIDDEN, WAITING, @@ -311,16 +312,3 @@ function promiseWaitFor(ms) { setTimeout(resolve, ms); }); } - -function isChrome() { - return !!window.chrome; -} - -function getChromeVersion() { - const raw = navigator.userAgent.match(/Chrom(e|ium)\/([0-9]+)\./); - return raw ? parseInt(raw[2], 10) : false; -} - -function isWindows() { - return navigator.platform.indexOf('Win') > -1; -} diff --git a/apps/src/lib/kits/maker/util/SetupChecker.js b/apps/src/lib/kits/maker/util/SetupChecker.js index c6288ba795013..16137dcd3e9d3 100644 --- a/apps/src/lib/kits/maker/util/SetupChecker.js +++ b/apps/src/lib/kits/maker/util/SetupChecker.js @@ -1,5 +1,6 @@ /** @file Stubbable core setup check behavior for the setup page. */ import {ensureAppInstalled} from '../portScanning'; +import {isChrome, gtChrome33} from './browserChecks'; export default class SetupChecker { /** @@ -25,17 +26,3 @@ export default class SetupChecker { return ensureAppInstalled(); } } - -// TODO deduplicate these helpers into yet another util file -function gtChrome33() { - return getChromeVersion() >= 33; -} - -function isChrome() { - return !!window.chrome; -} - -function getChromeVersion() { - const raw = navigator.userAgent.match(/Chrom(e|ium)\/([0-9]+)\./); - return raw ? parseInt(raw[2], 10) : false; -} diff --git a/apps/src/lib/kits/maker/util/browserChecks.js b/apps/src/lib/kits/maker/util/browserChecks.js new file mode 100644 index 0000000000000..418c400d03ac6 --- /dev/null +++ b/apps/src/lib/kits/maker/util/browserChecks.js @@ -0,0 +1,18 @@ +/** @file Some misc. browser check methods for maker */ + +export function gtChrome33() { + return getChromeVersion() >= 33; +} + +export function isChrome() { + return !!window.chrome; +} + +export function getChromeVersion() { + const raw = navigator.userAgent.match(/Chrom(e|ium)\/([0-9]+)\./); + return raw ? parseInt(raw[2], 10) : false; +} + +export function isWindows() { + return navigator.platform.indexOf('Win') > -1; +} From 8d7c9b92a225fabc2f505a3fd80ddc42b14c4e0a Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Tue, 21 Mar 2017 17:40:06 -0700 Subject: [PATCH 04/16] Deduplicate step wrapper --- .../src/lib/kits/maker/ui/BoardSetupCheck.jsx | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx index c89d8f07cbfce..9a67f5c665a21 100644 --- a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx +++ b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx @@ -118,31 +118,38 @@ export default class BoardSetupCheck extends Component { }); } - detectChromeVersion() { - const {setupChecker} = this.props; - this.spin(STATUS_IS_CHROME); + /** + * Perform the work to check a step, wrapped in appropriate status changes. + * @param {string} stepKey + * @param {function:Promise} stepWork + * @return {Promise} + */ + detectStep(stepKey, stepWork) { + this.spin(stepKey); return promiseWaitFor(200) - .then(() => setupChecker.detectChromeVersion()) - .then(() => this.succeed(STATUS_IS_CHROME)) + .then(stepWork) + .then(() => this.succeed(stepKey)) .catch(error => { - this.fail(STATUS_IS_CHROME); + this.fail(stepKey); return Promise.reject(error); }); } + detectChromeVersion() { + const {setupChecker} = this.props; + return this.detectStep( + STATUS_IS_CHROME, + () => setupChecker.detectChromeVersion()); + } + /** * @return {Promise} */ detectChromeAppInstalled() { const {setupChecker} = this.props; - this.spin(STATUS_APP_INSTALLED); - return promiseWaitFor(200) - .then(() => setupChecker.detectChromeAppInstalled()) - .then(() => this.succeed(STATUS_APP_INSTALLED)) - .catch(error => { - this.fail(STATUS_APP_INSTALLED); - return Promise.reject(error); - }); + return this.detectStep( + STATUS_APP_INSTALLED, + () => setupChecker.detectChromeAppInstalled()); } /** From 14b8900dc1ef1cf49c3223dcc93aa761198137cd Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Tue, 21 Mar 2017 17:45:49 -0700 Subject: [PATCH 05/16] Move detectBoardPluggedIn to SetupChecker --- .../src/lib/kits/maker/ui/BoardSetupCheck.jsx | 26 ++++++------------- apps/src/lib/kits/maker/util/SetupChecker.js | 7 ++++- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx index 9a67f5c665a21..265dfd6b3173f 100644 --- a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx +++ b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx @@ -1,7 +1,6 @@ /** @file Maker Board setup checker */ import React, {Component, PropTypes} from 'react'; import CircuitPlaygroundBoard from '../CircuitPlaygroundBoard'; -import {findPortWithViableDevice} from '../portScanning'; import SetupChecker from '../util/SetupChecker'; import {isWindows, isChrome, getChromeVersion} from '../util/browserChecks'; import SetupStep, { @@ -73,7 +72,6 @@ export default class BoardSetupCheck extends Component { this.hide(STATUS_WINDOWS_DRIVERS); } - let portName = null; let boardController = null; Promise.resolve() @@ -86,10 +84,9 @@ export default class BoardSetupCheck extends Component { // Is board plugged in? .then(() => this.detectBoardPluggedIn()) - .then(usablePort => portName = usablePort) // Can we talk to the firmware? - .then(() => this.detectCorrectFirmware(portName)) + .then(() => this.detectCorrectFirmware()) .then(board => boardController = board) // Can we initialize components successfully? @@ -113,7 +110,6 @@ export default class BoardSetupCheck extends Component { boardController.destroy(); boardController = null; } - portName = null; this.setState({isDetecting: false}); }); } @@ -156,25 +152,19 @@ export default class BoardSetupCheck extends Component { * @return {Promise.} Resolves to usable port name */ detectBoardPluggedIn() { - this.spin(STATUS_BOARD_PLUG); - return promiseWaitFor(200) - .then(findPortWithViableDevice) - .then(portName => { - this.succeed(STATUS_BOARD_PLUG); - return portName; - }) - .catch(error => { - this.fail(STATUS_BOARD_PLUG); - return Promise.reject(error); - }); + const {setupChecker} = this.props; + return this.detectStep( + STATUS_BOARD_PLUG, + () => setupChecker.detectBoardPluggedIn()); } /** * @return {Promise.} */ - detectCorrectFirmware(portName) { + detectCorrectFirmware() { + const {setupChecker} = this.props; this.spin(STATUS_BOARD_CONNECT); - const boardController = new CircuitPlaygroundBoard(portName); + const boardController = new CircuitPlaygroundBoard(setupChecker.port); return boardController.connectToFirmware() .then(() => { this.succeed(STATUS_BOARD_CONNECT); diff --git a/apps/src/lib/kits/maker/util/SetupChecker.js b/apps/src/lib/kits/maker/util/SetupChecker.js index 16137dcd3e9d3..4f5a8b7e7e6fc 100644 --- a/apps/src/lib/kits/maker/util/SetupChecker.js +++ b/apps/src/lib/kits/maker/util/SetupChecker.js @@ -1,5 +1,5 @@ /** @file Stubbable core setup check behavior for the setup page. */ -import {ensureAppInstalled} from '../portScanning'; +import {ensureAppInstalled, findPortWithViableDevice} from '../portScanning'; import {isChrome, gtChrome33} from './browserChecks'; export default class SetupChecker { @@ -25,4 +25,9 @@ export default class SetupChecker { detectChromeAppInstalled() { return ensureAppInstalled(); } + + detectBoardPluggedIn() { + return findPortWithViableDevice() + .then(port => this.port = port); + } } From 10b1ad7454997db4a7b47a5cab8896cbc396a3ae Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Tue, 21 Mar 2017 17:55:22 -0700 Subject: [PATCH 06/16] Move detectCorrectFirmware --- .../src/lib/kits/maker/ui/BoardSetupCheck.jsx | 37 ++++++------------- apps/src/lib/kits/maker/util/SetupChecker.js | 26 ++++++++++++- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx index 265dfd6b3173f..049f0ec325781 100644 --- a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx +++ b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx @@ -1,6 +1,5 @@ /** @file Maker Board setup checker */ import React, {Component, PropTypes} from 'react'; -import CircuitPlaygroundBoard from '../CircuitPlaygroundBoard'; import SetupChecker from '../util/SetupChecker'; import {isWindows, isChrome, getChromeVersion} from '../util/browserChecks'; import SetupStep, { @@ -72,8 +71,6 @@ export default class BoardSetupCheck extends Component { this.hide(STATUS_WINDOWS_DRIVERS); } - let boardController = null; - Promise.resolve() // Are we using a compatible browser? @@ -87,13 +84,12 @@ export default class BoardSetupCheck extends Component { // Can we talk to the firmware? .then(() => this.detectCorrectFirmware()) - .then(board => boardController = board) // Can we initialize components successfully? - .then(() => this.detectComponentsInitialize(boardController)) + .then(() => this.detectComponentsInitialize()) // Everything looks good, let's par-tay! - .then(() => this.celebrate(boardController)) + .then(() => this.celebrate()) // If anything goes wrong along the way, we'll end up in this // catch clause - make sure to report the error out. @@ -106,10 +102,7 @@ export default class BoardSetupCheck extends Component { // Finally... .then(() => { - if (boardController) { - boardController.destroy(); - boardController = null; - } + this.props.setupChecker.teardown(); this.setState({isDetecting: false}); }); } @@ -163,26 +156,19 @@ export default class BoardSetupCheck extends Component { */ detectCorrectFirmware() { const {setupChecker} = this.props; - this.spin(STATUS_BOARD_CONNECT); - const boardController = new CircuitPlaygroundBoard(setupChecker.port); - return boardController.connectToFirmware() - .then(() => { - this.succeed(STATUS_BOARD_CONNECT); - return boardController; - }) - .catch(error => { - this.fail(STATUS_BOARD_CONNECT); - return Promise.reject(error); - }); + return this.detectStep( + STATUS_BOARD_CONNECT, + () => setupChecker.detectCorrectFirmware()); } /** * @return {Promise} */ - detectComponentsInitialize(boardController) { + detectComponentsInitialize() { + const {setupChecker} = this.props; this.spin(STATUS_BOARD_COMPONENTS); return promiseWaitFor(200) - .then(() => boardController.initializeComponents()) + .then(() => setupChecker.boardController.initializeComponents()) .then(() => this.succeed(STATUS_BOARD_COMPONENTS)) .catch(error => { this.fail(STATUS_BOARD_COMPONENTS); @@ -193,9 +179,10 @@ export default class BoardSetupCheck extends Component { /** * @return {Promise} */ - celebrate(boardController) { + celebrate() { + const {setupChecker} = this.props; this.thumb(STATUS_BOARD_COMPONENTS); - return boardController.celebrateSuccessfulConnection() + return setupChecker.boardController.celebrateSuccessfulConnection() .then(() => this.succeed(STATUS_BOARD_COMPONENTS)); } diff --git a/apps/src/lib/kits/maker/util/SetupChecker.js b/apps/src/lib/kits/maker/util/SetupChecker.js index 4f5a8b7e7e6fc..1562f67289715 100644 --- a/apps/src/lib/kits/maker/util/SetupChecker.js +++ b/apps/src/lib/kits/maker/util/SetupChecker.js @@ -1,11 +1,15 @@ /** @file Stubbable core setup check behavior for the setup page. */ +import CircuitPlaygroundBoard from '../CircuitPlaygroundBoard'; import {ensureAppInstalled, findPortWithViableDevice} from '../portScanning'; import {isChrome, gtChrome33} from './browserChecks'; export default class SetupChecker { + port = null; + boardController = null; + /** * Resolve if using Chrome > 33 - * @returns {Promise} + * @return {Promise} */ detectChromeVersion() { return new Promise((resolve, reject) => { @@ -21,13 +25,33 @@ export default class SetupChecker { /** * Resolve if the Chrome Connector App is installed. + * @return {Promise} */ detectChromeAppInstalled() { return ensureAppInstalled(); } + /** + * @return {Promise} + */ detectBoardPluggedIn() { return findPortWithViableDevice() .then(port => this.port = port); } + + /** + * @return {Promise} + */ + detectCorrectFirmware() { + this.boardController = new CircuitPlaygroundBoard(this.port); + return this.boardController.connectToFirmware(); + } + + teardown() { + if (this.boardController) { + this.boardController.destroy(); + } + this.boardController = null; + this.port = null; + } } From cbc0a6a3ad171a6557a7982b95e1742164cfcd14 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Tue, 21 Mar 2017 17:58:13 -0700 Subject: [PATCH 07/16] Move detectComponentsInitialize --- apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx | 11 +++-------- apps/src/lib/kits/maker/util/SetupChecker.js | 10 ++++++++++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx index 049f0ec325781..cda3f12c03414 100644 --- a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx +++ b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx @@ -166,14 +166,9 @@ export default class BoardSetupCheck extends Component { */ detectComponentsInitialize() { const {setupChecker} = this.props; - this.spin(STATUS_BOARD_COMPONENTS); - return promiseWaitFor(200) - .then(() => setupChecker.boardController.initializeComponents()) - .then(() => this.succeed(STATUS_BOARD_COMPONENTS)) - .catch(error => { - this.fail(STATUS_BOARD_COMPONENTS); - return Promise.reject(error); - }); + return this.detectStep( + STATUS_BOARD_COMPONENTS, + () => setupChecker.detectComponentsInitialize()); } /** diff --git a/apps/src/lib/kits/maker/util/SetupChecker.js b/apps/src/lib/kits/maker/util/SetupChecker.js index 1562f67289715..ee2ff1eee9c93 100644 --- a/apps/src/lib/kits/maker/util/SetupChecker.js +++ b/apps/src/lib/kits/maker/util/SetupChecker.js @@ -47,6 +47,16 @@ export default class SetupChecker { return this.boardController.connectToFirmware(); } + /** + * @return {Promise} + */ + detectComponentsInitialize() { + return new Promise(resolve => { + this.boardController.initializeComponents(); + resolve(); + }); + } + teardown() { if (this.boardController) { this.boardController.destroy(); From 1ac1b66d18ed4a431fc8630450177276689e66c3 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Tue, 21 Mar 2017 18:05:44 -0700 Subject: [PATCH 08/16] Remove unneeded helpers --- .../src/lib/kits/maker/ui/BoardSetupCheck.jsx | 79 ++++--------------- apps/src/lib/kits/maker/util/SetupChecker.js | 7 ++ 2 files changed, 22 insertions(+), 64 deletions(-) diff --git a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx index cda3f12c03414..5c994729414a3 100644 --- a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx +++ b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx @@ -65,6 +65,7 @@ export default class BoardSetupCheck extends Component { } detect() { + const {setupChecker} = this.props; this.setState({...initialState, isDetecting: true}); if (!isWindows()) { @@ -74,22 +75,29 @@ export default class BoardSetupCheck extends Component { Promise.resolve() // Are we using a compatible browser? - .then(() => this.detectChromeVersion()) + .then(() => this.detectStep(STATUS_IS_CHROME, + () => setupChecker.detectChromeVersion())) // Is Chrome App Installed? - .then(() => this.detectChromeAppInstalled()) + .then(() => this.detectStep(STATUS_APP_INSTALLED, + () => setupChecker.detectChromeAppInstalled())) // Is board plugged in? - .then(() => this.detectBoardPluggedIn()) + .then(() => this.detectStep(STATUS_BOARD_PLUG, + () => setupChecker.detectBoardPluggedIn())) // Can we talk to the firmware? - .then(() => this.detectCorrectFirmware()) + .then(() => this.detectStep(STATUS_BOARD_CONNECT, + () => setupChecker.detectCorrectFirmware())) // Can we initialize components successfully? - .then(() => this.detectComponentsInitialize()) + .then(() => this.detectStep(STATUS_BOARD_COMPONENTS, + () => setupChecker.detectComponentsInitialize())) // Everything looks good, let's par-tay! - .then(() => this.celebrate()) + .then(() => this.thumb(STATUS_BOARD_COMPONENTS)) + .then(() => setupChecker.celebrate()) + .then(() => this.succeed(STATUS_BOARD_COMPONENTS)) // If anything goes wrong along the way, we'll end up in this // catch clause - make sure to report the error out. @@ -102,7 +110,7 @@ export default class BoardSetupCheck extends Component { // Finally... .then(() => { - this.props.setupChecker.teardown(); + setupChecker.teardown(); this.setState({isDetecting: false}); }); } @@ -124,63 +132,6 @@ export default class BoardSetupCheck extends Component { }); } - detectChromeVersion() { - const {setupChecker} = this.props; - return this.detectStep( - STATUS_IS_CHROME, - () => setupChecker.detectChromeVersion()); - } - - /** - * @return {Promise} - */ - detectChromeAppInstalled() { - const {setupChecker} = this.props; - return this.detectStep( - STATUS_APP_INSTALLED, - () => setupChecker.detectChromeAppInstalled()); - } - - /** - * @return {Promise.} Resolves to usable port name - */ - detectBoardPluggedIn() { - const {setupChecker} = this.props; - return this.detectStep( - STATUS_BOARD_PLUG, - () => setupChecker.detectBoardPluggedIn()); - } - - /** - * @return {Promise.} - */ - detectCorrectFirmware() { - const {setupChecker} = this.props; - return this.detectStep( - STATUS_BOARD_CONNECT, - () => setupChecker.detectCorrectFirmware()); - } - - /** - * @return {Promise} - */ - detectComponentsInitialize() { - const {setupChecker} = this.props; - return this.detectStep( - STATUS_BOARD_COMPONENTS, - () => setupChecker.detectComponentsInitialize()); - } - - /** - * @return {Promise} - */ - celebrate() { - const {setupChecker} = this.props; - this.thumb(STATUS_BOARD_COMPONENTS); - return setupChecker.boardController.celebrateSuccessfulConnection() - .then(() => this.succeed(STATUS_BOARD_COMPONENTS)); - } - /** * Helper to be used on second/subsequent attempts at detecing board usability. */ diff --git a/apps/src/lib/kits/maker/util/SetupChecker.js b/apps/src/lib/kits/maker/util/SetupChecker.js index ee2ff1eee9c93..7827d69b30262 100644 --- a/apps/src/lib/kits/maker/util/SetupChecker.js +++ b/apps/src/lib/kits/maker/util/SetupChecker.js @@ -57,6 +57,13 @@ export default class SetupChecker { }); } + /** + * @return {Promise} + */ + celebrate() { + return this.boardController.celebrateSuccessfulConnection(); + } + teardown() { if (this.boardController) { this.boardController.destroy(); From ec3e72e8eb8892e1a98e9e01c96d3fec45f71b5f Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Tue, 21 Mar 2017 18:14:46 -0700 Subject: [PATCH 09/16] BoardSetupCheckTest --- .../lib/kits/maker/ui/BoardSetupCheckTest.js | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js diff --git a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js new file mode 100644 index 0000000000000..4ca0406794eb2 --- /dev/null +++ b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js @@ -0,0 +1,45 @@ +/** @file Test BoardSetupCheck component */ +import React from 'react'; +import {expect} from '../../../../../util/configuredChai'; +import {mount} from 'enzyme'; +import BoardSetupCheck from '@cdo/apps/lib/kits/maker/ui/BoardSetupCheck'; + +describe('BoardSetupCheck', () => { + it('renders', done => { + const stubChecker = new StubSetupChecker(); + const wrapper = mount(); + expect(wrapper).not.to.be.null; + setTimeout(() => { + expect(wrapper).not.to.be.null; + done(); + }, 3000); + }); +}); + +class StubSetupChecker { + detectChromeVersion() { + return Promise.resolve(); + } + + detectChromeAppInstalled() { + return Promise.resolve(); + } + + detectBoardPluggedIn() { + return Promise.resolve(); + } + + detectCorrectFirmware() { + return Promise.resolve(); + } + + detectComponentsInitialize() { + return Promise.resolve(); + } + + celebrate() { + return Promise.resolve(); + } + + teardown() {} +} From dd4d8313e13ff5849d5fbf148e90853318edb74a Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Wed, 22 Mar 2017 10:34:52 -0700 Subject: [PATCH 10/16] StubSetupChecker extends real SetupChecker --- .../lib/kits/maker/ui/BoardSetupCheckTest.js | 47 ++++++++----------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js index 4ca0406794eb2..f7d462457d1ab 100644 --- a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js +++ b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js @@ -1,13 +1,15 @@ /** @file Test BoardSetupCheck component */ import React from 'react'; +import sinon from 'sinon'; import {expect} from '../../../../../util/configuredChai'; import {mount} from 'enzyme'; import BoardSetupCheck from '@cdo/apps/lib/kits/maker/ui/BoardSetupCheck'; +import SetupChecker from '@cdo/apps/lib/kits/maker/util/SetupChecker'; describe('BoardSetupCheck', () => { it('renders', done => { - const stubChecker = new StubSetupChecker(); - const wrapper = mount(); + const checker = new StubSetupChecker(); + const wrapper = mount(); expect(wrapper).not.to.be.null; setTimeout(() => { expect(wrapper).not.to.be.null; @@ -16,30 +18,21 @@ describe('BoardSetupCheck', () => { }); }); -class StubSetupChecker { - detectChromeVersion() { - return Promise.resolve(); +/** + * SetupChecker with all methods stubbed and by default everything succeeds. + * Since methods are sinon stubs, individual tests can modify stub behavior + * as needed. + * @see http://sinonjs.org/releases/v2.1.0/stubs/#defining-stub-behavior-on-consecutive-calls + */ +class StubSetupChecker extends SetupChecker { + constructor() { + super(); + sinon.stub(this, 'detectChromeVersion').returns(Promise.resolve()); + sinon.stub(this, 'detectChromeAppInstalled').returns(Promise.resolve()); + sinon.stub(this, 'detectBoardPluggedIn').returns(Promise.resolve()); + sinon.stub(this, 'detectCorrectFirmware').returns(Promise.resolve()); + sinon.stub(this, 'detectComponentsInitialize').returns(Promise.resolve()); + sinon.stub(this, 'celebrate').returns(Promise.resolve()); + sinon.stub(this, 'teardown'); } - - detectChromeAppInstalled() { - return Promise.resolve(); - } - - detectBoardPluggedIn() { - return Promise.resolve(); - } - - detectCorrectFirmware() { - return Promise.resolve(); - } - - detectComponentsInitialize() { - return Promise.resolve(); - } - - celebrate() { - return Promise.resolve(); - } - - teardown() {} } From d7e6420e5f71e8d065865d9a676ab6ce20015d56 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Wed, 22 Mar 2017 11:52:35 -0700 Subject: [PATCH 11/16] Add a test for rendering a failed check --- .../unit/lib/kits/maker/ui/BoardSetupCheckTest.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js index f7d462457d1ab..c49744be11379 100644 --- a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js +++ b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js @@ -16,6 +16,17 @@ describe('BoardSetupCheck', () => { done(); }, 3000); }); + + it('fails if chrome version is wrong', done => { + const checker = new StubSetupChecker(); + checker.detectChromeVersion.returns(Promise.reject(new Error('test error'))); + const wrapper = mount(); + expect(wrapper).not.to.be.null; + setTimeout(() => { + expect(wrapper).not.to.be.null; + done(); + }, 3000); + }); }); /** From b0fd5efd2462923dc53545424e168a1dcbdc0531 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Wed, 22 Mar 2017 12:40:22 -0700 Subject: [PATCH 12/16] Use sinon 2.1.0 features --- .../lib/kits/maker/ui/BoardSetupCheckTest.js | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js index c49744be11379..8b9f0bc55c40e 100644 --- a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js +++ b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js @@ -7,25 +7,38 @@ import BoardSetupCheck from '@cdo/apps/lib/kits/maker/ui/BoardSetupCheck'; import SetupChecker from '@cdo/apps/lib/kits/maker/util/SetupChecker'; describe('BoardSetupCheck', () => { + beforeEach(() => { + sinon.stub(window.console, 'error'); + }); + + afterEach(() => { + window.console.error.restore(); + }); + it('renders', done => { const checker = new StubSetupChecker(); + const spy = sinon.spy(); const wrapper = mount(); expect(wrapper).not.to.be.null; setTimeout(() => { expect(wrapper).not.to.be.null; + expect(spy).not.to.have.been.called; + expect(window.console.error).not.to.have.been.called; done(); - }, 3000); + }, 1500); }); it('fails if chrome version is wrong', done => { const checker = new StubSetupChecker(); - checker.detectChromeVersion.returns(Promise.reject(new Error('test error'))); + checker.detectChromeVersion.rejects(new Error('test error')); const wrapper = mount(); expect(wrapper).not.to.be.null; setTimeout(() => { - expect(wrapper).not.to.be.null; + expect(wrapper.find('.fa-times-circle')).to.have.length(1); + expect(wrapper.find('.fa-clock-o')).to.have.length(4); + expect(wrapper.text()).to.include('Your current browser is not supported at this time.'); done(); - }, 3000); + }, 1500); }); }); @@ -38,12 +51,12 @@ describe('BoardSetupCheck', () => { class StubSetupChecker extends SetupChecker { constructor() { super(); - sinon.stub(this, 'detectChromeVersion').returns(Promise.resolve()); - sinon.stub(this, 'detectChromeAppInstalled').returns(Promise.resolve()); - sinon.stub(this, 'detectBoardPluggedIn').returns(Promise.resolve()); - sinon.stub(this, 'detectCorrectFirmware').returns(Promise.resolve()); - sinon.stub(this, 'detectComponentsInitialize').returns(Promise.resolve()); - sinon.stub(this, 'celebrate').returns(Promise.resolve()); + sinon.stub(this, 'detectChromeVersion').resolves(); + sinon.stub(this, 'detectChromeAppInstalled').resolves(); + sinon.stub(this, 'detectBoardPluggedIn').resolves(); + sinon.stub(this, 'detectCorrectFirmware').resolves(); + sinon.stub(this, 'detectComponentsInitialize').resolves(); + sinon.stub(this, 'celebrate').resolves(); sinon.stub(this, 'teardown'); } } From 8550363fde9cafdf9c4b04037240c935135f32dc Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Wed, 22 Mar 2017 13:58:53 -0700 Subject: [PATCH 13/16] Cover re-detect --- .../lib/kits/maker/ui/BoardSetupCheckTest.js | 48 +++++++++++++++---- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js index 8b9f0bc55c40e..4f917b87577c7 100644 --- a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js +++ b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js @@ -7,36 +7,66 @@ import BoardSetupCheck from '@cdo/apps/lib/kits/maker/ui/BoardSetupCheck'; import SetupChecker from '@cdo/apps/lib/kits/maker/util/SetupChecker'; describe('BoardSetupCheck', () => { + let checker; + beforeEach(() => { sinon.stub(window.console, 'error'); + sinon.stub(window.location, 'reload'); + checker = new StubSetupChecker(); }); afterEach(() => { + window.location.reload.restore(); window.console.error.restore(); }); - it('renders', done => { - const checker = new StubSetupChecker(); - const spy = sinon.spy(); + it('renders success', done => { const wrapper = mount(); - expect(wrapper).not.to.be.null; + expect(wrapper.find('.fa-clock-o')).to.have.length(5); setTimeout(() => { - expect(wrapper).not.to.be.null; - expect(spy).not.to.have.been.called; + expect(wrapper.find('.fa-check-circle')).to.have.length(5); expect(window.console.error).not.to.have.been.called; done(); }, 1500); }); it('fails if chrome version is wrong', done => { - const checker = new StubSetupChecker(); - checker.detectChromeVersion.rejects(new Error('test error')); + const error = new Error('test error'); + checker.detectChromeVersion.rejects(error); const wrapper = mount(); - expect(wrapper).not.to.be.null; + expect(wrapper.find('.fa-clock-o')).to.have.length(5); setTimeout(() => { expect(wrapper.find('.fa-times-circle')).to.have.length(1); expect(wrapper.find('.fa-clock-o')).to.have.length(4); expect(wrapper.text()).to.include('Your current browser is not supported at this time.'); + expect(window.console.error).to.have.been.calledWith(error); + done(); + }, 1500); + }); + + it('does not reload the page on re-detect if successful', done => { + const wrapper = mount(); + setTimeout(() => { + expect(wrapper.find('.fa-check-circle')).to.have.length(5); + wrapper.find('input[value="re-detect"]').simulate('click'); + expect(wrapper.find('.fa-clock-o')).to.have.length(5); + setTimeout(() => { + expect(wrapper.find('.fa-check-circle')).to.have.length(5); + expect(window.location.reload).not.to.have.been.called; + done(); + }, 1500); + }, 1500); + }); + + it('reloads the page on re-detect if plugin not installed', done => { + checker.detectChromeAppInstalled.rejects(new Error('not installed')); + const wrapper = mount(); + setTimeout(() => { + expect(wrapper.find('.fa-check-circle')).to.have.length(1); + expect(wrapper.find('.fa-times-circle')).to.have.length(1); + expect(wrapper.find('.fa-clock-o')).to.have.length(3); + wrapper.find('input[value="re-detect"]').simulate('click'); + expect(window.location.reload).to.have.been.called; done(); }, 1500); }); From d2f084ff2fcc783525cb37937018ff555f1fc1ad Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Wed, 22 Mar 2017 14:19:56 -0700 Subject: [PATCH 14/16] Run tests faster --- .../src/lib/kits/maker/ui/BoardSetupCheck.jsx | 5 ++- .../lib/kits/maker/ui/BoardSetupCheckTest.js | 39 ++++++++++++++----- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx index 5c994729414a3..7704abd41f5f3 100644 --- a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx +++ b/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx @@ -33,7 +33,8 @@ export default class BoardSetupCheck extends Component { state = {...initialState}; static propTypes = { - setupChecker: PropTypes.instanceOf(SetupChecker) + setupChecker: PropTypes.instanceOf(SetupChecker).isRequired, + stepDelay: PropTypes.number, }; hide(selector) { @@ -123,7 +124,7 @@ export default class BoardSetupCheck extends Component { */ detectStep(stepKey, stepWork) { this.spin(stepKey); - return promiseWaitFor(200) + return promiseWaitFor(this.props.stepDelay || 200) .then(stepWork) .then(() => this.succeed(stepKey)) .catch(error => { diff --git a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js index 4f917b87577c7..de7f7d046e3cc 100644 --- a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js +++ b/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js @@ -7,6 +7,7 @@ import BoardSetupCheck from '@cdo/apps/lib/kits/maker/ui/BoardSetupCheck'; import SetupChecker from '@cdo/apps/lib/kits/maker/util/SetupChecker'; describe('BoardSetupCheck', () => { + const STEP_DELAY = 15; let checker; beforeEach(() => { @@ -21,19 +22,29 @@ describe('BoardSetupCheck', () => { }); it('renders success', done => { - const wrapper = mount(); + const wrapper = mount( + + ); expect(wrapper.find('.fa-clock-o')).to.have.length(5); setTimeout(() => { expect(wrapper.find('.fa-check-circle')).to.have.length(5); expect(window.console.error).not.to.have.been.called; done(); - }, 1500); + }, STEP_DELAY * 10); }); it('fails if chrome version is wrong', done => { const error = new Error('test error'); checker.detectChromeVersion.rejects(error); - const wrapper = mount(); + const wrapper = mount( + + ); expect(wrapper.find('.fa-clock-o')).to.have.length(5); setTimeout(() => { expect(wrapper.find('.fa-times-circle')).to.have.length(1); @@ -41,11 +52,16 @@ describe('BoardSetupCheck', () => { expect(wrapper.text()).to.include('Your current browser is not supported at this time.'); expect(window.console.error).to.have.been.calledWith(error); done(); - }, 1500); + }, STEP_DELAY * 10); }); it('does not reload the page on re-detect if successful', done => { - const wrapper = mount(); + const wrapper = mount( + + ); setTimeout(() => { expect(wrapper.find('.fa-check-circle')).to.have.length(5); wrapper.find('input[value="re-detect"]').simulate('click'); @@ -54,13 +70,18 @@ describe('BoardSetupCheck', () => { expect(wrapper.find('.fa-check-circle')).to.have.length(5); expect(window.location.reload).not.to.have.been.called; done(); - }, 1500); - }, 1500); + }, STEP_DELAY * 10); + }, STEP_DELAY * 10); }); it('reloads the page on re-detect if plugin not installed', done => { checker.detectChromeAppInstalled.rejects(new Error('not installed')); - const wrapper = mount(); + const wrapper = mount( + + ); setTimeout(() => { expect(wrapper.find('.fa-check-circle')).to.have.length(1); expect(wrapper.find('.fa-times-circle')).to.have.length(1); @@ -68,7 +89,7 @@ describe('BoardSetupCheck', () => { wrapper.find('input[value="re-detect"]').simulate('click'); expect(window.location.reload).to.have.been.called; done(); - }, 1500); + }, STEP_DELAY * 10); }); }); From 7ae06375ec9cfaecbfd2338968cb32a7664e8355 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Wed, 22 Mar 2017 14:31:11 -0700 Subject: [PATCH 15/16] Rename BoardSetupCheck to SetupChecklist --- .../ui/{BoardSetupCheck.jsx => SetupChecklist.jsx} | 2 +- apps/src/sites/studio/pages/maker/setup.js | 4 ++-- ...oardSetupCheckTest.js => SetupChecklistTest.js} | 14 +++++++------- 3 files changed, 10 insertions(+), 10 deletions(-) rename apps/src/lib/kits/maker/ui/{BoardSetupCheck.jsx => SetupChecklist.jsx} (99%) rename apps/test/unit/lib/kits/maker/ui/{BoardSetupCheckTest.js => SetupChecklistTest.js} (93%) diff --git a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx b/apps/src/lib/kits/maker/ui/SetupChecklist.jsx similarity index 99% rename from apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx rename to apps/src/lib/kits/maker/ui/SetupChecklist.jsx index 7704abd41f5f3..e242c6136c994 100644 --- a/apps/src/lib/kits/maker/ui/BoardSetupCheck.jsx +++ b/apps/src/lib/kits/maker/ui/SetupChecklist.jsx @@ -29,7 +29,7 @@ const initialState = { [STATUS_BOARD_COMPONENTS]: WAITING, }; -export default class BoardSetupCheck extends Component { +export default class SetupChecklist extends Component { state = {...initialState}; static propTypes = { diff --git a/apps/src/sites/studio/pages/maker/setup.js b/apps/src/sites/studio/pages/maker/setup.js index a94d929e38714..87e670ab237ee 100644 --- a/apps/src/sites/studio/pages/maker/setup.js +++ b/apps/src/sites/studio/pages/maker/setup.js @@ -1,12 +1,12 @@ import React from 'react'; import ReactDOM from 'react-dom'; -import BoardSetupCheck from '@cdo/apps/lib/kits/maker/ui/BoardSetupCheck'; +import SetupChecklist from '@cdo/apps/lib/kits/maker/ui/SetupChecklist'; import SetupChecker from '@cdo/apps/lib/kits/maker/util/SetupChecker'; $(function () { const setupChecker = new SetupChecker(); ReactDOM.render( - , + , document.getElementById('setup-status-mount') ); $('.maker-setup a').attr('target', '_blank'); diff --git a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js b/apps/test/unit/lib/kits/maker/ui/SetupChecklistTest.js similarity index 93% rename from apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js rename to apps/test/unit/lib/kits/maker/ui/SetupChecklistTest.js index de7f7d046e3cc..7df3e2e02bfbf 100644 --- a/apps/test/unit/lib/kits/maker/ui/BoardSetupCheckTest.js +++ b/apps/test/unit/lib/kits/maker/ui/SetupChecklistTest.js @@ -1,12 +1,12 @@ -/** @file Test BoardSetupCheck component */ +/** @file Test SetupChecklist component */ import React from 'react'; import sinon from 'sinon'; import {expect} from '../../../../../util/configuredChai'; import {mount} from 'enzyme'; -import BoardSetupCheck from '@cdo/apps/lib/kits/maker/ui/BoardSetupCheck'; +import SetupChecklist from '@cdo/apps/lib/kits/maker/ui/SetupChecklist'; import SetupChecker from '@cdo/apps/lib/kits/maker/util/SetupChecker'; -describe('BoardSetupCheck', () => { +describe('SetupChecklist', () => { const STEP_DELAY = 15; let checker; @@ -23,7 +23,7 @@ describe('BoardSetupCheck', () => { it('renders success', done => { const wrapper = mount( - @@ -40,7 +40,7 @@ describe('BoardSetupCheck', () => { const error = new Error('test error'); checker.detectChromeVersion.rejects(error); const wrapper = mount( - @@ -57,7 +57,7 @@ describe('BoardSetupCheck', () => { it('does not reload the page on re-detect if successful', done => { const wrapper = mount( - @@ -77,7 +77,7 @@ describe('BoardSetupCheck', () => { it('reloads the page on re-detect if plugin not installed', done => { checker.detectChromeAppInstalled.rejects(new Error('not installed')); const wrapper = mount( - From 9c340a1d956d06689db09cea3a087d5ec5a8f4a7 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Thu, 23 Mar 2017 12:12:03 -0700 Subject: [PATCH 16/16] Make tests more reliable --- .../lib/kits/maker/ui/SetupChecklistTest.js | 123 ++++++++++++------ 1 file changed, 82 insertions(+), 41 deletions(-) diff --git a/apps/test/unit/lib/kits/maker/ui/SetupChecklistTest.js b/apps/test/unit/lib/kits/maker/ui/SetupChecklistTest.js index 7df3e2e02bfbf..a89fbf26ab1d0 100644 --- a/apps/test/unit/lib/kits/maker/ui/SetupChecklistTest.js +++ b/apps/test/unit/lib/kits/maker/ui/SetupChecklistTest.js @@ -7,9 +7,17 @@ import SetupChecklist from '@cdo/apps/lib/kits/maker/ui/SetupChecklist'; import SetupChecker from '@cdo/apps/lib/kits/maker/util/SetupChecker'; describe('SetupChecklist', () => { - const STEP_DELAY = 15; let checker; + // Speed up the tests by reducing the artificial delay between steps + const STEP_DELAY_MS = 1; + + // Helpful selectors + const REDETECT_BUTTON = 'input[value="re-detect"]'; + const WAITING_ICON = '.fa-clock-o'; + const SUCCESS_ICON = '.fa-check-circle'; + const FAILURE_ICON = '.fa-times-circle'; + beforeEach(() => { sinon.stub(window.console, 'error'); sinon.stub(window.location, 'reload'); @@ -21,78 +29,111 @@ describe('SetupChecklist', () => { window.console.error.restore(); }); - it('renders success', done => { + it('renders success', () => { const wrapper = mount( ); - expect(wrapper.find('.fa-clock-o')).to.have.length(5); - setTimeout(() => { - expect(wrapper.find('.fa-check-circle')).to.have.length(5); - expect(window.console.error).not.to.have.been.called; - done(); - }, STEP_DELAY * 10); + expect(wrapper.find(REDETECT_BUTTON)).to.be.disabled; + expect(wrapper.find(WAITING_ICON)).to.have.length(5); + return yieldUntilDoneDetecting(wrapper) + .then(() => { + expect(wrapper.find(REDETECT_BUTTON)).not.to.be.disabled; + expect(wrapper.find(SUCCESS_ICON)).to.have.length(5); + expect(window.console.error).not.to.have.been.called; + }); }); - it('fails if chrome version is wrong', done => { + it('fails if chrome version is wrong', () => { const error = new Error('test error'); checker.detectChromeVersion.rejects(error); const wrapper = mount( ); - expect(wrapper.find('.fa-clock-o')).to.have.length(5); - setTimeout(() => { - expect(wrapper.find('.fa-times-circle')).to.have.length(1); - expect(wrapper.find('.fa-clock-o')).to.have.length(4); - expect(wrapper.text()).to.include('Your current browser is not supported at this time.'); - expect(window.console.error).to.have.been.calledWith(error); - done(); - }, STEP_DELAY * 10); + expect(wrapper.find(WAITING_ICON)).to.have.length(5); + return yieldUntilDoneDetecting(wrapper) + .then(() => { + expect(wrapper.find(FAILURE_ICON)).to.have.length(1); + expect(wrapper.find(WAITING_ICON)).to.have.length(4); + expect(wrapper.text()).to.include('Your current browser is not supported at this time.'); + expect(window.console.error).to.have.been.calledWith(error); + }); }); - it('does not reload the page on re-detect if successful', done => { + it('does not reload the page on re-detect if successful', () => { const wrapper = mount( ); - setTimeout(() => { - expect(wrapper.find('.fa-check-circle')).to.have.length(5); - wrapper.find('input[value="re-detect"]').simulate('click'); - expect(wrapper.find('.fa-clock-o')).to.have.length(5); - setTimeout(() => { - expect(wrapper.find('.fa-check-circle')).to.have.length(5); - expect(window.location.reload).not.to.have.been.called; - done(); - }, STEP_DELAY * 10); - }, STEP_DELAY * 10); + return yieldUntilDoneDetecting(wrapper) + .then(() => { + expect(wrapper.find(SUCCESS_ICON)).to.have.length(5); + wrapper.find(REDETECT_BUTTON).simulate('click'); + expect(wrapper.find(WAITING_ICON)).to.have.length(5); + }) + .then(() => yieldUntilDoneDetecting(wrapper)) + .then(() => { + expect(wrapper.find(SUCCESS_ICON)).to.have.length(5); + expect(window.location.reload).not.to.have.been.called; + }); }); - it('reloads the page on re-detect if plugin not installed', done => { + it('reloads the page on re-detect if plugin not installed', () => { checker.detectChromeAppInstalled.rejects(new Error('not installed')); const wrapper = mount( ); - setTimeout(() => { - expect(wrapper.find('.fa-check-circle')).to.have.length(1); - expect(wrapper.find('.fa-times-circle')).to.have.length(1); - expect(wrapper.find('.fa-clock-o')).to.have.length(3); - wrapper.find('input[value="re-detect"]').simulate('click'); - expect(window.location.reload).to.have.been.called; - done(); - }, STEP_DELAY * 10); + return yieldUntilDoneDetecting(wrapper) + .then(() => { + expect(wrapper.find(SUCCESS_ICON)).to.have.length(1); + expect(wrapper.find(FAILURE_ICON)).to.have.length(1); + expect(wrapper.find(WAITING_ICON)).to.have.length(3); + wrapper.find(REDETECT_BUTTON).simulate('click'); + expect(window.location.reload).to.have.been.called; + }); }); + + function yieldUntilDoneDetecting(wrapper) { + return yieldUntil(() => !wrapper.find(REDETECT_BUTTON).prop('disabled')); + } }); +/** + * Returns a promise that resolves when a condition becomes true, or rejects + * when a timeout is reached. + * @param {function():boolean} predicate + * @param {number} timeoutMs - maximum time to wait + * @param {number} intervalMs - time to wait between steps + * @return {Promise} + */ +function yieldUntil(predicate, timeoutMs = 2000, intervalMs = 5) { + return new Promise((resolve, reject) => { + let elapsedTime = 0; + const key = setInterval(() => { + if (predicate()) { + clearInterval(key); + resolve(); + } else { + elapsedTime += intervalMs; + if (elapsedTime > timeoutMs) { + clearInterval(key); + reject(new Error(`yieldUntil exceeded timeout of ${timeoutMs}ms`)); + } + } + }, intervalMs); + }); +} + /** * SetupChecker with all methods stubbed and by default everything succeeds. * Since methods are sinon stubs, individual tests can modify stub behavior