From cf66afd04992d184cd3f82f56d4d3fa4aa758f84 Mon Sep 17 00:00:00 2001 From: yatki Date: Mon, 30 Jul 2018 17:00:41 +0200 Subject: [PATCH 1/4] :recycle: Remove process.exit from config commands --- src/utils/config.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/utils/config.js b/src/utils/config.js index f4e491bb..40dc9e60 100644 --- a/src/utils/config.js +++ b/src/utils/config.js @@ -20,20 +20,18 @@ import defaultConfig from '../../default_config.json'; import { CONFIG_VARIABLES } from './constants'; import { ValidationError } from './error'; import { readJSONSync, writeJSONSync } from './fs'; -import logger from './logger'; const configFileName = 'config.json'; const lockfileName = 'config.lock'; const fileWriteErrorMessage = filePath => - `ERROR: Could not write to \`${filePath}\`. Your configuration will not be persisted.`; + `Could not write to \`${filePath}\`. Your configuration will not be persisted.`; const attemptCallWithError = (fn, errorMessage) => { try { return fn(); } catch (_) { - logger.error(errorMessage); - return process.exit(1); + throw new Error(errorMessage); } }; @@ -51,8 +49,7 @@ const checkLockfile = filePath => { const locked = lockfile.checkSync(filePath); const errorMessage = `Config lockfile at ${filePath} found. Are you running Lisk Commander in another process?`; if (locked) { - logger.error(errorMessage); - process.exit(1); + throw new Error(errorMessage); } }; From f849f0a706cb27108a8273f4b9e1689b9fadce77 Mon Sep 17 00:00:00 2001 From: yatki Date: Mon, 30 Jul 2018 17:01:26 +0200 Subject: [PATCH 2/4] :white_check_mark: Fix config command tests --- test/utils/config.js | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/test/utils/config.js b/test/utils/config.js index a8f29bed..fbc3dc7d 100644 --- a/test/utils/config.js +++ b/test/utils/config.js @@ -18,11 +18,10 @@ import os from 'os'; import lockfile from 'lockfile'; import { getConfig, setConfig } from '../../src/utils/config'; import * as fsUtils from '../../src/utils/fs'; -import logger from '../../src/utils/logger'; import defaultConfig from '../../default_config.json'; describe('config utils', () => { - const configDirName = '.lisk-commander'; + const configDirName = '.lisk'; const configFileName = 'config.json'; const lockfileName = 'config.lock'; @@ -30,8 +29,6 @@ describe('config utils', () => { beforeEach(() => { sandbox.stub(fsUtils, 'writeJSONSync'); - sandbox.stub(logger, 'error'); - sandbox.stub(process, 'exit'); return Promise.resolve(); }); @@ -55,10 +52,8 @@ describe('config utils', () => { it('should log error when it fails to write', () => { fs.mkdirSync.throws(new Error('failed to create folder')); - getConfig(defaultPath); - expect(process.exit).to.be.calledWithExactly(1); - return expect(logger.error).to.be.calledWithExactly( - `ERROR: Could not write to \`${defaultPath}\`. Your configuration will not be persisted.`, + return expect(() => getConfig(defaultPath)).to.throw( + `Could not write to \`${defaultPath}\`. Your configuration will not be persisted.`, ); }); }); @@ -79,10 +74,8 @@ describe('config utils', () => { it('should log error when it fails to write', () => { fsUtils.writeJSONSync.throws(new Error('failed to write to the file')); - getConfig(defaultPath); - expect(process.exit).to.be.calledWithExactly(1); - return expect(logger.error).to.be.calledWithExactly( - `ERROR: Could not write to \`${defaultPath}/${configFileName}\`. Your configuration will not be persisted.`, + return expect(() => getConfig(defaultPath)).to.throw( + `Could not write to \`${defaultPath}/${configFileName}\`. Your configuration will not be persisted.`, ); }); }); @@ -106,18 +99,14 @@ describe('config utils', () => { it('should log error and exit when it fails to read', () => { fsUtils.readJSONSync.throws(new Error('failed to read to the file')); - getConfig(defaultPath); - expect(process.exit).to.be.calledWithExactly(1); - return expect(logger.error).to.be.calledWithExactly( + return expect(() => getConfig(defaultPath)).to.throw( `Config file cannot be read or is not valid JSON. Please check ${defaultPath}/${configFileName} or delete the file so we can create a new one from defaults.`, ); }); it('should log error and exit when it has invalid keys', () => { fsUtils.readJSONSync.returns({ random: 'values' }); - getConfig(defaultPath); - expect(process.exit).to.be.calledWithExactly(1); - return expect(logger.error).to.be.calledWithExactly( + return expect(() => getConfig(defaultPath)).to.throw( `Config file seems to be corrupted: missing required keys. Please check ${defaultPath}/${configFileName} or delete the file so we can create a new one from defaults.`, ); }); @@ -140,9 +129,7 @@ describe('config utils', () => { describe('when lockfile exists', () => { it('should log error and exit', () => { lockfile.checkSync.returns(true); - setConfig(defaultPath, newConfigValue); - expect(process.exit).to.be.calledWithExactly(1); - return expect(logger.error).to.be.calledWithExactly( + return expect(() => setConfig(defaultPath, newConfigValue)).to.throw( `Config lockfile at ${defaultPath}/${lockfileName} found. Are you running Lisk Commander in another process?`, ); }); From 8b6323271215b4b200997b9bd6ff7f131594db23 Mon Sep 17 00:00:00 2001 From: yatki Date: Tue, 31 Jul 2018 12:45:38 +0200 Subject: [PATCH 3/4] :recycle: Remove logger.js --- src/utils/logger.js | 33 ---------------------- test/utils/logger.js | 66 -------------------------------------------- 2 files changed, 99 deletions(-) delete mode 100644 src/utils/logger.js delete mode 100644 test/utils/logger.js diff --git a/src/utils/logger.js b/src/utils/logger.js deleted file mode 100644 index a1a4c26e..00000000 --- a/src/utils/logger.js +++ /dev/null @@ -1,33 +0,0 @@ -/* - * LiskHQ/lisk-commander - * Copyright © 2017 Lisk Foundation - * - * See the LICENSE file at the top-level directory of this distribution - * for licensing information. - * - * Unless otherwise agreed in a custom licensing agreement with the Lisk Foundation, - * no part of this software, including this file, may be copied, modified, - * propagated, or distributed except according to the terms contained in the - * LICENSE file. - * - * Removal or modification of this copyright notice is prohibited. - * - */ -import chalk from 'chalk'; - -const PLACEHOLDERS = ['%s', '%d', '%i', '%f', '%j', '%o', '%O']; - -const wrapLogFunction = (fn, colour) => (...args) => { - const colourArg = arg => chalk[colour](arg); - const isPlaceholderPresent = placeholder => args[0].includes(placeholder); - return PLACEHOLDERS.some(isPlaceholderPresent) - ? fn(colourArg(args[0]), ...args.slice(1)) - : fn(...args.map(colourArg)); -}; - -const logger = { - warn: wrapLogFunction(console.warn, 'yellow'), - error: wrapLogFunction(console.error, 'red'), -}; - -export default logger; diff --git a/test/utils/logger.js b/test/utils/logger.js deleted file mode 100644 index efb463c5..00000000 --- a/test/utils/logger.js +++ /dev/null @@ -1,66 +0,0 @@ -/* - * LiskHQ/lisk-commander - * Copyright © 2017–2018 Lisk Foundation - * - * See the LICENSE file at the top-level directory of this distribution - * for licensing information. - * - * Unless otherwise agreed in a custom licensing agreement with the Lisk Foundation, - * no part of this software, including this file, may be copied, modified, - * propagated, or distributed except according to the terms contained in the - * LICENSE file. - * - * Removal or modification of this copyright notice is prohibited. - * - */ -import chalk from 'chalk'; - -describe('logger utils', () => { - let logger; - beforeEach(() => { - delete require.cache[require.resolve('../../src/utils/logger')]; - sandbox.stub(console, 'warn'); - sandbox.stub(console, 'error'); - // NOTE: This dynamic require is necessary because otherwise the log - // function is created with a bound console method rather than the stub. - // eslint-disable-next-line global-require - logger = require('../../src/utils/logger').default; - return Promise.resolve(); - }); - - describe('#console.warn', () => { - it('should log with yellow with 2 strings', () => { - const args = ['Something to be warned about', 'Something else']; - logger.warn(...args); - const yellowArguments = args.map(arg => chalk.yellow(arg)); - return expect(console.warn).to.be.calledWithExactly(...yellowArguments); - }); - - it('should log with yellow with string with placeholder', () => { - const args = ['Something to be %d', 1]; - logger.warn(...args); - return expect(console.warn).to.be.calledWithExactly( - chalk.yellow(args[0]), - args[1], - ); - }); - }); - - describe('#console.error', () => { - it('should log with red with 2 strings', () => { - const args = ['Something to be warned about', 'Something else']; - logger.error(...args); - const redArguments = args.map(arg => chalk.red(arg)); - return expect(console.error).to.be.calledWithExactly(...redArguments); - }); - - it('should log with yellow with string with placeholder', () => { - const args = ['Something to be %s and something', 'inserted']; - logger.error(...args); - return expect(console.error).to.be.calledWithExactly( - chalk.red(args[0]), - args[1], - ); - }); - }); -}); From 35ae8da4e39c0694c025ae830ac6b837645c98d8 Mon Sep 17 00:00:00 2001 From: yatki Date: Tue, 31 Jul 2018 12:48:40 +0200 Subject: [PATCH 4/4] :recycle: Rewrite getConfig & setConfig statements in tests --- test/utils/config.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/utils/config.js b/test/utils/config.js index fbc3dc7d..422b7b96 100644 --- a/test/utils/config.js +++ b/test/utils/config.js @@ -52,7 +52,7 @@ describe('config utils', () => { it('should log error when it fails to write', () => { fs.mkdirSync.throws(new Error('failed to create folder')); - return expect(() => getConfig(defaultPath)).to.throw( + return expect(getConfig.bind(null, defaultPath)).to.throw( `Could not write to \`${defaultPath}\`. Your configuration will not be persisted.`, ); }); @@ -74,7 +74,7 @@ describe('config utils', () => { it('should log error when it fails to write', () => { fsUtils.writeJSONSync.throws(new Error('failed to write to the file')); - return expect(() => getConfig(defaultPath)).to.throw( + return expect(getConfig.bind(null, defaultPath)).to.throw( `Could not write to \`${defaultPath}/${configFileName}\`. Your configuration will not be persisted.`, ); }); @@ -99,14 +99,14 @@ describe('config utils', () => { it('should log error and exit when it fails to read', () => { fsUtils.readJSONSync.throws(new Error('failed to read to the file')); - return expect(() => getConfig(defaultPath)).to.throw( + return expect(getConfig.bind(null, defaultPath)).to.throw( `Config file cannot be read or is not valid JSON. Please check ${defaultPath}/${configFileName} or delete the file so we can create a new one from defaults.`, ); }); it('should log error and exit when it has invalid keys', () => { fsUtils.readJSONSync.returns({ random: 'values' }); - return expect(() => getConfig(defaultPath)).to.throw( + return expect(getConfig.bind(null, defaultPath)).to.throw( `Config file seems to be corrupted: missing required keys. Please check ${defaultPath}/${configFileName} or delete the file so we can create a new one from defaults.`, ); }); @@ -129,7 +129,9 @@ describe('config utils', () => { describe('when lockfile exists', () => { it('should log error and exit', () => { lockfile.checkSync.returns(true); - return expect(() => setConfig(defaultPath, newConfigValue)).to.throw( + return expect( + setConfig.bind(null, defaultPath, newConfigValue), + ).to.throw( `Config lockfile at ${defaultPath}/${lockfileName} found. Are you running Lisk Commander in another process?`, ); });