[FEATURE] add exception chain to hld add variable group command (#470)

Author: dennisseah

src/commands/hld/append-variable-group.ts

@@ -3,6 +3,9 @@ import { build as buildCmd, exit as exitCmd } from "../../lib/commandBuilder";
 import { RENDER_HLD_PIPELINE_FILENAME } from "../../lib/constants";
 import { appendVariableGroupToPipelineYaml } from "../../lib/fileutils";
 import { logger } from "../../logger";
+import { build as buildError, log as logError } from "../../lib/errorBuilder";
+import { errorStatusCode } from "../../lib/errorStatusCode";
+import { hasValue } from "../../lib/validator";
 import decorator from "./append-variable-group.decorator.json";
 
 /**
@@ -18,22 +21,28 @@ export const execute = async (
   variableGroupName: string,
   exitFn: (status: number) => Promise<void>
 ): Promise<void> => {
-  if (!variableGroupName) {
-    logger.error("Variable group name is missing.");
-    await exitFn(1);
-    return;
-  }
-
   try {
+    if (!hasValue(variableGroupName)) {
+      throw buildError(
+        errorStatusCode.VALIDATION_ERR,
+        "hld-append-var-group-name-missing"
+      );
+    }
+
     appendVariableGroupToPipelineYaml(
       hldRepoPath,
       RENDER_HLD_PIPELINE_FILENAME,
       variableGroupName
     );
     await exitFn(0);
   } catch (err) {
-    logger.error(`Error occurred while appending variable group.`);
-    logger.error(err);
+    logError(
+      buildError(
+        errorStatusCode.CMD_EXE_ERR,
+        "hld-append-var-group-cmd-failed",
+        err
+      )
+    );
     await exitFn(1);
   }
 };

src/lib/errorStatusCode.ts

@@ -6,5 +6,6 @@ export enum errorStatusCode {
   VALIDATION_ERR = 1001,
   EXE_FLOW_ERR = 1002,
   ENV_SETTING_ERR = 1010,
+  FILE_IO_ERR = 1011,
   GIT_OPS_ERR = 1100,
 }

src/lib/fileutils.test.ts

@@ -1,4 +1,3 @@
-/* eslint-disable @typescript-eslint/no-non-null-assertion */
 ////////////////////////////////////////////////////////////////////////////////
 // !!NOTE!!
 // This test suite uses mock-fs to mock out the the file system
@@ -34,6 +33,7 @@ import {
   createTestServiceBuildAndUpdatePipelineYaml,
 } from "../test/mockFactory";
 import { AccessYaml, AzurePipelinesYaml, MaintainersFile } from "../types";
+import { errorStatusCode } from "./errorStatusCode";
 import {
   addNewServiceToMaintainersFile,
   appendVariableGroupToPipelineYaml,
@@ -461,6 +461,17 @@ describe("appendVariableGroupToHldAzurePipelinesYaml", () => {
       createTestHldAzurePipelinesYamlWithVariableGroup()
     );
   });
+  it("negative test", () => {
+    try {
+      appendVariableGroupToPipelineYaml(
+        uuid(),
+        RENDER_HLD_PIPELINE_FILENAME,
+        "my-vg"
+      );
+    } catch (err) {
+      expect(err.errorCode).toBe(errorStatusCode.FILE_IO_ERR);
+    }
+  });
 });
 
 describe("generateDefaultHldComponentYaml", () => {
@@ -667,26 +678,48 @@ describe("serviceBuildUpdatePipeline", () => {
     expect(deserializedYaml).toStrictEqual(buildPipelineYaml);
 
     // variables should include all groups
-    for (const group of variableGroups) {
-      expect(buildPipelineYaml.variables!.includes({ group }));
+    expect(buildPipelineYaml.variables).toBeDefined();
+    if (buildPipelineYaml.variables) {
+      expect(buildPipelineYaml.variables.length).toBe(variableGroups.length);
+
+      for (const group of variableGroups) {
+        expect(buildPipelineYaml.variables.includes({ group }));
+      }
     }
-    expect(buildPipelineYaml.variables!.length).toBe(variableGroups.length);
 
     // trigger branches should include all ring branches
-    for (const branch of ringBranches) {
-      expect(buildPipelineYaml.trigger!.branches!.include!.includes(branch));
+    expect(buildPipelineYaml.trigger).toBeDefined();
+    if (buildPipelineYaml.trigger) {
+      expect(buildPipelineYaml.trigger.branches).toBeDefined();
+
+      if (buildPipelineYaml.trigger.branches) {
+        expect(buildPipelineYaml.trigger.branches.include).toBeDefined();
+
+        if (buildPipelineYaml.trigger.branches.include) {
+          expect(buildPipelineYaml.trigger.branches.include.length).toBe(
+            ringBranches.length
+          );
+          for (const branch of ringBranches) {
+            expect(buildPipelineYaml.trigger.branches.include.includes(branch));
+          }
+        }
+      }
     }
-    expect(buildPipelineYaml.trigger!.branches!.include!.length).toBe(
-      ringBranches.length
-    );
 
     // verify components of stages
-    expect(buildPipelineYaml.stages && buildPipelineYaml.stages.length === 2);
-    for (const stage of buildPipelineYaml.stages!) {
-      for (const job of stage.jobs) {
-        // pool.vmImage should be 'gentoo'
-        expect(job.pool!.vmImage).toBe(VM_IMAGE);
-        expect(job.steps);
+    expect(buildPipelineYaml.stages).toBeDefined();
+    if (buildPipelineYaml.stages) {
+      expect(buildPipelineYaml.stages && buildPipelineYaml.stages.length === 2);
+      for (const stage of buildPipelineYaml.stages) {
+        for (const job of stage.jobs) {
+          expect(job.pool).toBeDefined();
+
+          if (job.pool) {
+            // pool.vmImage should be 'gentoo'
+            expect(job.pool.vmImage).toBe(VM_IMAGE);
+            expect(job.steps);
+          }
+        }
       }
     }
   });
@@ -723,28 +756,44 @@ describe("serviceBuildUpdatePipeline", () => {
           "utf8"
         )
       );
-      const hasCorrectIncludes = azureYaml.trigger!.paths!.include!.includes(
-        path.relative(randomDirPath, serviceReference.servicePath)
-      );
-      expect(hasCorrectIncludes).toBe(true);
 
-      let hasCorrectVariableGroup1 = false;
-      let hasCorrectVariableGroup2 = false;
-      for (const value of Object.values(azureYaml.variables!)) {
-        const item = value as { group: string };
+      expect(azureYaml.trigger).toBeDefined();
 
-        if (item.group === variableGroups[0]) {
-          hasCorrectVariableGroup1 = true;
-        }
+      if (azureYaml.trigger) {
+        expect(azureYaml.trigger.paths).toBeDefined();
+
+        if (azureYaml.trigger.paths) {
+          expect(azureYaml.trigger.paths.include).toBeDefined();
 
-        if (item.group === variableGroups[1]) {
-          hasCorrectVariableGroup2 = true;
+          if (azureYaml.trigger.paths.include) {
+            const hasCorrectIncludes = azureYaml.trigger.paths.include.includes(
+              path.relative(randomDirPath, serviceReference.servicePath)
+            );
+            expect(hasCorrectIncludes).toBe(true);
+          }
         }
       }
 
-      expect(hasCorrectIncludes).toBe(true);
-      expect(hasCorrectVariableGroup1).toBe(true);
-      expect(hasCorrectVariableGroup2).toBe(true);
+      expect(azureYaml.variables).toBeDefined();
+
+      if (azureYaml.variables) {
+        let hasCorrectVariableGroup1 = false;
+        let hasCorrectVariableGroup2 = false;
+        for (const value of Object.values(azureYaml.variables)) {
+          const item = value as { group: string };
+
+          if (item.group === variableGroups[0]) {
+            hasCorrectVariableGroup1 = true;
+          }
+
+          if (item.group === variableGroups[1]) {
+            hasCorrectVariableGroup2 = true;
+          }
+        }
+
+        expect(hasCorrectVariableGroup1).toBe(true);
+        expect(hasCorrectVariableGroup2).toBe(true);
+      }
     }
   });
 });

src/lib/fileutils.ts

@@ -20,6 +20,8 @@ import {
   User,
 } from "../types";
 import { readYaml, write } from "../config";
+import { build as buildError } from "../lib/errorBuilder";
+import { errorStatusCode } from "../lib/errorStatusCode";
 
 /**
  * Read given pipeline file as json object.
@@ -482,16 +484,23 @@ export const appendVariableGroupToPipelineYaml = (
   fileName: string,
   variableGroupName: string
 ): void => {
-  const pipelineFile = readYaml(path.join(dir, fileName)) as AzurePipelinesYaml;
-
-  if (!pipelineFile.variables) {
-    pipelineFile.variables = [];
+  try {
+    const pipelineFile = readYaml(
+      path.join(dir, fileName)
+    ) as AzurePipelinesYaml;
+    pipelineFile.variables = pipelineFile.variables || [];
+
+    pipelineFile.variables.push({ group: variableGroupName });
+
+    logger.info(`Updating '${dir}/${fileName}'.`);
+    write(pipelineFile, dir, fileName);
+  } catch (err) {
+    throw buildError(
+      errorStatusCode.FILE_IO_ERR,
+      "fileutils-append-variable-group-to-pipeline-yaml",
+      err
+    );
   }
-
-  pipelineFile.variables.push({ group: variableGroupName });
-
-  logger.info(`Updating '${dir}/${fileName}'.`);
-  write(pipelineFile, dir, fileName);
 };
 
 /**

src/lib/i18n.json

@@ -25,6 +25,11 @@
     "infra-err-tf-path-not-found": "Provided Terraform {0} path is invalid or cannot be found: {1}",
     "infra-err-create-scaffold": "Could not create scaffold",
     "infra-err-git-clone-failed": "Could not clone the source remote repository. The remote repo might not exist or you did not have the rights to access it",
-    "infra-git-source-no-exist": "Source path, {0} did not exist."
+    "infra-git-source-no-exist": "Source path, {0} did not exist.",
+
+    "hld-append-var-group-cmd-failed": "HLD Append Variable Group Command was not successfully executed.",
+    "hld-append-var-group-name-missing": "Variable group name was not provided. Provide variable group.",
+
+    "fileutils-append-variable-group-to-pipeline-yaml": "Could not append variable group name to manifest-generation.yaml file in HLD repo. Check this is file exist and if it is YAML format."
   }
 }