refactored the way cli config file is read because it was causing sco…

…pe pollution when running tests, added more tests for addClint

bin/.config.json

@@ -1,5 +1,8 @@
 {
   "config": {
     "filePath": null
-  }
+  },
+  "queries": [],
+  "dataClients": [],
+  "jsonConfig": {}
 }

bin/__tests__/commands/add/client.test.js

@@ -1,22 +1,26 @@
 const importedModule = require('../../../commands/add/client');
+const { getConfig, setConfig } = require('../../../lib/config');
 
+const { addClient, clientExists } = importedModule;
 const validFamily = 'mongodb';
-const validOrigin = 'whatever';
 const validTypeSource = 'source';
 const validTypeTarget = 'target';
-const validName = 'anyName';
+const validSourceName = 'my-source-db';
+const validTargetName = 'my-target-db';
+const validOrigin = validSourceName;
 const validUrl = 'mongodb://localhost:27017';
 const validBaseClient = {
   family: validFamily,
-  name: validName,
   url: validUrl,
 };
 const validSourceClient = {
   type: validTypeSource,
+  name: validSourceName,
   ...validBaseClient,
 };
 const validTargetClient = {
   type: validTypeTarget,
+  name: validTargetName,
   origin: validOrigin,
   ...validBaseClient,
 };
