diff --git a/src/commands/service/create-revision.test.ts b/src/commands/service/create-revision.test.ts index 045d8f932..98da37fec 100644 --- a/src/commands/service/create-revision.test.ts +++ b/src/commands/service/create-revision.test.ts @@ -26,7 +26,7 @@ describe("test makePullRequest function", () => { const createPullRequestFunc = jest.spyOn(azure, "createPullRequest"); // two times because there are two branches: master and stable - createPullRequestFunc.mockReturnValue(Promise.resolve()); + createPullRequestFunc.mockReturnValue(Promise.resolve({})); await makePullRequest(["master", "stable"], { description: "description", diff --git a/src/commands/service/create-revision.ts b/src/commands/service/create-revision.ts index b395d4b7f..c52b9a97b 100644 --- a/src/commands/service/create-revision.ts +++ b/src/commands/service/create-revision.ts @@ -167,10 +167,10 @@ export const makePullRequest = async ( for (const ring of defaultRings) { const title = opts.title || `[SPK] ${opts.sourceBranch} => ${ring}`; await createPullRequest(title, opts.sourceBranch!, ring, { - description: opts.description, - orgName: opts.orgName, - originPushUrl: opts.remoteUrl, - personalAccessToken: opts.personalAccessToken + description: opts.description!, + orgName: opts.orgName!, + originPushUrl: opts.remoteUrl!, + personalAccessToken: opts.personalAccessToken! }); } }; diff --git a/src/lib/git/azure.test.ts b/src/lib/git/azure.test.ts index 04a723925..083a93e3e 100644 --- a/src/lib/git/azure.test.ts +++ b/src/lib/git/azure.test.ts @@ -11,7 +11,13 @@ import { WebApi } from "azure-devops-node-api"; import uuid from "uuid/v4"; import { Config } from "../../config"; import { disableVerboseLogging, enableVerboseLogging } from "../../logger"; -import { createPullRequest, GitAPI } from "./azure"; +import * as gitutils from "../gitutils"; +import { + createPullRequest, + generatePRUrl, + getGitOrigin, + GitAPI +} from "./azure"; //////////////////////////////////////////////////////////////////////////////// // Tests @@ -24,52 +30,52 @@ afterAll(() => { disableVerboseLogging(); }); +describe("test getGitOrigin function", () => { + it("positive test: originPushUrl provided", async () => { + const res = await getGitOrigin("git-url"); + expect(res).toBe("git-url"); + }); + it("positive test: originPushUrl not provided", async () => { + jest + .spyOn(gitutils, "getOriginUrl") + .mockReturnValueOnce(Promise.resolve("origin-url")); + const res = await getGitOrigin(""); + expect(res).toBe("origin-url"); + }); + it("negative test: getOriginUrl throws error", async () => { + jest + .spyOn(gitutils, "getOriginUrl") + .mockReturnValueOnce(Promise.reject(new Error("Fake"))); + await expect(getGitOrigin("")).rejects.toThrow(); + }); +}); + describe("GitAPI", () => { test("should fail when PAT not set", async () => { - (Config as jest.Mock).mockReturnValue({ + (Config as jest.Mock).mockReturnValueOnce({ azure_devops: {} }); - - let invalidPatError: Error | undefined; - try { - await GitAPI(); - } catch (err) { - invalidPatError = err; - } - expect(invalidPatError).toBeDefined(); + await expect(GitAPI()).rejects.toThrow(); }); test("should fail when DevOps org is invalid", async () => { - (Config as jest.Mock).mockReturnValue({ + (Config as jest.Mock).mockReturnValueOnce({ azure_devops: { access_token: uuid() } }); - - let invalidOrgError: Error | undefined; - try { - await GitAPI(); - } catch (err) { - invalidOrgError = err; - } - expect(invalidOrgError).toBeDefined(); + await expect(GitAPI()).rejects.toThrow(); }); test("should pass if org url and PAT set", async () => { - (Config as jest.Mock).mockReturnValue({ + (Config as jest.Mock).mockReturnValueOnce({ azure_devops: { access_token: uuid(), org: uuid() } }); - let error: Error | undefined; - try { - await GitAPI(); - } catch (err) { - error = err; - } - expect(error).toBeUndefined(); + await GitAPI(); }); }); @@ -160,4 +166,47 @@ describe("createPullRequest", () => { expect(true).toBe(false); } }); + it("negative test", async () => { + gitApi.createPullRequest = async () => { + throw new Error("fake"); + }; + + await expect( + createPullRequest("random title", "sourceRef", "targetRef", { + description: uuid(), + originPushUrl: "my-git-url" + }) + ).rejects.toThrow(); + }); + it("negative test: TF401179 error", async () => { + gitApi.createPullRequest = async () => { + throw new Error("TF401179"); + }; + + await expect( + createPullRequest("random title", "sourceRef", "targetRef", { + description: uuid(), + originPushUrl: "my-git-url" + }) + ).rejects.toThrow(); + }); +}); + +describe("test generatePRUrl function", async () => { + test("positive test", async () => { + const res = await generatePRUrl({ + pullRequestId: 1, + repository: { + id: "test" + } + }); + expect(res).toBe("http://foobar.com/pullrequest/1"); + }); + test("negative test", async () => { + await expect( + generatePRUrl({ + repository: {} + }) + ).rejects.toThrow(); + }); }); diff --git a/src/lib/git/azure.ts b/src/lib/git/azure.ts index af4b04e8e..857d80d66 100644 --- a/src/lib/git/azure.ts +++ b/src/lib/git/azure.ts @@ -1,7 +1,8 @@ import * as azdo from "azure-devops-node-api"; import { IGitApi } from "azure-devops-node-api/GitApi"; import AZGitInterfaces, { - GitPullRequestSearchCriteria + GitPullRequestSearchCriteria, + GitRepository } from "azure-devops-node-api/interfaces/GitInterfaces"; import { IAzureDevOpsOpts, PullRequest } from "."; import { Config } from "../../config"; @@ -24,7 +25,7 @@ let gitApi: IGitApi | undefined; // keep track of the gitApi so it can be reused * * @param pr Target PR to generate a link for */ -const generatePRUrl = async ( +export const generatePRUrl = async ( pr: AZGitInterfaces.GitPullRequest ): Promise => { if ( @@ -96,62 +97,45 @@ export const GitAPI = async (opts: IAzureDevOpsOpts = {}): Promise => { }; /** - * Creates a pull request in the DevOps organization associated with the - * personal access token in global config; Refer to GitAPI() on how that is - * saturated. + * Get the git origin from args or fallback to parsing from git client * - * @param title Title of the pull request - * @param sourceRef Source git ref to rebase from - * @param targetRef Target git ref to rebase onto - * @param options Additional options + * @param originPushUrl origin push URL */ -export const createPullRequest: PullRequest = async ( - title, - sourceRef, - targetRef, - options -) => { - const { description = "Automated PR generated by SPK", originPushUrl = "" } = - options || {}; - - // Minimal config to generate a PR - const prConfig: AZGitInterfaces.GitPullRequest = { - description, - sourceRefName: `refs/heads/${sourceRef}`, - targetRefName: `refs/heads/${targetRef}`, - title - }; - - // Get the git origin from args or fallback to parsing from git client - const gitOrigin: string = - originPushUrl.length > 0 - ? originPushUrl - : await getOriginUrl().catch(err => { - logger.error(err); - logger.error( - `Error parsing remote origin from git client, run 'git config --get remote.origin.url' for more information` - ); - return Promise.reject( - Error(`No remote origin found in the current git repository`) - ); - }); +export const getGitOrigin = async (originPushUrl: string): Promise => { + if (originPushUrl) { + return originPushUrl; + } - const gitOriginUrlForLogging = safeGitUrlForLogging(originPushUrl); + try { + return await getOriginUrl(); + } catch (err) { + logger.error(err); + logger.error( + `Error parsing remote origin from git client, run 'git config --get remote.origin.url' for more information` + ); + throw new Error(`No remote origin found in the current git repository`); + } +}; - // fetch all repositories associated with the personal access token - const gitAPI = await GitAPI(options); +/** + * Fetches all repositories associated with the personal access token + * + * @param gitAPI GIT API client + * @param gitOriginUrlForLogging URL for logging + */ +export const getAllRepos = async ( + gitAPI: IGitApi, + gitOriginUrlForLogging: string +): Promise => { logger.info( `Retrieving repositories associated with Azure PAT '${obfuscatedPAT}'` ); - const allRepos = await gitAPI.getRepositories().then(fetchedRepos => { - return fetchedRepos.length === 0 - ? Promise.reject( - Error( - `0 repositories found in Azure DevOps associated with PAT '${obfuscatedPAT}'` - ) - ) - : fetchedRepos; - }); + const allRepos = await gitAPI.getRepositories(); + if (allRepos.length === 0) { + throw new Error( + `0 repositories found in Azure DevOps associated with PAT '${obfuscatedPAT}'` + ); + } // Search for repos matching the current git origin logger.info( @@ -159,6 +143,17 @@ export const createPullRequest: PullRequest = async ( allRepos.length > 1 ? "ies" : "y" } found; searching for entries matching '${gitOriginUrlForLogging}'` ); + return allRepos; +}; + +export const getMatchingBranch = async ( + gitAPI: IGitApi, + sourceRef: string, + targetRef: string, + gitOrigin: string, + allRepos: GitRepository[], + gitOriginUrlForLogging: string +): Promise => { const reposWithMatchingOrigin = allRepos.filter(repo => [repo.url, repo.sshUrl, repo.webUrl, repo.remoteUrl].includes(gitOrigin) ); @@ -182,64 +177,121 @@ export const createPullRequest: PullRequest = async ( }; }) ) - ).filter(repo => { - // Valid repos must contain both the source and target repo - return repo.branches.length >= 2; - }); + ) + .filter(repo => { + // Valid repos must contain both the source and target repo + return repo.branches.length >= 2; + }) + .map(repo => repo.repo); // Only allow one matching repo to be found if (reposWithMatchingBranches.length === 0) { - return Promise.reject( - Error( - `0 repositories found with remote url '${gitOriginUrlForLogging}' and branches '${sourceRef}' and '${targetRef}'; Ensure both '${sourceRef}' and '${targetRef}' exist on '${gitOriginUrlForLogging}'. Cannot automate pull request` - ) + throw new Error( + `0 repositories found with remote url '${gitOriginUrlForLogging}' and branches '${sourceRef}' and '${targetRef}'; Ensure both '${sourceRef}' and '${targetRef}' exist on '${gitOriginUrlForLogging}'. Cannot automate pull request` ); - } else if (reposWithMatchingBranches.length > 1) { - return Promise.reject( - Error( - `Multiple repositories (${reposWithMatchingBranches.length}) found with branches '${sourceRef}' and '${targetRef}'; Cannot automate pull request` - ) + } + if (reposWithMatchingBranches.length > 1) { + throw new Error( + `Multiple repositories (${reposWithMatchingBranches.length}) found with branches '${sourceRef}' and '${targetRef}'; Cannot automate pull request` ); } - // Target repo will be the first in the list - const repoToPRAgainst = reposWithMatchingBranches[0]; - logger.info( - `Creating pull request in repository '${repoToPRAgainst.repo - .name!}' to merge branch '${sourceRef}' into '${targetRef}'` - ); + return reposWithMatchingBranches[0]; +}; - const createdPR = await gitAPI - .createPullRequest(prConfig, repoToPRAgainst.repo.id!) - .catch(async err => { - if (err instanceof Error && err.message.match(/TF401179/)) { - // PR already exists targeting source and target refs - // Search for the existing PR - logger.warn( - `Existing pull requests found in repository '${repoToPRAgainst.repo.name}' matching target refs; Searching for matching pull requests` - ); - const searchCriteria: GitPullRequestSearchCriteria = { - sourceRefName: `refs/heads/${sourceRef}`, - targetRefName: `refs/heads/${targetRef}` - }; - const existingPRs = await gitAPI.getPullRequests( - repoToPRAgainst.repo.id!, - searchCriteria - ); - for (const pr of existingPRs) { - logger.info( - `Existing pull request found targeting source '${sourceRef}' and target '${targetRef}': '${await generatePRUrl( - pr - )}'` - ); - } - } - return Promise.reject(err); - }); +const handleErrorForCreatePullRequest = async ( + gitAPI: IGitApi, + sourceRef: string, + targetRef: string, + repoToPRAgainst: GitRepository, + err: Error +) => { + if (err instanceof Error && err.message.match(/TF401179/)) { + // PR already exists targeting source and target refs + // Search for the existing PR + logger.warn( + `Existing pull requests found in repository '${repoToPRAgainst.name}' matching target refs; Searching for matching pull requests` + ); + const searchCriteria: GitPullRequestSearchCriteria = { + sourceRefName: `refs/heads/${sourceRef}`, + targetRefName: `refs/heads/${targetRef}` + }; + const existingPRs = await gitAPI.getPullRequests( + repoToPRAgainst.id!, + searchCriteria + ); + for (const pr of existingPRs) { + logger.info( + `Existing pull request found targeting source '${sourceRef}' and target '${targetRef}': '${await generatePRUrl( + pr + )}'` + ); + } + } + throw err; +}; + +/** + * Creates a pull request in the DevOps organization associated with the + * personal access token in global config; Refer to GitAPI() on how that is + * saturated. + * + * @param title Title of the pull request + * @param sourceRef Source git ref to rebase from + * @param targetRef Target git ref to rebase onto + * @param options Additional options + */ +export const createPullRequest = async ( + title: string, + sourceRef: string, + targetRef: string, + options: { [key: string]: string } +) => { + const { description = "Automated PR generated by SPK", originPushUrl = "" } = + options || {}; + + // Minimal config to generate a PR + const prConfig: AZGitInterfaces.GitPullRequest = { + description, + sourceRefName: `refs/heads/${sourceRef}`, + targetRefName: `refs/heads/${targetRef}`, + title + }; + + // Get the git origin from args or fallback to parsing from git client + const gitOrigin = await getGitOrigin(originPushUrl); + const gitOriginUrlForLogging = safeGitUrlForLogging(originPushUrl); + const gitAPI = await GitAPI(options); + const allRepos = await getAllRepos(gitAPI, gitOriginUrlForLogging); + const repoToPRAgainst = await getMatchingBranch( + gitAPI, + sourceRef, + targetRef, + gitOrigin, + allRepos, + gitOriginUrlForLogging + ); logger.info( - `Successfully created pull request '${await generatePRUrl(createdPR)}'` + `Creating pull request in repository '${repoToPRAgainst.name!}' to merge branch '${sourceRef}' into '${targetRef}'` ); - return createdPR; + try { + const createdPR = await gitAPI.createPullRequest( + prConfig, + repoToPRAgainst.id! + ); + logger.info( + `Successfully created pull request '${await generatePRUrl(createdPR)}'` + ); + return createdPR; + } catch (err) { + await handleErrorForCreatePullRequest( + gitAPI, + sourceRef, + targetRef, + repoToPRAgainst, + err + ); + } };