Removed duplicate call to check for bedrock file that enforces project commands order (#235)

* Closes #927

* Fixed test case

* Incrporated feedback. Moved test cases to apporpriate file.

* Incrporated feedback

* Fixed test case

Author: sarath-p

src/commands/project/create-variable-group.ts

@@ -2,26 +2,22 @@ import { VariableGroup } from "azure-devops-node-api/interfaces/ReleaseInterface
 import commander from "commander";
 import path from "path";
 import { echo } from "shelljs";
-import {
-  Bedrock,
-  bedrockFileInfo,
-  Config,
-  readYaml,
-  write
-} from "../../config";
-import { projectInitDependencyErrorMessage } from "../../constants";
+import { Bedrock, Config, readYaml, write } from "../../config";
+import { fileInfo as bedrockFileInfo } from "../../lib/bedrockYaml";
 import {
   build as buildCmd,
   exit as exitCmd,
   validateForRequiredValues
 } from "../../lib/commandBuilder";
+import { PROJECT_INIT_DEPENDENCY_ERROR_MESSAGE } from "../../lib/constants";
 import { IAzureDevOpsOpts } from "../../lib/git";
 import { addVariableGroup } from "../../lib/pipelines/variableGroup";
 import { hasValue } from "../../lib/validator";
 import { logger } from "../../logger";
 import {
   IAzurePipelinesYaml,
   IBedrockFile,
+  IBedrockFileInfo,
   IVariableGroupData,
   IVariableGroupDataVariable
 } from "../../types";
@@ -39,6 +35,13 @@ interface ICommandOptions {
   project: string | undefined;
 }
 
+export const checkDependencies = (projectPath: string) => {
+  const fileInfo: IBedrockFileInfo = bedrockFileInfo(projectPath);
+  if (fileInfo.exist === false) {
+    throw new Error(PROJECT_INIT_DEPENDENCY_ERROR_MESSAGE);
+  }
+};
+
 /**
  * Executes the command.
  *
@@ -59,12 +62,7 @@ export const execute = async (
     const projectPath = process.cwd();
     logger.verbose(`project path: ${projectPath}`);
 
-    const fileInfo = await bedrockFileInfo(projectPath);
-    if (fileInfo.exist === false) {
-      logger.error(projectInitDependencyErrorMessage);
-      await exitFn(1);
-      return;
-    }
+    checkDependencies(projectPath);
 
     const { azure_devops } = Config();
 

src/commands/project/pipeline.test.ts

@@ -1,6 +1,10 @@
 import { create as createBedrockYaml } from "../../lib/bedrockYaml";
 import { createTempDir } from "../../lib/ioUtil";
-import { disableVerboseLogging, enableVerboseLogging } from "../../logger";
+import {
+  disableVerboseLogging,
+  enableVerboseLogging,
+  logger
+} from "../../logger";
 jest.mock("../../lib/pipelines/pipelines");
 
 import {
@@ -10,11 +14,11 @@ import {
 } from "../../lib/pipelines/pipelines";
 
 import {
+  checkDependencies,
   execute,
   fetchValidateValues,
   ICommandOptions,
-  installLifecyclePipeline,
-  validate
+  installLifecyclePipeline
 } from "./pipeline";
 
 beforeAll(() => {
@@ -48,10 +52,10 @@ const mockMissingValues: ICommandOptions = {
 const gitUrl = "https://github.com/CatalystCode/spk.git";
 
 describe("test valid function", () => {
-  it("negative test", () => {
+  it("negative test", async () => {
     try {
       const tmpDir = createBedrockYaml();
-      validate(tmpDir);
+      await checkDependencies(tmpDir);
       expect(true).toBe(false);
     } catch (e) {
       expect(e).not.toBeNull();

src/commands/project/pipeline.ts

@@ -4,17 +4,17 @@ import {
   BuildDefinitionVariable
 } from "azure-devops-node-api/interfaces/BuildInterfaces";
 import commander from "commander";
-import { bedrockFileInfo, Config } from "../../config";
-import {
-  projectCvgDependencyErrorMessage,
-  projectInitCvgDependencyErrorMessage
-} from "../../constants";
-import { isExists as isBedrockFileExists } from "../../lib/bedrockYaml";
+import { Config } from "../../config";
+import { fileInfo as bedrockFileInfo } from "../../lib/bedrockYaml";
 import {
   build as buildCmd,
   exit as exitCmd,
   validateForRequiredValues
 } from "../../lib/commandBuilder";
+import {
+  PROJECT_CVG_DEPENDENCY_ERROR_MESSAGE,
+  PROJECT_INIT_CVG_DEPENDENCY_ERROR_MESSAGE
+} from "../../lib/constants";
 import { BUILD_SCRIPT_URL } from "../../lib/constants";
 import {
   getOriginUrl,
@@ -41,12 +41,12 @@ export interface ICommandOptions {
   buildScriptUrl: string | undefined;
 }
 
-export const validate = async (projectPath: string) => {
-  const exist = isBedrockFileExists(projectPath);
-  if (exist === false) {
-    throw new Error(
-      "Please run `spk project init` command before running this command to initialize the project."
-    );
+export const checkDependencies = (projectPath: string) => {
+  const file: IBedrockFileInfo = bedrockFileInfo(projectPath);
+  if (file.exist === false) {
+    throw new Error(PROJECT_INIT_CVG_DEPENDENCY_ERROR_MESSAGE);
+  } else if (file.hasVariableGroups === false) {
+    throw new Error(PROJECT_CVG_DEPENDENCY_ERROR_MESSAGE);
   }
 };
 
@@ -113,22 +113,10 @@ export const execute = async (
     return;
   }
 
-  const fileInfo: IBedrockFileInfo = await bedrockFileInfo(projectPath);
-  if (fileInfo.exist === false) {
-    logger.error(projectInitCvgDependencyErrorMessage);
-    await exitFn(1);
-    return;
-  }
-  if (fileInfo.hasVariableGroups === false) {
-    logger.error(projectCvgDependencyErrorMessage);
-    await exitFn(1);
-    return;
-  }
-
   logger.verbose(`project path: ${projectPath}`);
 
   try {
-    await validate(projectPath);
+    checkDependencies(projectPath);
     const gitOriginUrl = await getOriginUrl();
     const values = fetchValidateValues(opts, gitOriginUrl, Config());
 

src/commands/service/create.test.ts

@@ -4,6 +4,7 @@ import { promisify } from "util";
 import uuid from "uuid/v4";
 import { Bedrock } from "../../config";
 import * as config from "../../config";
+import * as bedrockYaml from "../../lib/bedrockYaml";
 import { DEFAULT_CONTENT as BedrockMockedContent } from "../../lib/bedrockYaml";
 import { checkoutCommitPushCreatePRLink } from "../../lib/gitutils";
 import { createTempDir, removeDir } from "../../lib/ioUtil";
@@ -115,12 +116,10 @@ describe("Test execute function", () => {
   it("Negative test: missing bedrock file", async () => {
     const testServiceName = uuid();
     const exitFn = jest.fn();
-    jest.spyOn(config, "bedrockFileInfo").mockImplementation(() =>
-      Promise.resolve({
-        exist: false,
-        hasVariableGroups: false
-      })
-    );
+    jest.spyOn(bedrockYaml, "fileInfo").mockImplementation(() => ({
+      exist: false,
+      hasVariableGroups: false
+    }));
     try {
       await execute(testServiceName, getMockValues(), exitFn);
       expect(exitFn).toBeCalledTimes(1);
@@ -132,26 +131,10 @@ describe("Test execute function", () => {
   it("Negative test: missing bedrock variable groups", async () => {
     const testServiceName = uuid();
     const exitFn = jest.fn();
-    jest.spyOn(config, "bedrockFileInfo").mockImplementation(() =>
-      Promise.resolve({
-        exist: true,
-        hasVariableGroups: false
-      })
-    );
-    try {
-      await execute(testServiceName, getMockValues(), exitFn);
-      expect(exitFn).toBeCalledTimes(1);
-      expect(exitFn.mock.calls).toEqual([[1]]);
-    } finally {
-      removeDir(testServiceName); // housekeeping
-    }
-  });
-  it("Negative test: simulated exception thrown", async () => {
-    const testServiceName = uuid();
-    const exitFn = jest.fn();
-    jest
-      .spyOn(config, "bedrockFileInfo")
-      .mockImplementation(() => Promise.reject());
+    jest.spyOn(bedrockYaml, "fileInfo").mockImplementation(() => ({
+      exist: true,
+      hasVariableGroups: false
+    }));
     try {
       await execute(testServiceName, getMockValues(), exitFn);
       expect(exitFn).toBeCalledTimes(1);

src/commands/service/create.ts

@@ -1,16 +1,17 @@
 import commander from "commander";
 import path from "path";
 import shelljs from "shelljs";
-import { Bedrock, bedrockFileInfo } from "../../config";
-import {
-  projectCvgDependencyErrorMessage,
-  projectInitCvgDependencyErrorMessage
-} from "../../constants";
+import { Bedrock } from "../../config";
 import {
   addNewService as addNewServiceToBedrockFile,
+  fileInfo as bedrockFileInfo,
   YAML_NAME as BedrockFileName
 } from "../../lib/bedrockYaml";
 import { build as buildCmd, exit as exitCmd } from "../../lib/commandBuilder";
+import {
+  PROJECT_CVG_DEPENDENCY_ERROR_MESSAGE,
+  PROJECT_INIT_CVG_DEPENDENCY_ERROR_MESSAGE
+} from "../../lib/constants";
 import {
   addNewServiceToMaintainersFile,
   generateDockerfile,
@@ -88,6 +89,15 @@ export const fetchValues = (opts: ICommandOptions) => {
   return values;
 };
 
+export const checkDependencies = (projectPath: string) => {
+  const fileInfo: IBedrockFileInfo = bedrockFileInfo(projectPath);
+  if (fileInfo.exist === false) {
+    throw new Error(PROJECT_INIT_CVG_DEPENDENCY_ERROR_MESSAGE);
+  } else if (fileInfo.hasVariableGroups === false) {
+    throw new Error(PROJECT_CVG_DEPENDENCY_ERROR_MESSAGE);
+  }
+};
+
 export const execute = async (
   serviceName: string,
   opts: ICommandOptions,
@@ -103,19 +113,7 @@ export const execute = async (
   logger.verbose(`project path: ${projectPath}`);
 
   try {
-    const fileInfo: IBedrockFileInfo = await bedrockFileInfo(projectPath);
-    if (fileInfo.exist === false) {
-      logger.error(projectInitCvgDependencyErrorMessage());
-      await exitFn(1);
-      return;
-    }
-
-    if (fileInfo.hasVariableGroups === false) {
-      logger.error(projectCvgDependencyErrorMessage());
-      await exitFn(1);
-      return;
-    }
-
+    checkDependencies(projectPath);
     const values = fetchValues(opts);
     await createService(projectPath, serviceName, values);
     await exitFn(0);

src/config.test.ts

@@ -1,10 +1,8 @@
-import fs from "fs";
-import yaml from "js-yaml";
 import os from "os";
 import path from "path";
 import shell from "shelljs";
 import uuid from "uuid/v4";
-import { Bedrock, bedrockFileInfo, write } from "./config";
+import { Bedrock, write } from "./config";
 import { disableVerboseLogging, enableVerboseLogging, logger } from "./logger";
 import { IBedrockFile, IBedrockFileInfo } from "./types";
 
@@ -103,85 +101,3 @@ describe("Bedrock", () => {
     expect(error).toBeDefined();
   });
 });
-
-describe("isBedrockFileValid", () => {
-  test("Should fail when empty file directory is passed", async () => {
-    let invalidDirError: Error | undefined;
-
-    try {
-      logger.info("calling create");
-      await bedrockFileInfo("");
-    } catch (err) {
-      invalidDirError = err;
-    }
-    expect(invalidDirError).toBeDefined();
-  });
-
-  test("Should return false when bedrock file does not exist", async () => {
-    // Create random directory to initialize
-    const randomTmpDir = path.join(os.tmpdir(), uuid());
-    fs.mkdirSync(randomTmpDir);
-
-    const fileInfo: IBedrockFileInfo = await bedrockFileInfo(randomTmpDir);
-
-    logger.info(`bedrock.yaml file exists: ${fileInfo.exist}`);
-
-    expect(fileInfo.exist).toBe(false);
-  });
-
-  test("Should pass when bedrock file exists with variable groups length 0", async () => {
-    // Create random directory to initialize
-    const randomTmpDir = path.join(os.tmpdir(), uuid());
-    fs.mkdirSync(randomTmpDir);
-
-    logger.info(`random temp dir: ${randomTmpDir}`);
-
-    // create bedrock file to simulate the the use case that `spk project init` ran before
-    const bedrockFileData: IBedrockFile = {
-      rings: {},
-      services: {},
-      variableGroups: []
-    };
-
-    const asYaml = yaml.safeDump(bedrockFileData, {
-      lineWidth: Number.MAX_SAFE_INTEGER
-    });
-    fs.writeFileSync(path.join(randomTmpDir, "bedrock.yaml"), asYaml);
-
-    const fileInfo: IBedrockFileInfo = await bedrockFileInfo(randomTmpDir);
-    logger.verbose(
-      `bedrock.yaml file exists: ${fileInfo.exist} in ${randomTmpDir}`
-    );
-
-    expect(fileInfo.exist).toBe(true);
-    expect(fileInfo.hasVariableGroups).toBe(false);
-  });
-
-  test("Should pass when bedrock file exists with one variable group", async () => {
-    // Create random directory to initialize
-    const randomTmpDir = path.join(os.tmpdir(), uuid());
-    fs.mkdirSync(randomTmpDir);
-
-    logger.info(`random temp dir: ${randomTmpDir}`);
-
-    // create bedrock file to simulate the the use case that `spk project init` ran before
-    const bedrockFileData: IBedrockFile = {
-      rings: {},
-      services: {},
-      variableGroups: [variableGroupName]
-    };
-
-    const asYaml = yaml.safeDump(bedrockFileData, {
-      lineWidth: Number.MAX_SAFE_INTEGER
-    });
-    fs.writeFileSync(path.join(randomTmpDir, "bedrock.yaml"), asYaml);
-
-    const fileInfo: IBedrockFileInfo = await bedrockFileInfo(randomTmpDir);
-    logger.info(
-      `bedrock.yaml file exists: ${fileInfo.exist} in ${randomTmpDir}`
-    );
-
-    expect(fileInfo.exist).toBe(true);
-    expect(fileInfo.hasVariableGroups).toBe(true);
-  });
-});