From da24e568720d4f4322cfa9bccbd87ab2e00cc5d8 Mon Sep 17 00:00:00 2001 From: Yvonne Radsmikham Date: Thu, 30 Apr 2020 12:58:19 -0700 Subject: [PATCH] Resolve infra unit tests (#31) --- src/commands/infra/generate.test.ts | 151 +++++++++--------- src/commands/infra/generate.ts | 27 +++- .../central/definition.yaml | 4 +- .../definition.yaml | 4 +- .../west/definition.yaml | 6 +- .../mocks/missing-parent-defn/definition.yaml | 4 +- 6 files changed, 108 insertions(+), 88 deletions(-) rename src/commands/infra/mocks/{discovery-service => fabrikam}/central/definition.yaml (92%) rename src/commands/infra/mocks/{discovery-service => fabrikam}/definition.yaml (94%) rename src/commands/infra/mocks/{discovery-service => fabrikam}/west/definition.yaml (90%) diff --git a/src/commands/infra/generate.test.ts b/src/commands/infra/generate.test.ts index 375daafc..aaf9660a 100644 --- a/src/commands/infra/generate.test.ts +++ b/src/commands/infra/generate.test.ts @@ -6,12 +6,11 @@ import { loadConfigurationFromLocalEnv, readYaml } from "../../config"; import { getErrorMessage } from "../../lib/errorBuilder"; import { safeGitUrlForLogging } from "../../lib/gitutils"; import { createTempDir, removeDir } from "../../lib/ioUtil"; -import { disableVerboseLogging, enableVerboseLogging } from "../../logger"; +import { disableVerboseLogging, enableVerboseLogging, logger } from "../../logger"; import { InfraConfigYaml } from "../../types"; import { checkModuleSource, checkRemoteGitExist, - createGenerated, DefinitionYAMLExistence, dirIteration, execute, @@ -28,6 +27,7 @@ import { validateRemoteSource, validateTemplateSources, } from "./generate"; +import * as scaffold from "./scaffold"; import * as generate from "./generate"; import { BACKEND_TFVARS, @@ -36,9 +36,12 @@ import { getSourceFolderNameFromURL, BEDROCK_TFVARS, bedrockTemplatesPath, - VARIABLES_TF, } from "./infra_common"; import * as infraCommon from "./infra_common"; +import { exec } from "../../lib/shell"; + +jest.mock("../../lib/shell"); +jest.mock("simple-git/promise") interface GitTestData { source: string; @@ -76,6 +79,7 @@ afterAll(() => { beforeEach(() => { jest.clearAllMocks(); jest.restoreAllMocks(); + jest.spyOn(scaffold, "copyTfTemplate").mockReturnValue(Promise.resolve()) }); afterEach(() => { @@ -85,7 +89,7 @@ afterEach(() => { "commands", "infra", "mocks", - "discovery-service-generated" + "fabrikam-generated" ) ); }); @@ -98,8 +102,8 @@ afterEach(() => { const getMockedDataForGitTests = async ( positive: boolean ): Promise => { - const mockParentPath = "src/commands/infra/mocks/discovery-service"; - const mockProjectPath = "src/commands/infra/mocks/discovery-service/west"; + const mockParentPath = "src/commands/infra/mocks/fabrikam"; + const mockProjectPath = "src/commands/infra/mocks/fabrikam/west"; const sourceConfiguration = validateDefinition( mockParentPath, mockProjectPath @@ -127,55 +131,54 @@ const getMockedDataForGitTests = async ( }; }; -const testCheckRemoteGitExist = async (positive: boolean): Promise => { - const { safeLoggingUrl, source, sourcePath } = await getMockedDataForGitTests( - positive - ); - if (positive && !fs.existsSync(sourcePath)) { - createGenerated(sourcePath); - } - if (!positive) { - removeDir(sourcePath); - } - await checkRemoteGitExist(sourcePath, source, safeLoggingUrl); -}; - describe("test checkRemoteGitExist function", () => { - it.skip("postive Test", async () => { - await testCheckRemoteGitExist(true); - // no exception thrown + it("positive Test", async () => { + (exec as jest.Mock).mockClear(); + jest.spyOn(generate, "checkSrcPath").mockReturnValueOnce(Promise.resolve()); + const { safeLoggingUrl, source, sourcePath } = await getMockedDataForGitTests( + true + ); + (simpleGit as jest.Mock).mockImplementation(() => { + return {listRemote:async ():Promise=> "https://github.com/microsoft/bedrock"} + }); + let gitError = undefined; + try { + await checkRemoteGitExist(sourcePath, source, safeLoggingUrl); + } catch(e) { + logger.info(e); + gitError=e; + }; + expect(gitError).not.toBeDefined(); }); - // cannot do negative test because it will take too long - // and timeout - it("negative Test", async (done) => { - await expect(testCheckRemoteGitExist(false)).rejects.toThrow(); - done(); + it("negative Test", async () => { + (exec as jest.Mock).mockClear(); + jest.spyOn(generate, "checkSrcPath").mockReturnValueOnce(Promise.resolve()); + const { safeLoggingUrl, source, sourcePath } = await getMockedDataForGitTests( + true + ); + (simpleGit as jest.Mock).mockImplementation(() => { + return {listRemote:async ():Promise=> undefined} + }); + let gitError = undefined; + try { + await checkRemoteGitExist(sourcePath, source, safeLoggingUrl); + } catch(e) { + logger.info(e); + gitError=e; + }; + expect(gitError).toBeDefined(); }); }); -const testGitPull = async (positive: boolean): Promise => { - const { safeLoggingUrl, sourcePath } = await getMockedDataForGitTests( - positive - ); - if (!positive || fs.existsSync(path.join(sourcePath, ".git"))) { - await gitPull(sourcePath, safeLoggingUrl); - } -}; - describe("test gitPull function", () => { - it("postive Test", async () => { - await testGitPull(true); - // no exception thrown - }); - it("negative Test", async () => { - try { - await testGitPull(false); - expect(true).toBe(false); - } catch (e) { - expect(e).toBeDefined(); - } + it("should call exec with the proper git arguments", async () => { + (exec as jest.Mock).mockClear(); + const { safeLoggingUrl, sourcePath } = await getMockedDataForGitTests(true); + await gitPull(sourcePath, safeLoggingUrl); + expect(exec).toHaveBeenCalledTimes(1); + expect(exec).toHaveBeenCalledWith("git", ["symbolic-ref", "HEAD"], { cwd: sourcePath }); }); -}); +}) const testGitCheckout = async (positive: boolean): Promise => { const { sourcePath } = await getMockedDataForGitTests(positive); @@ -224,8 +227,8 @@ describe("Validate remote git source", () => { jest.spyOn(generate, "gitPull").mockResolvedValueOnce(); jest.spyOn(generate, "gitCheckout").mockResolvedValueOnce(); - const mockParentPath = "src/commands/infra/mocks/discovery-service"; - const mockProjectPath = "src/commands/infra/mocks/discovery-service/west"; + const mockParentPath = "src/commands/infra/mocks/fabrikam"; + const mockProjectPath = "src/commands/infra/mocks/fabrikam/west"; const sourceConfiguration = validateDefinition( mockParentPath, mockProjectPath @@ -576,11 +579,11 @@ describe("test dirIteration", () => { }); describe("Validate sources in definition.yaml files", () => { - it.skip("definition.yaml of leaf override parent's variable", async () => { - const mockParentPath = "src/commands/infra/mocks/discovery-service"; - const mockProjectPath = "src/commands/infra/mocks/discovery-service/west"; + it("definition.yaml of leaf override parent's variable", async () => { + const mockParentPath = "src/commands/infra/mocks/fabrikam"; + const mockProjectPath = "src/commands/infra/mocks/fabrikam/west"; const expectedSourceWest = { - source: "https://github.com/contoso/fabrikam", + source: "https://github.com/fabrikam/bedrock", template: "cluster/environments/azure-single-keyvault", version: "v0.0.2", }; @@ -603,12 +606,13 @@ describe("Validate sources in definition.yaml files", () => { sourceData, outputPath ); + expect(scaffold.copyTfTemplate).toHaveBeenCalled(); }); - it.skip("definition.yaml of leaf and parent configuration are the same", async () => { - const mockParentPath = "src/commands/infra/mocks/discovery-service"; + it("definition.yaml of leaf and parent configuration are the same", async () => { + const mockParentPath = "src/commands/infra/mocks/fabrikam"; const mockProjectPath = mockParentPath; const expectedSource = { - source: "https://github.com/contoso/fabrikam", + source: "https://github.com/fabrikam/bedrock", template: "cluster/environments/azure-single-keyvault", version: "v0.0.1", }; @@ -632,15 +636,12 @@ describe("Validate sources in definition.yaml files", () => { outputPath ); [ - "acr.tf", BACKEND_TFVARS, - "main.tf", - "README.md", BEDROCK_TFVARS, - VARIABLES_TF, ].forEach((f) => { fs.unlinkSync(path.join(mockParentPath, f)); }); + expect(scaffold.copyTfTemplate).toHaveBeenCalled(); }); test("without parent's definition.yaml", () => { const mockParentPath = "src/commands/infra/mocks/missing-parent-defn"; @@ -653,11 +654,11 @@ describe("Validate sources in definition.yaml files", () => { } }); - test.skip("without project's definition.yaml", async () => { - const mockParentPath = "src/commands/infra/mocks/discovery-service"; - const mockProjectPath = "src/commands/infra/mocks/discovery-service/east"; + test("without project's definition.yaml", async () => { + const mockParentPath = "src/commands/infra/mocks/fabrikam"; + const mockProjectPath = "src/commands/infra/mocks/fabrikam/east"; const expectedSourceEast = { - source: "https://github.com/contoso/fabrikam", + source: "https://github.com/fabrikam/bedrock", template: "cluster/environments/azure-single-keyvault", version: "v0.0.1", }; @@ -680,13 +681,14 @@ describe("Validate sources in definition.yaml files", () => { sourceData, outputPath ); + expect(scaffold.copyTfTemplate).toHaveBeenCalled(); }); - test.skip("git source, template and version are missing in project path", async () => { - const mockParentPath = "src/commands/infra/mocks/discovery-service"; + test("git source, template and version are missing in project path", async () => { + const mockParentPath = "src/commands/infra/mocks/fabrikam"; const mockProjectPath = - "src/commands/infra/mocks/discovery-service/central"; + "src/commands/infra/mocks/fabrikam/central"; const expectedSourceCentral = { - source: "https://github.com/contoso/fabrikam", + source: "https://github.com/fabrikam/bedrock", template: "cluster/environments/azure-single-keyvault", version: "v0.0.1", }; @@ -709,6 +711,7 @@ describe("Validate sources in definition.yaml files", () => { sourceData, outputPath ); + expect(scaffold.copyTfTemplate).toHaveBeenCalled(); }); test("without parent's and project's definition.yaml", () => { const mockParentPath = "src/commands/infra/mocks"; @@ -723,14 +726,14 @@ describe("Validate sources in definition.yaml files", () => { describe("Validate replacement of variables between parent and leaf definitions", () => { test("Validating that leaf definitions take precedence when generating multi-cluster definitions", () => { - const mockParentPath = "src/commands/infra/mocks/discovery-service"; - const mockProjectPath = "src/commands/infra/mocks/discovery-service/west"; + const mockParentPath = "src/commands/infra/mocks/fabrikam"; + const mockProjectPath = "src/commands/infra/mocks/fabrikam/west"; const finalArray = [ 'acr_enabled = "true"', `address_space = "${DEFAULT_VAR_VALUE}"`, `agent_vm_count = "${DEFAULT_VAR_VALUE}"`, `agent_vm_size = "${DEFAULT_VAR_VALUE}"`, - 'cluster_name = "discovery-service-west"', + 'cluster_name = "fabrikam-west"', `dns_prefix = "${DEFAULT_VAR_VALUE}"`, `flux_recreate = "${DEFAULT_VAR_VALUE}"`, `kubeconfig_recreate = "${DEFAULT_VAR_VALUE}"`, @@ -778,7 +781,7 @@ describe("Validate replacement of variables between parent and leaf definitions" describe("Validate bedrock.tfvars file", () => { test("Validating that a bedrock.tfvars is generated and has appropriate format", () => { - const mockProjectPath = "src/commands/infra/mocks/discovery-service"; + const mockProjectPath = "src/commands/infra/mocks/fabrikam"; const data = readYaml( path.join(mockProjectPath, DEFINITION_YAML) ); @@ -790,7 +793,7 @@ describe("Validate bedrock.tfvars file", () => { describe("Validate backend.tfvars file", () => { test("Validating that a backend.tfvars is generated and has appropriate format", () => { - const mockProjectPath = "src/commands/infra/mocks/discovery-service"; + const mockProjectPath = "src/commands/infra/mocks/fabrikam"; const data = readYaml( path.join(mockProjectPath, DEFINITION_YAML) ); diff --git a/src/commands/infra/generate.ts b/src/commands/infra/generate.ts index 45b1ddf2..4462e92d 100644 --- a/src/commands/infra/generate.ts +++ b/src/commands/infra/generate.ts @@ -23,7 +23,6 @@ import { copyTfTemplate } from "./scaffold"; import { build as buildError, log as logError } from "../../lib/errorBuilder"; import { errorStatusCode } from "../../lib/errorStatusCode"; import { exec } from "../../lib/shell"; -import { TriggerUpdateParameters } from "@azure/arm-containerregistry/esm/models/mappers"; interface CommandOptions { project: string | undefined; @@ -192,10 +191,13 @@ export const gitClone = async ( logger.info(`Cloning source repo to .bedrock/templates was successful.`); }; -export const checkRemoteGitExist = async ( - sourcePath: string, - source: string, - safeLoggingUrl: string +/** + * Checks to see if a directory exists. If not, throw error. + * + * @param sourcePath location to clone repo to + */ +export const checkSrcPath = async ( + sourcePath: string ): Promise => { // Checking for git remote if (!fs.existsSync(sourcePath)) { @@ -204,6 +206,21 @@ export const checkRemoteGitExist = async ( values: [sourcePath], }); } +} + +/** + * Verifies that a remote git repo exists. If not, throw error. + * + * @param sourcePath location to clone repo to + * @param source git url of repo + * @safeLoggingUrl git Repo URL that may contain a PAT or auth token. + */ +export const checkRemoteGitExist = async ( + sourcePath: string, + source: string, + safeLoggingUrl: string +): Promise => { + await checkSrcPath(sourcePath); const result = await simpleGit(sourcePath).listRemote([source]); if (!result) { diff --git a/src/commands/infra/mocks/discovery-service/central/definition.yaml b/src/commands/infra/mocks/fabrikam/central/definition.yaml similarity index 92% rename from src/commands/infra/mocks/discovery-service/central/definition.yaml rename to src/commands/infra/mocks/fabrikam/central/definition.yaml index 1b7e605f..f682cf41 100644 --- a/src/commands/infra/mocks/discovery-service/central/definition.yaml +++ b/src/commands/infra/mocks/fabrikam/central/definition.yaml @@ -1,4 +1,4 @@ -name: discovery-service-central +name: fabrikam-central backend: storage_account_name: storage-account-name access_key: storage-account-access-key @@ -9,7 +9,7 @@ variables: address_space: agent_vm_count: agent_vm_size: - cluster_name: "discovery-service-central" + cluster_name: "fabrikam-central" dns_prefix: flux_recreate: kubeconfig_recreate: diff --git a/src/commands/infra/mocks/discovery-service/definition.yaml b/src/commands/infra/mocks/fabrikam/definition.yaml similarity index 94% rename from src/commands/infra/mocks/discovery-service/definition.yaml rename to src/commands/infra/mocks/fabrikam/definition.yaml index 35c5db62..6d059829 100644 --- a/src/commands/infra/mocks/discovery-service/definition.yaml +++ b/src/commands/infra/mocks/fabrikam/definition.yaml @@ -1,5 +1,5 @@ -name: discovery-service -source: "https://github.com/contoso/fabrikam" +name: fabrikam +source: "https://github.com/fabrikam/bedrock" template: cluster/environments/azure-single-keyvault version: v0.0.1 backend: diff --git a/src/commands/infra/mocks/discovery-service/west/definition.yaml b/src/commands/infra/mocks/fabrikam/west/definition.yaml similarity index 90% rename from src/commands/infra/mocks/discovery-service/west/definition.yaml rename to src/commands/infra/mocks/fabrikam/west/definition.yaml index a5aad468..dc30049f 100644 --- a/src/commands/infra/mocks/discovery-service/west/definition.yaml +++ b/src/commands/infra/mocks/fabrikam/west/definition.yaml @@ -1,5 +1,5 @@ -name: discovery-service-west -source: "https://github.com/contoso/fabrikam" +name: fabrikam-west +source: "https://github.com/fabrikam/bedrock" template: cluster/environments/azure-single-keyvault version: v0.0.2 backend: @@ -12,7 +12,7 @@ variables: address_space: agent_vm_count: agent_vm_size: - cluster_name: "discovery-service-west" + cluster_name: "fabrikam-west" dns_prefix: flux_recreate: kubeconfig_recreate: diff --git a/src/commands/infra/mocks/missing-parent-defn/definition.yaml b/src/commands/infra/mocks/missing-parent-defn/definition.yaml index a5aad468..23c6c666 100644 --- a/src/commands/infra/mocks/missing-parent-defn/definition.yaml +++ b/src/commands/infra/mocks/missing-parent-defn/definition.yaml @@ -1,5 +1,5 @@ -name: discovery-service-west -source: "https://github.com/contoso/fabrikam" +name: fabrikam-west +source: "https://github.com/fabrikam/bedrock" template: cluster/environments/azure-single-keyvault version: v0.0.2 backend: