[REFACTOR] project init command (#203)

* replace any with interface

* correct the type definition for IVariableGroupDataVariable[]

* [REFACTOR] making code compact

* add test to create-variable-group command

* breaking larger function in scaffold class into smaller one, so we can add tests

* simplier commander interface

* using type guard in hasValue function

* incorporate Evan's feedback

* fix jsdoc

* fixed the problem where definition.yaml file is not created

* [REFACTOR] commands/hld/init.ts and add tests

1. move the command decoration out to a JSON file
2. standardized on command decorator function name, initCommandDecorator -> commandDecorator
3. added support for having default values in command builder
4. added tests to boost the coverage from 59% to 89%
5. create helper functions in tests so test code is more manage-able

* modified code to make sure that log is flushed before the command exit

* adding missing Promise<void> return type

* [REFACTOR] project init command

1. have command decoration to a separation JSON file
2. improve test coverage from 60 to 80%.
3. create helper functions for creating random dir
4. create helper functions for checking if file exists
5. remove unused function (ls) in init.ts
6. proper exit code for init.ts
7. make reduce function in init.ts more readable

successfully ran validations.sh

* have ringName = uuid() in test

* fix typo

* remove hld-lifecycle.yaml

* add doc to test

Co-authored-by: Yvonne Radsmikham <yvonne.radsmikham@gmail.com>
Co-authored-by: Andre Briggs <andrebriggs@users.noreply.github.com>

Author: 3 people

src/commands/project/index.ts

@@ -1,6 +1,6 @@
 import { Command } from "../command";
 import { commandDecorator as createVariableGroupCommandDecorator } from "./create-variable-group";
-import { initCommandDecorator } from "./init";
+import { commandDecorator as initCommandDecorator } from "./init";
 import { deployLifecyclePipelineCommandDecorator } from "./pipeline";
 
 export const projectCommand = Command(

src/commands/project/init.decorator.json

@@ -0,0 +1,13 @@
+{
+  "command": "init",
+  "alias": "i",
+  "description": "Initialize your spk repository. Add starter bedrock.yaml, maintainers.yaml, hld-lifecycle.yaml, and .gitignore files to your project.",
+  "options": [
+    {
+      "arg": "-r, --default-ring <branch-name>",
+      "description": "Specify a default ring; this corresponds to a default branch which you wish to push initial revisions to",
+      "required": false,
+      "defaultValue": "master"
+    }
+  ]
+}

src/commands/project/init.test.ts

@@ -1,51 +1,36 @@
-import fs from "fs";
-import os from "os";
-import path from "path";
 import uuid from "uuid/v4";
 import { Bedrock, Maintainers, write } from "../../config";
+import { createTempDir, getMissingFilenames } from "../../lib/ioUtil";
 import { IBedrockFile, IMaintainersFile } from "../../types";
-import { initialize } from "./init";
+import { execute, initialize } from "./init";
+import * as init from "./init";
 
-/**
- * Helper to create a new random directory to initialize
- */
-const createNewProject = () => {
-  // Create random directory to initialize
-  const randomTmpDir = path.join(os.tmpdir(), uuid());
-  fs.mkdirSync(randomTmpDir);
-  return randomTmpDir;
-};
+const CREATED_FILES = [
+  ".gitignore",
+  "bedrock.yaml",
+  "maintainers.yaml",
+  "hld-lifecycle.yaml"
+];
 
 describe("Initializing a blank/new bedrock repository", () => {
   test("all standard files get generated in the project root on init", async () => {
-    // init
-    const randomTmpDir = createNewProject();
+    const randomTmpDir = createTempDir();
     await initialize(randomTmpDir);
 
     // bedrock.yaml, maintainers.yaml should be in a the root for a 'standard' project
-    const filepathsShouldExist = [
-      ".gitignore",
-      "bedrock.yaml",
-      "maintainers.yaml",
-      "hld-lifecycle.yaml"
-    ].map(filename => path.join(randomTmpDir, filename));
-
-    for (const filepath of filepathsShouldExist) {
-      expect(fs.existsSync(filepath)).toBe(true);
-    }
+    const missing = getMissingFilenames(randomTmpDir, CREATED_FILES);
+    expect(missing.length).toBe(0); // no files are missing hence length 0
 
     // ensure service specific files do not get created
-    const filepathsShouldNotExist = [
+    const unexpected = getMissingFilenames(randomTmpDir, [
       "Dockerfile",
       "azure-pipelines.yaml"
-    ].map(filename => path.join(randomTmpDir, filename));
-    for (const filepath of filepathsShouldNotExist) {
-      expect(fs.existsSync(filepath)).toBe(false);
-    }
+    ]);
+    expect(unexpected.length).toBe(2);
   });
 
   test("defaultRings gets injected successfully", async () => {
-    const randomTmpDir = createNewProject();
+    const randomTmpDir = createTempDir();
     const ringName = uuid();
     await initialize(randomTmpDir, { defaultRing: ringName });
     const bedrock = Bedrock(randomTmpDir);
@@ -55,7 +40,7 @@ describe("Initializing a blank/new bedrock repository", () => {
 
 describe("initializing an existing file does not modify it", () => {
   test("bedrock.yaml does not get modified", async () => {
-    const randomDir = createNewProject();
+    const randomDir = createTempDir();
     const bedrockFile: IBedrockFile = {
       rings: { master: { isDefault: true } },
       services: {
@@ -80,7 +65,7 @@ describe("initializing an existing file does not modify it", () => {
   });
 
   test("maintainers.yaml does not get modified", async () => {
-    const randomDir = createNewProject();
+    const randomDir = createTempDir();
     const maintainersFile: IMaintainersFile = {
       services: {
         "some/random/dir": {
@@ -96,3 +81,34 @@ describe("initializing an existing file does not modify it", () => {
     expect(updatedMaintainers).toStrictEqual(maintainersFile);
   });
 });
+
+describe("Test execute function", () => {
+  it("positive test", async () => {
+    jest.spyOn(init, "initialize");
+    const exitFn = jest.fn();
+    await execute(
+      {
+        defaultRing: "master"
+      },
+      exitFn
+    );
+    expect(exitFn).toBeCalledTimes(1);
+    expect(initialize).toBeCalledTimes(1);
+    expect(exitFn.mock.calls).toEqual([[0]]); // 0: success
+  });
+
+  it("negative test", async () => {
+    jest.spyOn(init, "initialize").mockImplementation(() => {
+      throw new Error();
+    });
+    const exitFn = jest.fn();
+    await execute(
+      {
+        defaultRing: "master"
+      },
+      exitFn
+    );
+    expect(exitFn).toBeCalledTimes(1);
+    expect(exitFn.mock.calls).toEqual([[1]]); // 1: error
+  });
+});

src/commands/project/init.ts

@@ -1,62 +1,54 @@
 import commander from "commander";
 import fs from "fs";
 import path from "path";
-import shelljs from "shelljs";
 import { Bedrock, write } from "../../config";
+import { build as buildCmd, exit as exitCmd } from "../../lib/commandBuilder";
 import {
   generateGitIgnoreFile,
   generateHldLifecyclePipelineYaml
 } from "../../lib/fileutils";
 import { exec } from "../../lib/shell";
 import { logger } from "../../logger";
 import { IBedrockFile, IHelmConfig, IMaintainersFile } from "../../types";
+import decorator from "./init.decorator.json";
 
-/**
- * Adds the init command to the commander command object
- *
- * @param command Commander command object to decorate
- */
-export const initCommandDecorator = (command: commander.Command): void => {
-  command
-    .command("init")
-    .alias("i")
-    .description(
-      "Initialize your spk repository. Will add starter bedrock.yaml, maintainers.yaml, hld-lifecycle.yaml, and .gitignore files to your project."
-    )
-    .option(
-      "-r, --default-ring <branch-name>",
-      "Specify a default ring; this corresponds to a default branch which you wish to push initial revisions to",
-      "master"
-    )
-    .action(async opts => {
-      const { defaultRing } = opts;
-      const projectPath = process.cwd();
+// values that we need to pull out from command operator
+interface ICommandOptions {
+  defaultRing: string;
+}
 
-      try {
-        let bedrockFile: IBedrockFile | undefined;
-        try {
-          bedrockFile = Bedrock();
-        } catch (err) {
-          logger.info(err);
-        }
+export const execute = async (
+  opts: ICommandOptions,
+  exitFn: (status: number) => Promise<void>
+) => {
+  // defaultRing shall always have value (not undefined nor null)
+  // because it has default value as "master"
+  const defaultRing = opts.defaultRing;
+  const projectPath = process.cwd();
+
+  try {
+    const _ = Bedrock(); // TOFIX: Is this to check if Bedrock config exist?
+  } catch (err) {
+    logger.info(err);
+  }
 
-        // Type check all parsed command line args here.
-        if (typeof defaultRing !== "string") {
-          throw new Error(
-            `--default-ring must be of type 'string', '${defaultRing}' of type '${typeof defaultRing}' given`
-          );
-        }
+  try {
+    await initialize(projectPath, { defaultRing });
+    await exitFn(0);
+  } catch (err) {
+    logger.error(`Error occurred while initializing project ${projectPath}`);
+    logger.error(err);
+    await exitFn(1);
+  }
+};
 
-        await initialize(projectPath, {
-          defaultRing
-        });
-      } catch (err) {
-        logger.error(
-          `Error occurred while initializing project ${projectPath}`
-        );
-        logger.error(err);
-      }
+export const commandDecorator = (command: commander.Command): void => {
+  buildCmd(command, decorator).action(async (opts: ICommandOptions) => {
+    await execute(opts, async (status: number) => {
+      await exitCmd(logger);
+      process.exit(status);
     });
+  });
 };
 
 /**
@@ -70,47 +62,21 @@ export const initCommandDecorator = (command: commander.Command): void => {
  */
 export const initialize = async (
   rootProjectPath: string,
-  opts?: {
-    defaultRing?: string;
-  }
+  opts?: ICommandOptions
 ) => {
-  const { defaultRing } = opts || {};
   const absProjectRoot = path.resolve(rootProjectPath);
   logger.info(`Initializing project Bedrock project ${absProjectRoot}`);
 
+  const defaultRing = opts ? [opts.defaultRing] : [];
   // Initialize all paths
-  await generateBedrockFile(
-    absProjectRoot,
-    [],
-    defaultRing ? [defaultRing] : []
-  );
-  await generateMaintainersFile(absProjectRoot, []);
+  await generateBedrockFile(absProjectRoot, [], defaultRing);
+  await generateMaintainersFile(absProjectRoot, []); // TOFIX: packagePaths is hardcoded to []
   await generateHldLifecyclePipelineYaml(absProjectRoot);
   generateGitIgnoreFile(absProjectRoot, "spk.log");
 
   logger.info(`Project initialization complete!`);
 };
 
-/**
- * Helper function for listing files/dirs in a path
- *
- * @param dir path-like string; what you would pass to ls in bash
- */
-const ls = async (dir: string): Promise<string[]> => {
-  const lsRet = shelljs.ls(dir);
-  if (lsRet.code !== 0) {
-    logger.error(lsRet.stderr);
-    throw new Error(
-      `Error listing files in ${dir}; Ensure this directory exists or specify a different one with the --packages-dir option.`
-    );
-  }
-
-  // Returned object includes piping functions as well; strings represent the actual output of the function
-  const filesAndDirectories = lsRet.filter(out => typeof out === "string");
-
-  return filesAndDirectories;
-};
-
 /**
  * Writes out a default maintainers.yaml file
  *
@@ -199,8 +165,19 @@ const generateBedrockFile = async (
   const absPackagePaths = packagePaths.map(p => path.resolve(p));
   logger.info(`Generating bedrock.yaml file in ${absProjectPath}`);
 
+  const base: IBedrockFile = {
+    rings: defaultRings.reduce<{ [ring: string]: { isDefault: boolean } }>(
+      (defaults, ring) => {
+        defaults[ring] = { isDefault: true };
+        return defaults;
+      },
+      {}
+    ),
+    services: {}
+  };
+
   // Populate bedrock file
-  const bedrockFile: IBedrockFile = absPackagePaths.reduce<IBedrockFile>(
+  const bedrockFile = absPackagePaths.reduce<IBedrockFile>(
     (file, absPackagePath) => {
       const relPathToPackageFromRoot = path.relative(
         absProjectPath,
@@ -221,16 +198,7 @@ const generateBedrockFile = async (
       };
       return file;
     },
-    {
-      rings: defaultRings.reduce<{ [ring: string]: { isDefault: boolean } }>(
-        (defaults, ring) => {
-          defaults[ring] = { isDefault: true };
-          return defaults;
-        },
-        {}
-      ),
-      services: {}
-    }
+    base
   );
 
   // Check if a bedrock.yaml already exists; skip write if present

src/lib/ioUtil.test.ts

@@ -0,0 +1,55 @@
+import fs from "fs";
+import path from "path";
+import { createTempDir, getMissingFilenames } from "./ioUtil";
+
+describe("test createTempDir function", () => {
+  it("create and existence check", () => {
+    const name = createTempDir();
+    expect(fs.existsSync(name)).toBe(true);
+    fs.rmdirSync(name);
+    expect(fs.existsSync(name)).toBe(false);
+  });
+});
+
+describe("test doFilesExist function", () => {
+  it("all files exists", () => {
+    const dir = createTempDir();
+    const files = ["hello", "world"];
+
+    files.forEach(f => {
+      fs.writeFileSync(path.join(dir, `${f}.txt`), f);
+    });
+
+    const missing = getMissingFilenames(
+      dir,
+      files.map(f => `${f}.txt`)
+    );
+    expect(missing.length).toBe(0);
+  });
+  it("none of files exists", () => {
+    const dir = createTempDir();
+    const files = ["hello", "world"];
+
+    const missing = getMissingFilenames(
+      dir,
+      files.map(f => `${f}.txt`)
+    );
+    expect(missing.length).toBe(2);
+  });
+  it("some of files exists", () => {
+    const dir = createTempDir();
+    const files = ["hello", "world", "again"];
+
+    files
+      .filter(f => f !== "again")
+      .forEach(f => {
+        fs.writeFileSync(path.join(dir, `${f}.txt`), f);
+      });
+
+    const missing = getMissingFilenames(
+      dir,
+      files.map(f => `${f}.txt`)
+    );
+    expect(missing.length).toBe(1);
+  });
+});