fix: Bug Fix (#8)

* Now the init function works correctly with the default parameters
* Load and merge of profiles now respects the priority order

README.md

@@ -72,13 +72,11 @@ config["my-app-name"]
 
 Each configuration file in `auto-config-js` is called a profile configuration. Because of that, configuration files are named using the following convention:
 ```
-app-<PROFILE>.config.yaml
+app.<PROFILE>.config.yaml
 ```
 
 The `<PROFILE>` placeholder uses by default the `NODE_ENV` value. It can be overridden by passing the optional configuration parameter to the `autoConfig` function (check [API](#api)).
 
-If no profile specific file is found, `auto-config-js` will do a last attempt and try to load `app.config.yaml`.
-
 Profiles can be defined hierarchically using the `include` keyword. The include keyword expects an array of profiles names (strings) to be included. The configuration load each file and merge if the current configuration. Example:
 ```yaml
 include: ['base', 'staging']

lib/utils.js

@@ -112,31 +112,24 @@ function loadYamlFile(configDirectory, profile) {
 
 // This function will lead to an infinite loop on circular references
 function loadConfiguration(configDirectory, profile) {
-  const queue = [];
-
-  const rootConfigObject = loadYamlFile(configDirectory, profile);
-
-  let resultConfig = {};
-  let currentConfig = rootConfigObject;
-  let currentProfile = profile;
-  do {
+  const toLoadStack = [profile];
+  const toProcessStack = [];
+
+  while (toLoadStack.length) {
+    const currentProfile = toLoadStack.shift();
+    const currentConfig = loadYamlFile(configDirectory, currentProfile);
+    toProcessStack.unshift(currentConfig);
+    
     if (currentConfig.include) {
       if (!Array.isArray(currentConfig.include)) {
         throw new Error(
           `Include field must be an array in profile: ${profile}!`
         );
       }
-      currentConfig.include.forEach(p => queue.unshift(p));
-    }
-
-    currentProfile = queue.shift();
-    if (currentProfile) {
-      currentConfig = loadYamlFile(configDirectory, currentProfile);
-      resultConfig = mergeDeep(resultConfig, currentConfig);
+      toLoadStack.unshift(...currentConfig.include.reverse());
     }
-  } while (queue.length);
-
-  return mergeDeep(resultConfig, rootConfigObject);
+  }
+  return toProcessStack.reduce((acc, current) => mergeDeep(acc, current));
 }
 
 module.exports = {

package.json

@@ -61,7 +61,15 @@
     "collectCoverage": true,
     "collectCoverageFrom": [
       "lib/**/*.js"
-    ]
+    ],
+    "coverageThreshold": {
+      "global": {
+        "branches": 90,
+        "functions": 90,
+        "lines": 90,
+        "statements": -15
+      }
+    }
   },
   "resolutions": {
     "lodash": "^4.17.19"

test/autoConfig.test.js

@@ -37,7 +37,7 @@ describe('test autoConfig', () => {
     spy.mockRestore();
   });
 
-  it('should not contain keyword include in the config object', () => {
+  it('should not contain keyword [include] in the config object', () => {
     autoConfig.init();
     expect(autoConfig.getConfig().include).toBeUndefined();
   });
@@ -68,4 +68,12 @@ describe('test autoConfig', () => {
     autoConfig.init();
     expect(autoConfig.getConfig()).toEqual(mockConfig);
   });
+
+  it('should work with default params', () => {
+    autoConfig.init();
+    expect(utils.loadConfiguration).toHaveBeenCalledWith(
+      process.cwd(),
+      process.env.NODE_ENV
+    );
+  });
 });

test/utils.test.js

@@ -1,8 +1,17 @@
+jest.mock('fs');
+jest.mock('path');
+jest.mock('js-yaml');
+
+const path = require('path');
+const yaml = require('js-yaml');
+
 const {
   hasValue,
   getPropertyCaseInsensitive,
   setPropertyCaseInsensitive,
   overrideConfigValuesFromSystemVariables,
+  mergeDeep,
+  loadConfiguration,
 } = require('../lib/utils');
 
 it('hasValue', () => {
@@ -157,3 +166,172 @@ describe('overrideConfigValuesFromSystemVariables', () => {
     );
   });
 });
+
+describe('mergeDeep', () => {
+  it('should merge empty objects', () => {
+    expect(mergeDeep({}, {})).toEqual({});
+  });
+
+  it('should create non existing properties in the target object', () => {
+    expect(mergeDeep({}, { test: 1 })).toEqual({ test: 1 });
+  });
+
+  it('should not remove any existing properties from target object', () => {
+    expect(mergeDeep({ test: 1 }, {})).toEqual({ test: 1 });
+  });
+
+  it('should override target property', () => {
+    expect(mergeDeep({ test: 1 }, { test: 'test' })).toEqual({ test: 'test' });
+  });
+
+  it('should override nested objects correctly', () => {
+    const target = { a: 1, b: { c: 'c', d: { e: 'e', f: 'f' } } };
+    const source = { b: { c: 'cTest', d: { e: 'eTest' } } };
+    expect(mergeDeep(target, source)).toEqual({
+      a: 1,
+      b: { c: 'cTest', d: { e: 'eTest', f: 'f' } },
+    });
+  });
+});
+
+describe('loadConfiguration', () => {
+  it('should fail if include tag is invalid', () => {
+    yaml.safeLoad = jest.fn(() => ({ include: 1 }));
+    expect(() => loadConfiguration('mockPath', 'mockProfile')).toThrowError(
+      /Include field must be an array in profile: mockProfile!/
+    );
+
+    yaml.safeLoad = jest.fn(() => ({ include: 'test' }));
+    expect(() => loadConfiguration('mockPath', 'mockProfile')).toThrowError(
+      /Include field must be an array in profile: mockProfile!/
+    );
+
+    yaml.safeLoad = jest.fn(() => ({ include: {} }));
+    expect(() => loadConfiguration('mockPath', 'mockProfile')).toThrowError(
+      /Include field must be an array in profile: mockProfile!/
+    );
+  });
+
+  it('should load config with no includes', () => {
+    yaml.safeLoad = jest.fn(() => ({ myApp: 'mock' }));
+    expect(loadConfiguration('mockPath', 'mockProfile')).toEqual({
+      myApp: 'mock',
+    });
+  });
+
+  it('should load config with single include', () => {
+    yaml.safeLoad = jest
+      .fn()
+      .mockImplementationOnce(() => ({
+        myApp: 'mock',
+        include: ['subProfile'],
+      }))
+      .mockImplementationOnce(() => ({ test: 1 }));
+
+    expect(loadConfiguration('mockPath', 'mockProfile')).toEqual({
+      myApp: 'mock',
+      test: 1,
+      include: ['subProfile'],
+    });
+  });
+
+  it('should load config with multiple includes', () => {
+    yaml.safeLoad = jest
+      .fn()
+      .mockImplementationOnce(() => ({
+        myApp: 'mock',
+        include: ['subProfile', 'subProfile2'],
+      }))
+      .mockImplementationOnce(() => ({ test: 1 }))
+      .mockImplementationOnce(() => ({ mock: 2 }));
+
+    expect(loadConfiguration('mockPath', 'mockProfile')).toEqual({
+      myApp: 'mock',
+      test: 1,
+      mock: 2,
+      include: expect.anything(),
+    });
+  });
+
+  it('should load config with nested includes', () => {
+    yaml.safeLoad = jest
+      .fn()
+      .mockImplementationOnce(() => ({
+        myApp: 'mock',
+        include: ['subProfile'],
+      }))
+      .mockImplementationOnce(() => ({ test: 1, include: ['subProfile2'] }))
+      .mockImplementationOnce(() => ({ mock: 2 }));
+
+    expect(loadConfiguration('mockPath', 'mockProfile')).toEqual({
+      myApp: 'mock',
+      test: 1,
+      mock: 2,
+      include: ['subProfile'],
+    });
+  });
+
+  it('should load respect merge priority', () => {
+    yaml.safeLoad = jest
+      .fn()
+      .mockImplementationOnce(() => ({
+        myApp: 'root',
+        include: ['subProfile', 'subProfile3'],
+      }))
+      .mockImplementationOnce(() => ({
+        myApp: 'subProfile3',
+        test: 'subProfile3',
+        include: ['subProfile4', 'subProfile5'],
+      }))
+      .mockImplementationOnce(() => ({
+        myApp: 'subProfile5',
+      }))
+      .mockImplementationOnce(() => ({
+        myApp: 'subProfile4',
+      }))
+      .mockImplementationOnce(() => ({
+        myApp: 'subProfile',
+        test: 'test',
+        include: ['subProfile2'],
+      }))
+      .mockImplementationOnce(() => ({ myApp: 'subProfile2' }));
+
+    const mockPath = 'mockPath';
+    expect(loadConfiguration(mockPath, 'mockProfile')).toEqual({
+      myApp: 'root',
+      test: 'subProfile3',
+      include: expect.anything(),
+    });
+
+    expect(path.join).toHaveBeenNthCalledWith(
+      1,
+      mockPath,
+      'app.mockProfile.config.yaml'
+    );
+    expect(path.join).toHaveBeenNthCalledWith(
+      2,
+      mockPath,
+      'app.subProfile3.config.yaml'
+    );
+    expect(path.join).toHaveBeenNthCalledWith(
+      3,
+      mockPath,
+      'app.subProfile5.config.yaml'
+    );
+    expect(path.join).toHaveBeenNthCalledWith(
+      4,
+      mockPath,
+      'app.subProfile4.config.yaml'
+    );
+    expect(path.join).toHaveBeenNthCalledWith(
+      5,
+      mockPath,
+      'app.subProfile.config.yaml'
+    );
+    expect(path.join).toHaveBeenNthCalledWith(
+      6,
+      mockPath,
+      'app.subProfile2.config.yaml'
+    );
+  });
+});