@@ -46,85 +50,107 @@ describe('weaver add client command tests', () => {
 });
 
 describe('addClient', () => {
-  test('returns false if config is undefined', () => {
-    const arg1 = undefined;
-    const expected = false;
-    const result = importedModule.addClient(arg1);
-    expect(result).toBe(expected);
+  describe('returns false if config is invalid', () => {
+    test('returns false if config is undefined', () => {
+      const arg1 = undefined;
+      const expected = false;
+      const result = importedModule.addClient(arg1);
+      expect(result).toBe(expected);
+    });
+    test('returns false if config is empty obj', () => {
+      const arg1 = {};
+      const expected = false;
+      const result = importedModule.addClient(arg1);
+      expect(result).toBe(expected);
+    });
+    test('returns false if config is invalid, mix', () => {
+      const case1 = {
+        family: '', origin: '', type: '', name: '', url: '',
+      };
+      const case2 = {
+        family: 0, origin: null, type: false, name: null, url: undefined,
+      };
+      const case3 = {
+        family: validFamily,
+        origin: null,
+        type: validTypeTarget,
+        name: validSourceName,
+        url: validUrl,
+      };
+      const expected = false;
+
+      const result1 = importedModule.addClient(case1);
+      const result2 = importedModule.addClient(case2);
+      const result3 = importedModule.addClient(case3);
+
+      expect(result1).toBe(expected);
+      expect(result2).toBe(expected);
+      expect(result3).toBe(expected);
+    });
   });
-  test('returns false if config is empty obj', () => {
-    const arg1 = {};
-    const expected = false;
-    const result = importedModule.addClient(arg1);
-    expect(result).toBe(expected);
-  });
-  test('returns false if config is invalid', () => {
-    const case1 = {
-      family: '', origin: '', type: '', name: '', url: '',
-    };
-    const case2 = {
-      family: 0, origin: null, type: false, name: null, url: undefined,
-    };
-    const case3 = {
-      family: validFamily,
-      origin: null,
-      type: validTypeTarget,
-      name: validName,
-      url: validUrl,
-    };
-    const expected = false;
-
-    const result1 = importedModule.addClient(case1);
-    const result2 = importedModule.addClient(case2);
-    const result3 = importedModule.addClient(case3);
-
-    expect(result1).toBe(expected);
-    expect(result2).toBe(expected);
-    expect(result3).toBe(expected);
-  });
-  test('returns the new config object if config is valid', () => {
-    const result1 = importedModule.addClient(validTargetClient);
-    const result2 = importedModule.addClient(validSourceClient);
-
-    expect(result1.config).not.toBe(undefined);
-    expect(result1.dataClients).not.toBe(undefined);
-    expect(result1.dataClients[0].db.name).toBe(validName);
-    expect(result1.dataClients[0].db.url).toBe(validUrl);
-    expect(result1.dataClients[0].family).toBe(validFamily);
-    expect(result1.dataClients[0].origin).toBe(validOrigin);
-    expect(result1.dataClients[0].type).toBe(validTypeTarget);
-    expect(result1.jsonConfig).not.toBe(undefined);
-    expect(result1.queries).not.toBe(undefined);
-
-    expect(result2.config).not.toBe(undefined);
-    expect(result2.dataClients).not.toBe(undefined);
-    expect(result2.dataClients[0].db.name).toBe(validName);
-    expect(result2.dataClients[0].db.url).toBe(validUrl);
-    expect(result2.dataClients[0].family).toBe(validFamily);
-    expect(result2.dataClients[0].origin).toBe(undefined);
-    expect(result2.dataClients[0].type).toBe(validTypeSource);
-    expect(result2.jsonConfig).not.toBe(undefined);
-    expect(result2.queries).not.toBe(undefined);
+
+  describe('returns the new config object if clients are valid', () => {
+    const initialConfig = { ...getConfig() };
+    afterEach(() => { setConfig(initialConfig); });
+
+    test('returns the new config object if source is valid', () => {
+      const result = addClient(validSourceClient);
+      expect(result.config).not.toBe(undefined);
+      expect(result.dataClients).not.toBe(undefined);
+      expect(result.dataClients[0].db.name).toBe(validSourceName);
+      expect(result.dataClients[0].db.url).toBe(validUrl);
+      expect(result.dataClients[0].family).toBe(validFamily);
+      expect(result.dataClients[0].origin).toBe(undefined);
+      expect(result.dataClients[0].type).toBe(validTypeSource);
+      expect(result.jsonConfig).not.toBe(undefined);
+      expect(result.queries).not.toBe(undefined);
+    });
+    test('returns the new config object if target is valid', () => {
+      const result = addClient(validTargetClient);
+      expect(result.config).not.toBe(undefined);
+      expect(result.dataClients).not.toBe(undefined);
+      expect(result.dataClients[0].db.name).toBe(validTargetName);
+      expect(result.dataClients[0].db.url).toBe(validUrl);
+      expect(result.dataClients[0].family).toBe(validFamily);
+      expect(result.dataClients[0].origin).toBe(validOrigin);
+      expect(result.dataClients[0].type).toBe(validTypeTarget);
+      expect(result.jsonConfig).not.toBe(undefined);
+      expect(result.queries).not.toBe(undefined);
+    });
   });
 });
 
 describe('clientExists', () => {
-  test('returns false if not already saved', () => {
-    const expected = false;
-    const result1 = importedModule.clientExists(validSourceClient);
-    const result2 = importedModule.clientExists(validTargetClient);
-
-    expect(result1.exists).toBe(expected);
-    expect(result2.exists).toBe(expected);
+  describe('returns false if not already saved', () => {
+    test('returns false if not already saved', () => {
+      const expected = false;
+      const result1 = clientExists(validSourceClient);
+      const result2 = clientExists(validTargetClient);
+
+      expect(result1.exists).toBe(expected);
+      expect(result2.exists).toBe(expected);
+    });
+  });
+  describe('returns true if already saved', () => {
+    const initialConfig = { ...getConfig() };
+    beforeAll(() => {
+      const sourceClient = addClient(validSourceClient);
+      setConfig(sourceClient);
+
+      const targetClient = addClient(validTargetClient);
+      setConfig(targetClient);
+    });
+    afterAll(() => { setConfig(initialConfig); });
+
+    test('returns true if already saved', () => {
+      const expected = true;
+      const result1 = importedModule.clientExists(validSourceClient);
+      const result2 = importedModule.clientExists(validTargetClient);
+
+      expect(result1.exists).toBe(expected);
+      expect(result2.exists).toBe(expected);
+    });
   });
-  // test('returns true if already saved', () => {
-  //   const expected = false;
-  //   const result1 = importedModule.clientExists(validSourceClient);
-  //   const result2 = importedModule.clientExists(validTargetClient);
-
-  //   expect(result1.exists).toBe(expected);
-  //   expect(result2.exists).toBe(expected);
-  // });
 });
 
 // describe('commandName', () => {

bin/__tests__/options/config.test.js

@@ -3,10 +3,8 @@ const logging = require('debug')('Weaver:bin:__tests__:options:config');
 const shell = require('shelljs');
 const path = require('path');
 const { DEFAULT_CONFIG_PATH } = require('../../lib/constants');
-const CFG = require('./../../.config');
 const {
   pathExists,
-  getJSONContent,
   isJSONFile,
   isValidConfigObject,
   validateConfig,
@@ -15,12 +13,14 @@ const {
   saveConfigPath,
   applyConfig,
 } = require('../../options/config');
+const { getCLIJSONContent, getJSONContent } = require('../../options/shared');
 
 const CLI_DIR = path.resolve(__dirname, '../../cli.js');
 const MOCKS_DIR = path.resolve(__dirname, '../__mock__');
 
 
 describe('weaver --config tests', () => {
+  const CFG = getCLIJSONContent();
   afterAll(() => {
     logging('restoring config to original values', CFG.config.filePath);
     saveConfigPath(CFG.config.filePath);
@@ -38,13 +38,11 @@ describe('weaver --config tests', () => {
     const valid = `${MOCKS_DIR}/.weaver.mock.json`;
 
     expect(pathExists(invalid)).toEqual(false);
-    expect(getJSONContent(invalid)).toEqual(null);
     expect(isJSONFile(invalid)).toEqual(false);
     expect(validateConfig(invalid).valid).toEqual(false);
     expect(isValidConfigObject(invalid)).toEqual(false);
 
     expect(pathExists(valid)).toEqual(true);
-    expect(getJSONContent(valid)).not.toEqual(null);
     expect(isJSONFile(valid)).toEqual(true);
     expect(validateConfig(valid).valid).toEqual(true);
     expect(isValidConfigObject(getJSONContent(valid))).toEqual(true);

bin/__tests__/options/shared.test.js

@@ -1,15 +1,23 @@
+const HOMEDIR = require('os').homedir();
+const path = require('path');
 const logging = require('debug')('Weaver:bin:__tests__:options:shared');
 const {
   absPathname,
+  getJSONContent,
 } = require('../../options/shared');
 
+const MOCKS_DIR = path.resolve(__dirname, '../__mock__');
 
 describe('weaver argument parser tests', () => {
   beforeAll(() => {
     logging('beforeAll');
   });
 
   test('Base behavior', () => {
+    const invalid = `${HOMEDIR}/random/.weaver.json`;
+    const valid = `${MOCKS_DIR}/.weaver.mock.json`;
     expect(absPathname(process.env.PWD)).toEqual(process.env.PWD);
+    expect(getJSONContent(invalid)).toEqual(null);
+    expect(getJSONContent(valid)).not.toEqual(null);
   });
 });

bin/lib/config.js

@@ -1,18 +1,18 @@
 const fs = require('fs');
 const shell = require('shelljs');
-const CLI_CONFIG = require('../.config');
 
 const {
   getJSONContent,
   validateConfig,
   isValidConfigObject,
 } = require('../options/config');
-const { absPathname } = require('../options/shared');
+const { absPathname, getCLIJSONContent } = require('../options/shared');
 
 let CLI_ARGS = {};
 
 const readConfigFile = () => {
   let config = {};
+  const CLI_CONFIG = getCLIJSONContent();
   const path = CLI_CONFIG.filePath;
   const isValid = validateConfig(path).valid;
   if (isValid) {
@@ -42,6 +42,7 @@ const parseCLIConfig = () => {
 
 const readCLISettings = () => {
   const stdin = parseCLIConfig();
+  const CLI_CONFIG = getCLIJSONContent();
   const queries = stdin.queries || CLI_CONFIG.queries;
   const dataClients = stdin.dataClients || CLI_CONFIG.dataClients;
   const jsonConfig = stdin.jsonConfig || CLI_CONFIG.jsonConfig;

bin/options/config.js

@@ -3,14 +3,12 @@ const fs = require('fs');
 const Debug = require('debug');
 const shell = require('shelljs');
 
-const CFG = require('../.config');
 const {
   TEST_NODE_ENV,
   REQUIRED_CONFIG_KEYS,
 } = require('../lib/constants');
 const { InvalidJSONFileError } = require('../lib/utils');
-const { absPathname } = require('./shared');
-
+const { cfgAbsPath, getJSONContent, getCLIJSONContent } = require('./shared');
 
 const logging = Debug('Weaver:bin:options:config');
 
@@ -23,22 +21,6 @@ const pathExists = (pathname) => {
   }
 };
 
-const getJSONContent = (filePath) => {
-  logging(`getJSONContent:filePath ${filePath}`);
-
-  try {
-    const content = fs.readFileSync(filePath);
-    const parsedContent = JSON.parse(content);
-    logging(`getJSONContent:content ${content}`);
-    return parsedContent;
-  } catch (error) {
-    const errorName = error.constructor.prototype.name.split(' ')[0];
-    const handledErrs = ['SyntaxError', 'TypeError'];
-    if (handledErrs.indexOf(errorName)) return null;
-    throw error;
-  }
-};
-
 const isJSONFile = (filePath) => {
   const parsedContent = getJSONContent(filePath);
   const isJSONContent = parsedContent instanceof Object;
@@ -70,6 +52,7 @@ const validateConfig = (filePath) => {
 };
 
 const showConfig = () => {
+  const CFG = getCLIJSONContent();
   const cfgString = JSON.stringify(CFG, null, 2);
   if (!TEST_NODE_ENV) shell.echo(cfgString);
   return false;
@@ -84,7 +67,7 @@ const validationFeedback = (validation, filePath) => {
 };
 
 const saveConfigPath = (configFilePath) => {
-  const cfgAbsPath = absPathname(__dirname, './../.config.json');
+  const CFG = getCLIJSONContent();
   const configContent = ld.assign({}, CFG, { config: { filePath: configFilePath } });
   try {
     fs.writeFileSync(cfgAbsPath, JSON.stringify(configContent, null, 2));
@@ -105,7 +88,6 @@ const applyConfig = (configFilePath) => {
 
 module.exports = {
   pathExists,
-  getJSONContent,
   isJSONFile,
   isValidConfigObject,
   validateConfig,