fix: fixes #16, fixes #17

packages/cli/package.json

@@ -49,7 +49,7 @@
     "@types/js-yaml": "^3.12.5",
     "@types/node": "^14.14.10",
     "@types/npmlog": "^4.1.2",
-    "@types/yargs": "^15.0.11",
+    "@types/yargs": "^16.0.0",
     "faker": "^5.1.0",
     "jest": "^26.6.3",
     "ts-jest": "^26.4.4",

packages/cli/src/cli.spec.ts

@@ -1,11 +1,14 @@
 /* eslint-disable import/first */
 import * as faker from 'faker';
 
+const buildFolder = faker.random.word();
+
 const optionsFromConfigFiles = {
   option: faker.random.word(),
   optionEnv: faker.random.word(),
   environments: {
     Production: {
+      buildFolder,
       optionEnv: faker.random.word(),
     },
   },
@@ -15,42 +18,50 @@ jest.mock('deepmerge', () => ({
   all: jest.fn().mockReturnValue(optionsFromConfigFiles),
 }));
 
+jest.mock('./deploy/staticApp/staticApp', () => ({
+  deployStaticApp: jest.fn(),
+}));
+
 import cli from './cli';
 
-describe('handle options correctly', () => {
-  test('argv must have the options passed to CLI', () => {
+import { deployStaticApp } from './deploy/staticApp/staticApp';
+
+describe('handle merge config correctly', () => {
+  describe('Config merging errors when default values is present #16 https://github.com/ttoss/carlin/issues/16', () => {
+    test('deploy static-app --build-folder should not be the default', async () => {
+      await cli().parse('deploy static-app', { environment: 'Production' });
+      expect(deployStaticApp).toHaveBeenCalledWith(
+        expect.objectContaining({
+          buildFolder,
+        }),
+      );
+    });
+  });
+
+  test('argv must have the options passed to CLI', async () => {
     const options = {
       region: faker.random.word(),
     };
-    cli.parse('print-args', options, (_err, argv) => {
-      expect(argv).toMatchObject(options);
-      expect(argv).toMatchObject(options);
-    });
+    const argv = await cli().parse('print-args', options);
+    expect(argv.environment).toBeUndefined();
+    expect(argv).toMatchObject(options);
   });
 
-  test('argv must have the environment option', () => {
-    cli.parse(
-      'print-args',
-      { environment: 'Production' },
-      (_err: any, argv: any) => {
-        expect(argv.optionEnv).toEqual(
-          optionsFromConfigFiles.environments.Production.optionEnv,
-        );
-      },
+  test('argv must have the environment option', async () => {
+    const argv = await cli().parse('print-args', { environment: 'Production' });
+    expect(argv.environment).toBe('Production');
+    expect(argv.optionEnv).toEqual(
+      optionsFromConfigFiles.environments.Production.optionEnv,
     );
   });
 
-  test('argv must have the CLI optionEnv', () => {
+  test('argv must have the CLI optionEnv', async () => {
     const newOptionEnv = faker.random.word();
-    cli.parse(
-      'print-args',
-      {
-        environment: 'Production',
-        optionEnv: newOptionEnv,
-      },
-      (_err: any, argv: any) => {
-        expect(argv.optionEnv).toEqual(newOptionEnv);
-      },
-    );
+    const argv = await cli().parse('print-args', {
+      environment: 'Production',
+      optionEnv: newOptionEnv,
+    });
+    expect(argv.environment).toBe('Production');
+    expect(argv.optionEnv).toEqual(newOptionEnv);
   });
 });

packages/cli/src/cli.ts

@@ -1,4 +1,5 @@
-import { constantCase } from 'change-case';
+/* eslint-disable no-param-reassign */
+import { constantCase, paramCase } from 'change-case';
 import deepmerge from 'deepmerge';
 import findUp from 'find-up';
 import path from 'path';
@@ -9,40 +10,10 @@ import { NAME } from './config';
 import { deployCommand } from './deploy/command';
 import { addGroupToOptions, setEnvironment, readObjectFile } from './utils';
 
-let finalConfig: any;
-
-/**
- * Get all carlin configs from directories.
- */
-export const getConfig = () => {
-  const names = ['js', 'yml', 'yaml', 'json'].map((ext) => `${NAME}.${ext}`);
-  const paths = [];
-  let currentPath = process.cwd();
-  let findUpPath: string | undefined;
-
-  do {
-    findUpPath = findUp.sync(names, { cwd: currentPath });
-    if (findUpPath) {
-      currentPath = path.resolve(findUpPath, '../..');
-      paths.push(findUpPath);
-    }
-  } while (findUpPath);
-
-  const configs = paths.map((p) => readObjectFile({ path: p }) || {});
-
-  /**
-   * Using configs.reverser() to get the most far config first. This way the
-   * nearest configs will replace others.
-   */
-  finalConfig = deepmerge.all(configs.reverse());
-
-  return finalConfig;
-};
-
 export const options = {
   config: {
     alias: 'c',
-    describe: 'Path to JavaScript, JSON or YAML file.',
+    describe: 'Path to config file.',
     require: false,
     type: 'string',
   },
@@ -59,39 +30,112 @@ export const options = {
   environments: {},
 } as const;
 
-yargs
-  .scriptName(NAME)
-  .env(constantCase(NAME))
-  .options(addGroupToOptions(options, 'Common Options'))
-  .middleware((argv) => {
-    const { environment, environments } = argv as any;
+/**
+ * Transformed to method because finalConfig was failing the tests.
+ */
+const cli = () => {
+  /**
+   * All config files merged.
+   */
+  let finalConfig: any;
+
+  /**
+   * Get all carlin configs from directories.
+   */
+  const getConfig = () => {
+    const names = ['js', 'yml', 'yaml', 'json'].map((ext) => `${NAME}.${ext}`);
+    const paths = [];
+    let currentPath = process.cwd();
+    let findUpPath: string | undefined;
+
+    do {
+      findUpPath = findUp.sync(names, { cwd: currentPath });
+      if (findUpPath) {
+        currentPath = path.resolve(findUpPath, '../..');
+        paths.push(findUpPath);
+      }
+    } while (findUpPath);
+
+    const configs = paths.map((p) => readObjectFile({ path: p }) || {});
 
     /**
-     * Create final options with environment and environments.
+     * Using configs.reverser() to get the most far config first. This way the
+     * nearest configs will replace others.
      */
-    if (environment && environments && environments[environment as string]) {
-      Object.entries(environments[environment]).forEach(([key, value]) => {
-        /**
-         * Fixes #13 https://github.com/ttoss/carlin/issues/13
-         */
-        if (!argv[key] || argv[key] === finalConfig[key]) {
-          // eslint-disable-next-line no-param-reassign
-          argv[key] = value;
-        }
-      });
-    }
-  })
-  .pkgConf(NAME)
-  .config(getConfig())
-  .config('config', (configPath: string) =>
-    readObjectFile({ path: configPath }),
-  )
-  .command({
-    command: 'print-args',
-    describe: false,
-    handler: (argv) => console.log(JSON.stringify(argv, null, 2)),
-  })
-  .command(deployCommand)
-  .help();
-
-export default yargs;
+    finalConfig = deepmerge.all(configs.reverse());
+
+    return finalConfig;
+  };
+
+  return yargs
+    .scriptName(NAME)
+    .env(constantCase(NAME))
+    .options(addGroupToOptions(options, 'Common Options'))
+    .middleware(((argv: any, { parsed }: any) => {
+      const { environment, environments } = argv;
+
+      /**
+       * Create final options with environment and environments.
+       */
+      if (environment && environments && environments[environment as string]) {
+        Object.entries(environments[environment]).forEach(([key, value]) => {
+          /**
+           * The case where argv[key] must not have the environment value is
+           * when such value is passed as option via CLI. For instance,
+           *
+           * $ carlin deploy --stack-name SomeName
+           *
+           * SomeName must be used as stack name independently of the
+           * environment values https://github.com/ttoss/carlin/issues/13.
+           *
+           * Three cases set argv:
+           *
+           * 1. Default.
+           * 2. Config file.
+           * 3. CLI
+           *
+           * - Case 1 we determine if the parsed.defaulted is true.
+           * - Case 2 we determine if `argv[key] === finalConfig[key]`.
+           * - Case 3 if the two above are falsy.
+           */
+          const isKeyFormCli = (() => {
+            const paramCaseKey = paramCase(key);
+
+            /**
+             * Fixes #16 https://github.com/ttoss/carlin/issues/16
+             */
+            if (parsed?.defaulted?.[paramCaseKey]) {
+              return false;
+            }
+
+            /**
+             * Fixes #13 https://github.com/ttoss/carlin/issues/13
+             */
+            if (argv[key] === finalConfig[key]) {
+              return false;
+            }
+
+            return true;
+          })();
+
+          if (!isKeyFormCli) {
+            argv[key] = value;
+          }
+        });
+      }
+    }) as any)
+    .pkgConf(NAME)
+    .config(getConfig())
+    .config('config', (configPath: string) =>
+      readObjectFile({ path: configPath }),
+    )
+    .command({
+      command: 'print-args',
+      describe: false,
+      handler: (argv) => console.log(JSON.stringify(argv, null, 2)),
+    })
+    .command(deployCommand)
+    .help();
+};
+
+export default cli;

packages/cli/src/deploy/command.ts

@@ -101,14 +101,31 @@ export const options = {
   },
 } as const;
 
+export const examples: ReadonlyArray<[string, string?]> = [
+  [
+    'carlin deploy -t src/cloudformation.template1.yml',
+    'Change the CloudFormation template path.',
+  ],
+  ['carlin deploy -e Production', 'Set environment.'],
+  [
+    'carlin deploy --lambda-input src/lambda/index.ts --lambda-externals momentjs',
+    "Lambda exists. Don't bundle momentjs.",
+  ],
+  [
+    'carlin deploy --destroy --stack-name StackToBeDeleted',
+    'Destroy a specific stack.',
+  ],
+];
+
 export const deployCommand: CommandModule<
   any,
   yargs.InferredOptionTypes<typeof options>
 > = {
-  command: 'deploy [specific]',
+  command: 'deploy [deploy]',
   describe: 'Deploy cloud resources.',
   builder: (yargsBuilder) => {
     yargsBuilder
+      .example(examples)
       .options(addGroupToOptions(options, 'Deploy Options'))
       /**
        * Set AWS region.

packages/cli/src/deploy/s3.ts

@@ -182,7 +182,7 @@ export const uploadDirectoryToS3 = async ({
 }) => {
   log.info(
     logPrefix,
-    `Uploading directory ${directory} to ${bucket}/${bucketKey}...`,
+    `Uploading directory ${directory}/ to ${bucket}/${bucketKey}...`,
   );
 
   /**
@@ -202,6 +202,14 @@ export const uploadDirectoryToS3 = async ({
      */
     .filter((item) => fs.lstatSync(item).isFile());
 
+  /**
+   * If the folder has no files (the folder name may be wrong), thrown an
+   * error. Discovered at #16 https://github.com/ttoss/carlin/issues/16.
+   */
+  if (allFiles.length === 0) {
+    throw new Error(`Directory ${directory}/ has no files.`);
+  }
+
   const GROUP_MAX_LENGTH = 63;
 
   const numberOfGroups = Math.ceil(allFiles.length / GROUP_MAX_LENGTH);