From 214041a34fa7c529f88c6126a079d46c7769a0f2 Mon Sep 17 00:00:00 2001 From: Jono Kolnik <1164060+JonathanKolnik@users.noreply.github.com> Date: Wed, 10 Jan 2024 15:28:28 -0500 Subject: [PATCH 1/3] detech merge queue branch and retrieve real branch name from pull request --- ...etBranchFromMergeQueuePullRequestNumber.ts | 30 +++++++++++++++++++ node-src/git/getCommitAndBranch.test.ts | 18 ++++++++++- node-src/git/getCommitAndBranch.ts | 17 +++++++++-- node-src/git/git.test.ts | 19 +++++++++++- node-src/git/git.ts | 7 +++++ 5 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 node-src/git/getBranchFromMergeQueuePullRequestNumber.ts diff --git a/node-src/git/getBranchFromMergeQueuePullRequestNumber.ts b/node-src/git/getBranchFromMergeQueuePullRequestNumber.ts new file mode 100644 index 000000000..6a5e03e38 --- /dev/null +++ b/node-src/git/getBranchFromMergeQueuePullRequestNumber.ts @@ -0,0 +1,30 @@ +import gql from 'fake-tag'; +import { Context } from '../types'; + +const MergeQueueOriginalBranchQuery = gql` + query MergeQueueOriginalBranchQuery($number: Int!) { + app { + pullRequest(number: $number) { + branch: headRefName + } + } + } +` +interface MergeQueueOriginalBranchQueryResult { + app: { + pullRequest: { + branch: string; + }; + }; +} + +export async function getBranchFromMergeQueuePullRequestNumber( + ctx: Pick, + { number }: { number: number } +) { + const { app } = await ctx.client.runQuery(MergeQueueOriginalBranchQuery, { + number + }); + + return app?.pullRequest?.branch; +} diff --git a/node-src/git/getCommitAndBranch.test.ts b/node-src/git/getCommitAndBranch.test.ts index 5fc63ea94..20f8f0be2 100644 --- a/node-src/git/getCommitAndBranch.test.ts +++ b/node-src/git/getCommitAndBranch.test.ts @@ -2,15 +2,18 @@ import envCi from 'env-ci'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as git from './git'; - +import * as mergeQueue from './getBranchFromMergeQueuePullRequestNumber'; import getCommitAndBranch from './getCommitAndBranch'; vi.mock('env-ci'); vi.mock('./git'); +vi.mock('./getBranchFromMergeQueuePullRequestNumber'); const getBranch = vi.mocked(git.getBranch); const getCommit = vi.mocked(git.getCommit); const hasPreviousCommit = vi.mocked(git.hasPreviousCommit); +const getBranchFromMergeQueue = vi.mocked(mergeQueue.getBranchFromMergeQueuePullRequestNumber); +const mergeQueueBranchMatch = vi.mocked(git.mergeQueueBranchMatch ); const log = { info: vi.fn(), warn: vi.fn(), debug: vi.fn() }; @@ -26,12 +29,14 @@ beforeEach(() => { committerEmail: 'noreply@github.com', }); hasPreviousCommit.mockResolvedValue(true); + mergeQueueBranchMatch.mockResolvedValue(null); }); afterEach(() => { envCi.mockReset(); getBranch.mockReset(); getCommit.mockReset(); + getBranchFromMergeQueue.mockReset(); }); const commitInfo = { @@ -228,4 +233,15 @@ describe('getCommitAndBranch', () => { ); }); }); + + describe('with mergeQueue branch', () => { + it('uses PRs branchName as branch instead of temporary mergeQueue branch', async () => { + mergeQueueBranchMatch.mockResolvedValue(4); + getBranchFromMergeQueue.mockResolvedValue('branch-before-merge-queue'); + const info = await getCommitAndBranch({ log }, { + branchName: 'this-is-merge-queue-branch-format/main/pr-4-48e0c83fadbf504c191bc868040b7a969a4f1feb', + }); + expect(info).toMatchObject({ branch: 'branch-before-merge-queue' }); + }); + }); }); diff --git a/node-src/git/getCommitAndBranch.ts b/node-src/git/getCommitAndBranch.ts index 4f20ac369..4b9da13df 100644 --- a/node-src/git/getCommitAndBranch.ts +++ b/node-src/git/getCommitAndBranch.ts @@ -7,7 +7,9 @@ import missingTravisInfo from '../ui/messages/errors/missingTravisInfo'; import customGitHubAction from '../ui/messages/info/customGitHubAction'; import travisInternalBuild from '../ui/messages/warnings/travisInternalBuild'; import noCommitDetails from '../ui/messages/warnings/noCommitDetails'; -import { getBranch, getCommit, hasPreviousCommit } from './git'; +import { getBranch, getCommit, hasPreviousCommit, mergeQueueBranchMatch } from './git'; + +import { getBranchFromMergeQueuePullRequestNumber } from './getBranchFromMergeQueuePullRequestNumber'; const ORIGIN_PREFIX_REGEXP = /^origin\//; const notHead = (branch) => (branch && branch !== 'HEAD' ? branch : false); @@ -21,13 +23,14 @@ interface CommitInfo { } export default async function getCommitAndBranch( - { log }, + ctx, { branchName, patchBaseRef, ci, }: { branchName?: string; patchBaseRef?: string; ci?: boolean } = {} ) { + const { log } = ctx; let commit: CommitInfo = await getCommit(); let branch = notHead(branchName) || notHead(patchBaseRef) || (await getBranch()); let slug; @@ -157,6 +160,16 @@ export default async function getCommitAndBranch( branch = branch.replace(ORIGIN_PREFIX_REGEXP, ''); } + const mergeQueueBranchPrNumber = await mergeQueueBranchMatch(commit.commit, branch) + + if (mergeQueueBranchPrNumber) { + const branchFromMergeQueuePullRequestNumber = await getBranchFromMergeQueuePullRequestNumber(ctx, { number: mergeQueueBranchPrNumber }); + + if (branchFromMergeQueuePullRequestNumber) { + branch = branchFromMergeQueuePullRequestNumber; + } + } + log.debug( `git info: ${JSON.stringify({ commit, diff --git a/node-src/git/git.test.ts b/node-src/git/git.test.ts index d3eb061b6..aaca383c3 100644 --- a/node-src/git/git.test.ts +++ b/node-src/git/git.test.ts @@ -1,7 +1,7 @@ import { execaCommand } from 'execa'; import { describe, expect, it, vi } from 'vitest'; -import { getCommit, getSlug, hasPreviousCommit } from './git'; +import { getCommit, getSlug, hasPreviousCommit, mergeQueueBranchMatch } from './git'; vi.mock('execa'); @@ -91,3 +91,20 @@ gpg: Can't check signature: No public key expect(await hasPreviousCommit()).toEqual(true); }); }); + + +describe('mergeQueueBranchMatch', () => { + it('returns pr number if it is a merge queue branch', async () => { + const branch = "gh-readonly-queue/main/pr-4-da07417adc889156224d03a7466ac712c647cd36" + const commit = "da07417adc889156224d03a7466ac712c647cd36" + expect(await mergeQueueBranchMatch(commit, branch)).toEqual(4); + }); + + it('returns null if it is not a merge queue branch', async () => { + const branch = "develop" + const commit = "da07417adc889156224d03a7466ac712c647cd36" + expect(await mergeQueueBranchMatch(commit, branch)).toEqual( + null + ); + }); +}); diff --git a/node-src/git/git.ts b/node-src/git/git.ts index 70b290451..110d4ebdf 100644 --- a/node-src/git/git.ts +++ b/node-src/git/git.ts @@ -281,3 +281,10 @@ export async function findFiles(...patterns: string[]) { const files = await execGitCommand(`git ls-files -z ${patterns.map((p) => `'${p}'`).join(' ')}`); return files.split('\0').filter(Boolean); } + +export async function mergeQueueBranchMatch(commit, branch) { + const mergeQueuePattern = new RegExp(`/pr-(\\d+)-${commit}$`); + const match = branch.match(mergeQueuePattern); + + return match ? Number(match[1]) : null; +} From 6ab9b3e88a8a91ab266d431485bc6ddf8feb42b8 Mon Sep 17 00:00:00 2001 From: Jono Kolnik <1164060+JonathanKolnik@users.noreply.github.com> Date: Mon, 15 Jan 2024 10:33:07 -0500 Subject: [PATCH 2/3] add comment and make mergeQueueBranchMatch regex more specific --- node-src/git/getCommitAndBranch.ts | 8 +++++++- node-src/git/git.ts | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/node-src/git/getCommitAndBranch.ts b/node-src/git/getCommitAndBranch.ts index 4b9da13df..fa2e5547a 100644 --- a/node-src/git/getCommitAndBranch.ts +++ b/node-src/git/getCommitAndBranch.ts @@ -160,9 +160,15 @@ export default async function getCommitAndBranch( branch = branch.replace(ORIGIN_PREFIX_REGEXP, ''); } - const mergeQueueBranchPrNumber = await mergeQueueBranchMatch(commit.commit, branch) + + // When a PR is put into a merge queue, and prior PR is merged, the PR is retested against the latest on main. + // To do this, GitHub creates a new commit and does a CI run with the branch changed to e.g. gh-readonly-queue/main/pr-4-da07417adc889156224d03a7466ac712c647cd36 + // If you configure merge queues to rebase in this circumstance, + // we lose track of baselines as the branch name has changed so our usual rebase detection (based on branch name) doesn't work. + const mergeQueueBranchPrNumber = await mergeQueueBranchMatch(commit.commit, branch) if (mergeQueueBranchPrNumber) { + // This is why we extract the PR number from the branch name and use it to find the branch name of the PR that was merged. const branchFromMergeQueuePullRequestNumber = await getBranchFromMergeQueuePullRequestNumber(ctx, { number: mergeQueueBranchPrNumber }); if (branchFromMergeQueuePullRequestNumber) { diff --git a/node-src/git/git.ts b/node-src/git/git.ts index 110d4ebdf..aec6fd00f 100644 --- a/node-src/git/git.ts +++ b/node-src/git/git.ts @@ -283,7 +283,7 @@ export async function findFiles(...patterns: string[]) { } export async function mergeQueueBranchMatch(commit, branch) { - const mergeQueuePattern = new RegExp(`/pr-(\\d+)-${commit}$`); + const mergeQueuePattern = new RegExp(`gh-readonly-queue/.*/pr-(\\d+)-${commit}$`); const match = branch.match(mergeQueuePattern); return match ? Number(match[1]) : null; From 661bcd0b451e677f10c48e035ef149ce7cb95b51 Mon Sep 17 00:00:00 2001 From: Gert Hengeveld Date: Tue, 16 Jan 2024 13:35:58 +0100 Subject: [PATCH 3/3] Add missing mock --- node-src/index.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/node-src/index.test.ts b/node-src/index.test.ts index e4b855dce..04e59f2bf 100644 --- a/node-src/index.test.ts +++ b/node-src/index.test.ts @@ -293,6 +293,7 @@ vi.mock('./git/git', () => ({ getRepositoryRoot: () => Promise.resolve(process.cwd()), getUncommittedHash: () => Promise.resolve('abc123'), getUserEmail: () => Promise.resolve('test@test.com'), + mergeQueueBranchMatch: () => Promise.resolve(null), })); vi.mock('./git/getParentCommits', () => ({