Test merge branch command (#16299)

This PR addresses the bugs in the merge command. The following issues
have been resolved:

1. Pushing the merge commit to the automation pull request.

3. Corrected the retrieval of the assignee name for the automation PR.
Previously, the assignee name returned was undefined.
4. Implemented the ability to add custom reviewers or a team of
reviewers to the automation PRs.


[AB#4873](https://dev.azure.com/fluidframework/internal/_sprints/taskboard/Team%20Sasha/internal/Team%20Sasha%20-%202023.06-B?workitem=4873)

PR against my fork main:
sonalideshpandemsft#13

.github/workflows/sync-branch.yml

@@ -35,4 +35,4 @@ jobs:
           echo "$(pwd)" >> $GITHUB_PATH
       
       - name: Run merge command
-        run: flub merge branches -a ${{ secrets.BOT_MAIN_NEXT_WORKFLOW_PAT }} -s $SOURCE_BRANCH -t $TARGET_BRANCH -b 10
+        run: flub merge branches -a ${{ secrets.GITHUB_TOKEN }} -p ${{ secrets.BOT_MAIN_NEXT_WORKFLOW_PAT }} -s $SOURCE_BRANCH -t $TARGET_BRANCH -b 10 --reviewers sonalideshpandemsft

build-tools/packages/build-cli/docs/merge.md

@@ -12,15 +12,18 @@ Sync branches depending on the batch size passed
 
 ```
 USAGE
-  $ flub merge branches -a <value> -s <value> -t <value> -b <value> [-v | --quiet] [-r <value>]
+  $ flub merge branches -a <value> -p <value> -s <value> -t <value> -b <value> --reviewers <value> [-v | --quiet]
+    [-r <value>]
 
 FLAGS
   -a, --auth=<value>       (required) GitHub authentication token. For security reasons, this value should be passed
                            using the GITHUB_TOKEN environment variable.
   -b, --batchSize=<value>  (required) Maximum number of commits to include in the pull request
+  -p, --pat=<value>        (required) GitHub Personal Access Token
   -r, --remote=<value>     [default: origin]
   -s, --source=<value>     (required) Source branch name
   -t, --target=<value>     (required) Target branch name
+  --reviewers=<value>...   (required) Add reviewers to PR
 
 LOGGING FLAGS
   -v, --verbose  Enable verbose logging.

build-tools/packages/build-cli/src/commands/merge/branches.ts

@@ -7,13 +7,7 @@ import { Flags } from "@oclif/core";
 import chalk from "chalk";
 
 import { BaseCommand } from "../../base";
-import {
-	Repository,
-	createPullRequest,
-	getUserAccess,
-	pullRequestExists,
-	pullRequestInfo,
-} from "../../lib";
+import { Repository, createPullRequest, pullRequestExists, pullRequestInfo } from "../../lib";
 
 /**
  * This command class is used to merge two branches based on the batch size provided.
@@ -31,6 +25,11 @@ export default class MergeBranch extends BaseCommand<typeof MergeBranch> {
 			required: true,
 			env: "GITHUB_TOKEN",
 		}),
+		pat: Flags.string({
+			description: "GitHub Personal Access Token",
+			char: "p",
+			required: true,
+		}),
 		source: Flags.string({
 			description: "Source branch name",
 			char: "s",
@@ -51,6 +50,11 @@ export default class MergeBranch extends BaseCommand<typeof MergeBranch> {
 			char: "r",
 			default: "origin",
 		}),
+		reviewers: Flags.string({
+			description: "Add reviewers to PR",
+			required: true,
+			multiple: true,
+		}),
 		...BaseCommand.flags,
 	};
 
@@ -87,6 +91,7 @@ export default class MergeBranch extends BaseCommand<typeof MergeBranch> {
 		this.initialBranch = (await this.gitRepo.gitClient.status()).current ?? "main";
 
 		this.remote = flags.remote;
+
 		const prExists: boolean = await pullRequestExists(
 			flags.auth,
 			prTitle,
@@ -112,6 +117,8 @@ export default class MergeBranch extends BaseCommand<typeof MergeBranch> {
 			`refs/remotes/${this.remote}/${flags.source}`,
 		);
 
+		this.log(`Unmerged commit list: ${unmergedCommitList}`);
+
 		if (unmergedCommitList.length === 0) {
 			this.log(
 				chalk.green(
@@ -132,7 +139,6 @@ export default class MergeBranch extends BaseCommand<typeof MergeBranch> {
 
 		await this.gitRepo.gitClient
 			.checkoutBranch(tempBranchToCheckConflicts, flags.target)
-			.push(this.remote, tempBranchToCheckConflicts)
 			.branch(["--set-upstream-to", `${this.remote}/${tempBranchToCheckConflicts}`]);
 
 		const [commitListHasConflicts, conflictingCommitIndex] = await hasConflicts(
@@ -212,7 +218,9 @@ export default class MergeBranch extends BaseCommand<typeof MergeBranch> {
 		git push`;
 
 		if (prWillConflict === false) {
-			await this.gitRepo.gitClient.merge([flags.target, "-m", prTitle]);
+			await this.gitRepo.gitClient
+				.merge([flags.target, "-m", prTitle])
+				.push(this.remote, branchName);
 
 			/**
 			 * The below description is intended for PRs which may have CI failures with next.
@@ -236,30 +244,31 @@ export default class MergeBranch extends BaseCommand<typeof MergeBranch> {
 		/**
 		 * fetch name of owner associated to the pull request
 		 */
-		const pr = await pullRequestInfo(flags.auth, owner, repo, prHeadCommit, this.logger);
-		console.debug(pr);
-		const author = pr.data?.[0]?.assignee?.login;
+		const pr = await pullRequestInfo(flags.pat, owner, repo, prHeadCommit, this.logger);
+		if (pr === undefined) {
+			this.warning(`Unable to add assignee`);
+		}
+		const username = pr.data.author.login;
 		this.info(
-			`Fetching pull request info for commit id ${prHeadCommit} and assignee ${author}`,
+			`Fetching pull request info for commit id ${prHeadCommit} and assignee ${username}`,
 		);
-		const user = await getUserAccess(flags.auth, owner, repo, this.logger);
-		this.verbose(`List users with push access to main branch: ${JSON.stringify(user.data)}`);
 
 		const prObject = {
-			token: flags.auth,
+			token: flags.pat,
 			owner,
 			repo,
 			source: branchName,
 			target: flags.target,
-			assignee: author,
+			assignee: username,
 			title: prTitle,
 			description,
+			reviewers: flags.reviewers,
 		};
 
+		this.log(`Initiate PR creation: ${prObject}}`);
+
 		const prNumber = await createPullRequest(prObject, this.logger);
-		this.log(
-			`Opened pull request ${prNumber} for commit id ${prHeadCommit}. Please resolve the merge conflicts.`,
-		);
+		this.log(`Opened pull request ${prNumber} for commit id ${prHeadCommit}`);
 	}
 
 	/**

build-tools/packages/build-cli/src/lib/git.ts

@@ -187,16 +187,24 @@ export class Repository {
 	 * @returns An array of all commits between the base and head commits.
 	 */
 	public async revList(baseCommit: string, headCommit: string = "HEAD"): Promise<string[]> {
-		const result = await this.git.raw("rev-list", `${baseCommit}..${headCommit}`);
+		const result = await this.git.raw("rev-list", `${baseCommit}..${headCommit}`, "--reverse");
 		return result
 			.split(/\r?\n/)
 			.filter((value) => value !== null && value !== undefined && value !== "");
 	}
 
 	public async canMergeWithoutConflicts(commit: string): Promise<boolean> {
-		const mergeResult = await this.git.merge([commit, "--no-commit"]);
-		await this.git.merge(["--abort"]);
-		const canMerge = mergeResult.result === "success";
-		return canMerge;
+		let mergeResult;
+		try {
+			console.log(`Checking merge conflicts for: ${commit}`);
+			mergeResult = await this.git.merge([commit, "--no-commit", "--no-ff"]);
+			console.log(`Git merge result: ${mergeResult}`);
+			await this.git.merge(["--abort"]);
+		} catch {
+			await this.git.merge(["--abort"]);
+			return false;
+		}
+
+		return mergeResult.result === "success";
 	}
 }

build-tools/packages/build-cli/src/lib/github.ts

@@ -6,12 +6,11 @@ import { Octokit } from "@octokit/core";
 import { CommandLogger } from "../logging";
 
 const PULL_REQUEST_EXISTS = "GET /repos/{owner}/{repo}/pulls";
-const PULL_REQUEST_INFO = "GET /repos/{owner}/{repo}/commits/{commit_sha}/pulls";
+const PULL_REQUEST_INFO = "GET /repos/{owner}/{repo}/commits/{ref}";
 const PULL_REQUEST = "POST /repos/{owner}/{repo}/pulls";
 const ASSIGNEE = "POST /repos/{owner}/{repo}/issues/{issue_number}/assignees";
-const REVIEWER = "POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers";
 const LABEL = "POST /repos/{owner}/{repo}/issues/{issue_number}/labels";
-const GET_USER = "GET /repos/{owner}/{repo}/branches/{branch}/protection/restrictions/users";
+const REVIEWER = "POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers";
 
 /**
  *
@@ -45,39 +44,17 @@ export async function pullRequestInfo(
 	log: CommandLogger,
 ): Promise<any> {
 	const octokit = new Octokit({ auth: token });
+
 	const prInfo = await octokit.request(PULL_REQUEST_INFO, {
 		owner,
 		repo,
-		commit_sha,
+		ref: commit_sha,
 	});
 
-	log.verbose(`Get pull request info for ${commit_sha}: ${JSON.stringify(prInfo)}`);
+	log.verbose(`Get info from ref: ${JSON.stringify(prInfo)}`);
 	return prInfo;
 }
 
-/**
- *
- * @param token - GitHub authentication token
- * @returns Lists the user who have push access to this branch
- */
-export async function getUserAccess(
-	token: string,
-	owner: string,
-	repo: string,
-	log: CommandLogger,
-): Promise<any> {
-	const octokit = new Octokit({ auth: token });
-
-	const user = await octokit.request(GET_USER, {
-		owner,
-		repo,
-		branch: "main",
-	});
-
-	log.verbose(`Get list of users with push access ${JSON.stringify(user)}`);
-	return user;
-}
-
 /**
  *
  * @param auth - GitHub authentication token
@@ -96,6 +73,7 @@ export async function createPullRequest(
 		assignee: string;
 		title: string;
 		description: string;
+		reviewers: string[];
 	},
 	log: CommandLogger,
 ): Promise<any> {
@@ -118,12 +96,13 @@ export async function createPullRequest(
 		assignees: [pr.assignee],
 	});
 
-	log.verbose(`Adding reviewer to pull request ${newPr.data.number}`);
+	log.log(`Adding reviewer to pull request ${newPr.data.number}`);
 	await octokit.request(REVIEWER, {
 		owner: pr.owner,
 		repo: pr.repo,
 		pull_number: newPr.data.number,
-		reviewer: [],
+		reviewers: pr.reviewers,
+		team_reviewers: [""],
 	});
 
 	log.verbose(`Adding label to pull request ${newPr.data.number}`);

build-tools/packages/build-cli/src/lib/index.ts

@@ -31,7 +31,7 @@ export {
 } from "./package";
 export { difference } from "./sets";
 export { getIndent, indentString } from "./text";
-export { createPullRequest, getUserAccess, pullRequestExists, pullRequestInfo } from "./github";
+export { createPullRequest, pullRequestExists, pullRequestInfo } from "./github";
 export {
 	getRanges,
 	PackageVersionList,