Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Remove process exit from config commands - closes #580 #586

Merged
merged 4 commits into from
Jul 31, 2018
Merged

Remove process exit from config commands - closes #580 #586

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cf66afd
:recycle: Remove process.exit from config commands
yatki Jul 30, 2018
f849f0a
:white_check_mark: Fix config command tests
yatki Jul 30, 2018
8b63232
:recycle: Remove logger.js
yatki Jul 31, 2018
35ae8da
:recycle: Rewrite getConfig & setConfig statements in tests
yatki Jul 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/utils/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was actually the only place the logger was used.
Since we use the logger from the oclif now, i think we are good to delete this function


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);
}
};

Expand All @@ -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);
}
};

Expand Down
29 changes: 8 additions & 21 deletions test/utils/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,17 @@ 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';

const defaultPath = `${os.homedir()}/${configDirName}`;

beforeEach(() => {
sandbox.stub(fsUtils, 'writeJSONSync');
sandbox.stub(logger, 'error');
sandbox.stub(process, 'exit');
return Promise.resolve();
});

Expand All @@ -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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In other test suite, we do something like getConfig.bind(null, defaultPath)
so as other places below.

`Could not write to \`${defaultPath}\`. Your configuration will not be persisted.`,
);
});
});
Expand All @@ -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.`,
);
});
});
Expand All @@ -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.`,
);
});
Expand All @@ -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?`,
);
});
Expand Down