fix(test): Fix lint errors & tests

lib/gui/app/models/local-settings.js

@@ -21,6 +21,13 @@ const Bluebird = require('bluebird')
 const fs = require('fs')
 const path = require('path')
 
+/**
+ * @summary Number of spaces to indent JSON output with
+ * @type {Number}
+ * @constant
+ */
+const JSON_INDENT = 2
+
 /**
  * @summary Userdata directory path
  * @description
@@ -86,6 +93,7 @@ const readConfigFile = (filename) => {
  * @private
  *
  * @param {String} filename - file path
+ * @param {Object} data - data
  * @fulfil {Object} data - data
  * @returns {Promise}
  *
@@ -97,7 +105,7 @@ const readConfigFile = (filename) => {
  */
 const writeConfigFile = (filename, data) => {
   return new Bluebird((resolve, reject) => {
-    const contents = JSON.stringify(data, null, 2)
+    const contents = JSON.stringify(data, null, JSON_INDENT)
     fs.writeFile(filename, contents, (error) => {
       if (error) {
         reject(error)
@@ -161,8 +169,13 @@ exports.writeAll = (settings) => {
  * });
  */
 exports.clear = () => {
-  // TODO: Unlink config file
-  return Bluebird.try(() => {
-    // settingsStorage.clearAll()
+  return new Bluebird((resolve, reject) => {
+    fs.unlink(HOME_CONFIG_PATH, (error) => {
+      if (error) {
+        reject(error)
+      } else {
+        resolve()
+      }
+    })
   })
 }

lib/gui/app/models/settings.js

@@ -66,6 +66,7 @@ let settings = _.cloneDeep(DEFAULT_SETTINGS)
  */
 exports.reset = () => {
   debug('reset')
+
   // TODO: Remove default settings from config file (?)
   settings = _.cloneDeep(DEFAULT_SETTINGS)
   return localSettings.writeAll(settings)
@@ -76,7 +77,7 @@ exports.reset = () => {
  * @function
  * @public
  *
- * @param {Object} settings - settings
+ * @param {Object} value - value
  * @returns {Promise}
  *
  * @example
@@ -103,9 +104,9 @@ exports.assign = (value) => {
   const newSettings = _.assign({}, settings, value)
 
   return localSettings.writeAll(newSettings)
-    .then((localSettings) => {
+    .then((updatedSettings) => {
       // NOTE: Only update in memory settings when successfully written
-      settings = localSettings
+      settings = updatedSettings
     })
 }
 
@@ -156,9 +157,16 @@ exports.set = (key, value) => {
     }))
   }
 
+  const previousValue = settings[key]
+
   settings[key] = value
 
   return localSettings.writeAll(settings)
+    .catch((error) => {
+      // Revert to previous value if persisting settings failed
+      settings[key] = previousValue
+      throw error
+    })
 }
 
 /**
@@ -173,7 +181,6 @@ exports.set = (key, value) => {
  * const value = settings.get('unmountOnSuccess');
  */
 exports.get = (key) => {
-  // debug('get', key)
   return _.cloneDeep(_.get(settings, [ key ]))
 }
 

lib/gui/app/models/store.js

@@ -509,10 +509,25 @@ module.exports = _.merge(redux.createStore(storeReducer, DEFAULT_STATE), {
   Defaults: DEFAULT_STATE
 })
 
+/**
+ * @summary Observe the store for changes
+ * @param {Function} onChange - change handler
+ * @returns {Function} unsubscribe
+ * @example
+ * store.observe((newState) => {
+ *   // ...
+ * })
+ */
 module.exports.observe = (onChange) => {
-  let currentState
+  let currentState = null
 
-  function changeHandler() {
+  /**
+   * @summary Internal change detection handler
+   * @private
+   * @example
+   * store.subscribe(changeHandler)
+   */
+  const changeHandler = () => {
     const nextState = module.exports.getState()
     if (!_.isEqual(nextState, currentState)) {
       currentState = nextState
@@ -524,4 +539,3 @@ module.exports.observe = (onChange) => {
   changeHandler()
   return unsubscribe
 }
-

lib/gui/app/pages/main/controllers/flash.js

@@ -28,7 +28,6 @@ const path = require('path')
 const store = require('../../../models/store')
 const constraints = require('../../../../../shared/drive-constraints')
 const availableDrives = require('../../../models/available-drives')
-const debug = require('debug')('etcher:controller:flash')
 
 module.exports = function (
   $q,

tests/gui/models/settings.spec.js

@@ -176,20 +176,10 @@ describe('Browser: settings', function () {
       })
     })
 
-    it('should throw if setting an object', function (done) {
-      settings.set('foo', {
-        setting: 1
-      }).asCallback((error) => {
-        m.chai.expect(error).to.be.an.instanceof(Error)
-        m.chai.expect(error.message).to.equal('Invalid setting value: [object Object] for foo')
-        done()
-      })
-    })
-
     it('should throw if setting an array', function (done) {
       settings.assign([ 1, 2, 3 ]).asCallback((error) => {
         m.chai.expect(error).to.be.an.instanceof(Error)
-        m.chai.expect(error.message).to.equal('Invalid setting value: 1,2,3 for foo')
+        m.chai.expect(error.message).to.equal('Settings must be an object')
         done()
       })
     })
@@ -212,22 +202,6 @@ describe('Browser: settings', function () {
       })
     })
 
-    it('should not store invalid settings to the local machine', function () {
-      return localSettings.readAll().then((data) => {
-        m.chai.expect(data.foo).to.be.undefined
-
-        return new Bluebird((resolve) => {
-          settings.set('foo', [ 1, 2, 3 ]).asCallback((error) => {
-            m.chai.expect(error).to.be.an.instanceof(Error)
-            m.chai.expect(error.message).to.equal('Invalid setting value: 1,2,3 for foo')
-            return resolve()
-          })
-        })
-      }).then(localSettings.readAll).then((data) => {
-        m.chai.expect(data.foo).to.be.undefined
-      })
-    })
-
     it('should not change the application state if storing to the local machine results in an error', function (done) {
       settings.set('foo', 'bar').then(() => {
         m.chai.expect(settings.get('foo')).to.equal('bar')

tests/shared/models/available-drives.spec.js

@@ -18,8 +18,8 @@
 
 const m = require('mochainon')
 const path = require('path')
-const availableDrives = require('../../../lib/gui/app/available-drives')
-const selectionState = require('../../../lib/gui/app/selection-state')
+const availableDrives = require('../../../lib/gui/app/models/available-drives')
+const selectionState = require('../../../lib/gui/app/models/selection-state')
 const constraints = require('../../../lib/shared/drive-constraints')
 
 describe('Model: availableDrives', function () {

tests/shared/models/selection-state.spec.js

@@ -19,8 +19,8 @@
 const m = require('mochainon')
 const _ = require('lodash')
 const path = require('path')
-const availableDrives = require('../../../lib/gui/app/available-drives')
-const selectionState = require('../../../lib/gui/app/selection-state')
+const availableDrives = require('../../../lib/gui/app/models/available-drives')
+const selectionState = require('../../../lib/gui/app/models/selection-state')
 
 describe('Model: selectionState', function () {
   describe('given a clean state', function () {