diff --git a/src/chromium-handler.ts b/src/chromium-handler.ts index 9d37c3b..1db9489 100644 --- a/src/chromium-handler.ts +++ b/src/chromium-handler.ts @@ -12,7 +12,19 @@ import { Octokit } from '@octokit/rest'; type BranchItem = ReposGetBranchResponseItem | ReposListBranchesResponseItem; -async function rollReleaseBranch(github: Octokit, branch: BranchItem) { +// The outcome of a main branch roll: the Chromium version main is currently on +// (its landed DEPS version, not the version the open roll PR targets) and the +// release branches the roll PR covers with `target/N-x-y` labels. +interface MainRollResult { + currentVersion: string; + coveredBranches: string[]; +} + +async function rollReleaseBranch( + github: Octokit, + branch: BranchItem, + mainRoll?: MainRollResult | null, +) { const d = debug(`roller/chromium:rollReleaseBranch('${branch.name}')`); d(`Fetching DEPS for ${branch.name}`); @@ -36,6 +48,21 @@ async function rollReleaseBranch(github: Octokit, branch: BranchItem) { throw new Error(`${branch.name} roll failed: ${chromiumVersion} is not a valid version number`); } + // A branch covered by a target/ label on the main roll PR receives the main + // roll as a backport instead of an independent roll - but only skip it while + // it has kept pace with the Chromium version main has actually landed, so a + // branch whose backports stall can pull itself forward with its own roll. + // Explicitly targeted rolls pass no main roll info and are never suppressed. + if ( + mainRoll?.coveredBranches.includes(branch.name) && + compareChromiumVersions(chromiumVersion, mainRoll.currentVersion) >= 0 + ) { + d( + `${branch.name} is covered by the ${MAIN_BRANCH} roll and has kept pace with ${MAIN_BRANCH} at ${mainRoll.currentVersion} - skipping independent roll`, + ); + return; + } + d(`Computing latest upstream version for Chromium ${chromiumMajorVersion}`); const chromiumReleases = await getChromiumReleases({ milestone: chromiumMajorVersion }); const latestUpstreamVersion = chromiumReleases[chromiumReleases.length - 1]; @@ -61,7 +88,7 @@ async function rollReleaseBranch(github: Octokit, branch: BranchItem) { } } -async function rollMainBranch(github: Octokit) { +async function rollMainBranch(github: Octokit): Promise { const d = debug('roller/chromium:rollMainBranch()'); d(`Fetching ${MAIN_BRANCH} branch for electron/electron`); @@ -101,17 +128,18 @@ async function rollMainBranch(github: Octokit) { if (latestUpstreamVersion && currentVersion !== latestUpstreamVersion) { d(`Updating ${MAIN_BRANCH} from ${currentVersion} to ${latestUpstreamVersion}`); try { - await roll({ + const coveredBranches = await roll({ rollTarget: ROLL_TARGETS.chromium, electronBranch: mainBranch, targetVersion: latestUpstreamVersion, }); + return { currentVersion, coveredBranches: coveredBranches ?? [] }; } catch (e) { throw new Error(`Failed to roll ${MAIN_BRANCH} to ${latestUpstreamVersion}: ${e.message}`); } } - return true; + return null; } export async function handleChromiumCheck(target?: string): Promise { @@ -154,21 +182,24 @@ export async function handleChromiumCheck(target?: string): Promise { const releaseBranches = branches.filter((branch) => supported.includes(branch.name)); d(`Found ${releaseBranches.length} release branches`); + // Roll main first, so that the release branches its roll PR covers with + // target/ labels can skip their own rolls in favor of the backports. + let mainRoll: MainRollResult | null = null; + try { + mainRoll = await rollMainBranch(github); + } catch (e) { + failed = true; + } + // Roll all non-main release branches. for (const branch of releaseBranches) { try { - await rollReleaseBranch(github, branch); + await rollReleaseBranch(github, branch, mainRoll); } catch (e) { failed = true; continue; } } - - try { - await rollMainBranch(github); - } catch (e) { - failed = true; - } } if (failed) { diff --git a/src/utils/get-target-branch-labels.ts b/src/utils/get-target-branch-labels.ts new file mode 100644 index 0000000..4e7c00a --- /dev/null +++ b/src/utils/get-target-branch-labels.ts @@ -0,0 +1,47 @@ +import { Octokit } from '@octokit/rest'; + +import { REPOS } from '../constants.js'; +import { ReposListBranchesResponseItem } from '../types.js'; +import { getSupportedBranches } from './get-supported-branches.js'; + +export const ELECTRON_RELEASE_SCHEDULE_URL = 'https://releases.electronjs.org/schedule.json'; + +interface ReleaseScheduleEntry { + version: string; + branch: string; + chromiumVersion: number; +} + +// Returns the names of the supported release branches whose scheduled Chromium +// version is greater than or equal to the given Chromium major, per +// https://releases.electronjs.org/schedule - i.e. the branches whose Chromium +// upgrade is tracked by rolls to the main branch and should receive them as +// backports (via `target/N-x-y` labels) rather than independent rolls. +export async function getBranchesTrackedByMain( + octokit: Octokit, + chromiumMajorVersion: number, +): Promise { + const response = await fetch(ELECTRON_RELEASE_SCHEDULE_URL, { + headers: { accept: 'application/json' }, + }); + if (!response.ok) { + throw new Error(`Failed to fetch Electron release schedule: ${response.status}`); + } + const schedule = (await response.json()) as ReleaseScheduleEntry[]; + + const branches: ReposListBranchesResponseItem[] = await octokit.paginate( + octokit.repos.listBranches.endpoint.merge({ + ...REPOS.electron, + protected: true, + }), + ); + const supported = getSupportedBranches(branches); + + return schedule + .filter( + (entry) => + supported.includes(entry.branch) && Number(entry.chromiumVersion) >= chromiumMajorVersion, + ) + .map((entry) => entry.branch) + .sort(); +} diff --git a/src/utils/roll.ts b/src/utils/roll.ts index c1e26b0..c9bbfde 100644 --- a/src/utils/roll.ts +++ b/src/utils/roll.ts @@ -17,6 +17,7 @@ import { getPRText } from './pr-text.js'; import { updateDepsFile } from './update-deps.js'; import { Octokit } from '@octokit/rest'; import { addLabels, removeLabel } from './label-utils.js'; +import { getBranchesTrackedByMain } from './get-target-branch-labels.js'; interface RollParams { rollTarget: RollTarget; @@ -26,20 +27,113 @@ interface RollParams { previousVersion?: string; } +const TARGET_BRANCH_LABEL_PATTERN = /^target\/\d+-(?:\d+-x|x-y)$/; + +// The main branch roll PR is long-lived and relabeled on every update, and +// labels are otherwise only ever added - so a `target/N-x-y` label that no +// longer qualifies (main rolled past the branch's scheduled Chromium version) +// or a `no-backport` added while the schedule was unavailable would stick +// around forever. Remove only those roller-managed labels; never touch +// anything else (merged/*, trop, semver, ...) and never replace the label set. +// Throws if any stale label could not be confirmed removed. +async function removeStaleBackportLabels( + octokit: Octokit, + prNumber: number, + targetBranchLabels: string[], +) { + const d = debug('roller/chromium:removeStaleBackportLabels()'); + + const { data: labelData } = await octokit.issues.listLabelsOnIssue({ + ...REPOS.electron, + issue_number: prNumber, + per_page: 100, + page: 1, + }); + + const staleLabels = labelData + .map((label) => label.name) + .filter( + (name) => + (TARGET_BRANCH_LABEL_PATTERN.test(name) && !targetBranchLabels.includes(name)) || + (name === NO_BACKPORT && targetBranchLabels.length > 0), + ); + + for (const name of staleLabels) { + d(`Removing stale backport label ${name} from #${prNumber}`); + try { + await octokit.issues.removeLabel({ + ...REPOS.electron, + issue_number: prNumber, + name, + }); + } catch (e) { + // Already removed out from under us - that still counts as removed. + if (e.status === 404) continue; + throw e; + } + } +} + +// Updates the labels on a roll PR. Returns the names of the release branches +// the PR's `target/N-x-y` labels cover - non-empty only for Chromium rolls to +// the main branch whose backport labels were successfully reconciled. async function updateLabels( octokit: Octokit, { rollTarget, electronBranch, targetVersion, previousVersion, prNumber }: RollParams, -) { + isNewPr = false, +): Promise { + const d = debug(`roller/${rollTarget.name}:updateLabels()`); let labels: string[] = []; let labelToRemove: string; + let coveredBranches: string[] = []; + + if (electronBranch.name === MAIN_BRANCH) { + let targetBranchLabels: string[] = []; + let reconciled = false; + + // Chromium rolls to main should be labeled for backport to every supported + // release branch whose scheduled Chromium version is >= the rolled version. + if (rollTarget === ROLL_TARGETS.chromium) { + try { + const chromiumMajorVersion = Number(targetVersion.split('.')[0]); + if (Number.isNaN(chromiumMajorVersion)) { + throw new Error(`${targetVersion} is not a valid version number`); + } + const trackedBranches = await getBranchesTrackedByMain(octokit, chromiumMajorVersion); + targetBranchLabels = trackedBranches.map((branch) => `target/${branch}`); + // The PR must never carry both no-backport and target/ labels - trop + // rejects that as ambiguous. Only transition to the new backport label + // set once every label conflicting with it is confirmed removed. + await removeStaleBackportLabels(octokit, prNumber, targetBranchLabels); + coveredBranches = trackedBranches; + reconciled = true; + } catch (e) { + // Leave the PR's existing backport labels exactly as they were - a + // half-applied transition could strand conflicting labels on the PR. + targetBranchLabels = []; + coveredBranches = []; + d(`Failed to reconcile backport labels: ${e.message} - leaving existing labels unchanged`); + } + } - labels.push(electronBranch.name === MAIN_BRANCH ? NO_BACKPORT : BACKPORT_CHECK_SKIP); + if (targetBranchLabels.length > 0) { + d(`Adding target branch labels: ${targetBranchLabels.join(', ')}`); + labels.push(...targetBranchLabels); + } else if (rollTarget !== ROLL_TARGETS.chromium || reconciled || isNewPr) { + // A reconciled empty set means no release branch qualifies; a brand-new + // PR has no existing backport labels to preserve, so it can take the + // fallback even when reconciliation failed. + labels.push(NO_BACKPORT); + } + } else { + labels.push(BACKPORT_CHECK_SKIP); + } // Chromium bumps & roll bumps to the main branch are always patch bumps. if (electronBranch.name === MAIN_BRANCH || rollTarget === ROLL_TARGETS.chromium) { labels.push('semver/patch'); await addLabels(octokit, { prNumber, labels }); - return; + return coveredBranches; } // Check Node.js rolls against previous version and determine the semver label to add. @@ -54,6 +148,8 @@ async function updateLabels( await removeLabel(octokit, { prNumber, name: labelToRemove }); await addLabels(octokit, { prNumber, labels }); + + return coveredBranches; } async function triggerChromiumUpgradeWorkflow(octokit: Octokit) { @@ -66,11 +162,15 @@ async function triggerChromiumUpgradeWorkflow(octokit: Octokit) { } } +// Rolls `rollTarget` on `electronBranch` to `targetVersion`. Returns the names +// of the release branches covered by `target/N-x-y` labels on the roll PR - +// non-empty only for a Chromium roll to the main branch whose PR was +// successfully created or updated and correctly labeled. export async function roll({ rollTarget, electronBranch, targetVersion, -}: RollParams): Promise { +}: RollParams): Promise { const d = debug(`roller/${rollTarget.name}:roll()`); const github = await getOctokit(); @@ -79,6 +179,7 @@ export async function roll({ ); let didRoll = false; + let coveredBranches: string[] = []; // Look for a pre-existing PR that targets this branch to see if we can update that. const existingPrsForBranch = (await github.paginate('GET /repos/:owner/:repo/pulls', { @@ -140,6 +241,19 @@ export async function roll({ if (previousDEPSVersion === newDEPSVersion) { d(`DEPS version unchanged - skipping PR body update`); + // The release schedule moves independently of Chromium - a newly cut + // release branch, a schedule edit, or a label call that failed on a + // previous run must still reconcile the labels on the open roll PR + // even on a day with no DEPS change. + if (rollTarget === ROLL_TARGETS.chromium) { + coveredBranches = await updateLabels(github, { + rollTarget, + electronBranch, + targetVersion, + previousVersion: previousDEPSVersion, + prNumber: pr.number, + }); + } continue; } @@ -150,7 +264,7 @@ export async function roll({ if (!prVersionText || prVersionText.length === 0) { d('Could not find PR version text in existing PR - exiting'); - return; + return coveredBranches; } await github.pulls.update({ @@ -164,7 +278,7 @@ export async function roll({ }), }); - await updateLabels(github, { + coveredBranches = await updateLabels(github, { rollTarget, electronBranch, targetVersion, @@ -218,13 +332,17 @@ export async function roll({ }), }); - await updateLabels(github, { - rollTarget, - electronBranch, - targetVersion, - previousVersion: previousDEPSVersion, - prNumber: newPr.data.number, - }); + coveredBranches = await updateLabels( + github, + { + rollTarget, + electronBranch, + targetVersion, + previousVersion: previousDEPSVersion, + prNumber: newPr.data.number, + }, + true, + ); d(`New PR: ${newPr.data.html_url}`); @@ -234,4 +352,6 @@ export async function roll({ if (didRoll && rollTarget === ROLL_TARGETS.chromium && electronBranch.name === MAIN_BRANCH) { await triggerChromiumUpgradeWorkflow(github); } + + return coveredBranches; } diff --git a/tests/fixtures/electron-release-schedule.json b/tests/fixtures/electron-release-schedule.json new file mode 100644 index 0000000..5ce1c22 --- /dev/null +++ b/tests/fixtures/electron-release-schedule.json @@ -0,0 +1,68 @@ +[ + { + "version": "46.0.0", + "branch": "main", + "alphaDate": "2026-10-22", + "betaDate": "2026-12-01", + "stableDate": "2027-01-05", + "chromiumVersion": 160, + "nodeVersion": "24.20.0", + "eolDate": "2027-06-22", + "status": "nightly" + }, + { + "version": "45.0.0", + "branch": "45-x-y", + "alphaDate": "2026-08-27", + "betaDate": "2026-09-29", + "stableDate": "2026-10-20", + "chromiumVersion": 156, + "nodeVersion": "24.19.0", + "eolDate": "2027-04-27", + "status": "prerelease" + }, + { + "version": "44.0.0", + "branch": "44-x-y", + "alphaDate": "2026-07-02", + "betaDate": "2026-07-28", + "stableDate": "2026-08-25", + "chromiumVersion": 152, + "nodeVersion": "24.18.1", + "eolDate": "2027-03-02", + "status": "stable" + }, + { + "version": "43.0.0", + "branch": "43-x-y", + "alphaDate": "2026-05-07", + "betaDate": "2026-06-02", + "stableDate": "2026-06-30", + "chromiumVersion": 150, + "nodeVersion": "24.17.0", + "eolDate": "2027-01-05", + "status": "stable" + }, + { + "version": "42.0.0", + "branch": "42-x-y", + "alphaDate": "2026-03-12", + "betaDate": "2026-04-07", + "stableDate": "2026-05-05", + "chromiumVersion": 148, + "nodeVersion": "24.15.0", + "eolDate": "2026-10-20", + "status": "stable" + }, + { + "version": "41.0.0", + "branch": "41-x-y", + "alphaDate": "2026-01-15", + "betaDate": "2026-02-10", + "stableDate": "2026-03-10", + "chromiumVersion": 146, + "nodeVersion": "24.14.0", + "eolDate": "2026-08-25", + "status": "eol" + } +] diff --git a/tests/handlers.spec.ts b/tests/handlers.spec.ts index cde9996..4eb080f 100644 --- a/tests/handlers.spec.ts +++ b/tests/handlers.spec.ts @@ -41,6 +41,7 @@ describe('handleChromiumCheck()', () => { }, }; vi.mocked(getOctokit).mockReturnValue(mockOctokit); + vi.mocked(roll).mockReset().mockResolvedValue([]); }); describe('release branches', () => { @@ -144,6 +145,118 @@ describe('handleChromiumCheck()', () => { ); }); + it('skips a release branch the main roll covers when it has kept pace with main', async () => { + vi.mocked(getChromiumReleases).mockResolvedValue(['1.1.0.0', '1.2.0.0']); + // Main and the branch are level on 1.0.0.0 (the branch's backports have + // kept pace with what main has landed) while main rolls to 1.2.0.0. + // The main roll PR covers 4-0-x with a target/ label. + vi.mocked(roll).mockResolvedValueOnce(['4-0-x']); + + await handleChromiumCheck(); + + expect(roll).toHaveBeenCalledTimes(1); + expect(roll).toHaveBeenCalledWith( + expect.objectContaining({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: expect.objectContaining({ name: MAIN_BRANCH }), + targetVersion: '1.2.0.0', + }), + ); + }); + + it('skips a covered branch level with main while the main roll PR is ahead of both', async () => { + // The everyday path: main has landed 1.1.0.0, its open roll PR targets + // 1.2.0.0, and the branch's backports have kept it level with main. + vi.mocked(getChromiumReleases).mockResolvedValue(['1.1.0.0', '1.2.0.0']); + vi.mocked(getContent).mockResolvedValue({ + content: `${ROLL_TARGETS.chromium.depsKey}':\n '1.1.0.0',`, + sha: '1234', + }); + vi.mocked(roll).mockResolvedValueOnce(['4-0-x']); + + await handleChromiumCheck(); + + expect(roll).toHaveBeenCalledTimes(1); + expect(roll).toHaveBeenCalledWith( + expect.objectContaining({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: expect.objectContaining({ name: MAIN_BRANCH }), + targetVersion: '1.2.0.0', + }), + ); + }); + + it('rolls a covered branch independently while it lags what main has landed', async () => { + vi.mocked(getChromiumReleases).mockResolvedValue(['1.1.0.0', '1.2.0.0']); + // Main has landed 1.1.0.0 but the branch is still on 1.0.0.0 - its + // backport stalled, so it must be able to pull itself forward. + vi.mocked(getContent) + .mockResolvedValueOnce({ + content: `${ROLL_TARGETS.chromium.depsKey}':\n '1.1.0.0',`, + sha: '1234', + }) + .mockResolvedValue({ + content: `${ROLL_TARGETS.chromium.depsKey}':\n '1.0.0.0',`, + sha: '1234', + }); + vi.mocked(roll).mockResolvedValueOnce(['4-0-x']); + + await handleChromiumCheck(); + + expect(roll).toHaveBeenCalledTimes(2); + expect(roll).toHaveBeenCalledWith( + expect.objectContaining({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: expect.objectContaining({ name: '4-0-x' }), + targetVersion: '1.2.0.0', + }), + ); + }); + + it('does not skip release branches if the main roll fails', async () => { + vi.mocked(getChromiumReleases).mockResolvedValue(['1.1.0.0', '1.2.0.0']); + vi.mocked(roll).mockImplementationOnce(() => { + throw new Error('main roll failed'); + }); + + await expect(handleChromiumCheck()).rejects.toThrowError( + 'One or more upgrade checks failed - see logs for more details', + ); + + expect(roll).toHaveBeenCalledTimes(2); + expect(roll).toHaveBeenLastCalledWith( + expect.objectContaining({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: expect.objectContaining({ name: '4-0-x' }), + targetVersion: '1.2.0.0', + }), + ); + }); + + it('never skips an explicitly targeted release branch roll', async () => { + vi.mocked(getChromiumReleases).mockResolvedValue(['1.1.0.0', '1.2.0.0']); + + mockOctokit.repos.getBranch.mockReturnValue({ + data: { + name: '4-0-x', + commit: { + sha: '1234', + }, + }, + }); + + await handleChromiumCheck('4-0-x'); + + expect(roll).toHaveBeenCalledTimes(1); + expect(roll).toHaveBeenCalledWith( + expect.objectContaining({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: expect.objectContaining({ name: '4-0-x' }), + targetVersion: '1.2.0.0', + }), + ); + }); + it('fails if an invalid target is passed', async () => { mockOctokit.repos.getBranch.mockReturnValue(null); diff --git a/tests/utils/get-target-branch-labels.spec.ts b/tests/utils/get-target-branch-labels.spec.ts new file mode 100644 index 0000000..824b928 --- /dev/null +++ b/tests/utils/get-target-branch-labels.spec.ts @@ -0,0 +1,88 @@ +import fs from 'node:fs'; +import path from 'node:path'; + +import nock from 'nock'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { + ELECTRON_RELEASE_SCHEDULE_URL, + getBranchesTrackedByMain, +} from '../../src/utils/get-target-branch-labels.js'; + +describe('getBranchesTrackedByMain', () => { + let mockOctokit: any; + + const fixture = fs.readFileSync( + path.join(import.meta.dirname, '../fixtures/electron-release-schedule.json'), + 'utf8', + ); + const url = new URL(ELECTRON_RELEASE_SCHEDULE_URL); + + beforeEach(() => { + nock.cleanAll(); + mockOctokit = { + paginate: vi.fn().mockResolvedValue( + // 41-x-y is EOL and outside the supported window of 4. + ['41-x-y', '42-x-y', '43-x-y', '44-x-y', '45-x-y'].map((name) => ({ name })), + ), + repos: { + listBranches: { + endpoint: { + merge: vi.fn(), + }, + }, + }, + }; + }); + + it('returns supported branches scheduled for >= the rolled major', async () => { + nock(url.origin).get(url.pathname).reply(200, fixture); + + // 45-x-y ships Chromium 156, 44-x-y ships 152 - rolling to 154 only + // targets 45-x-y. + await expect(getBranchesTrackedByMain(mockOctokit, 154)).resolves.toEqual(['45-x-y']); + }); + + it('includes a branch whose scheduled major equals the rolled major', async () => { + nock(url.origin).get(url.pathname).reply(200, fixture); + + await expect(getBranchesTrackedByMain(mockOctokit, 156)).resolves.toEqual(['45-x-y']); + }); + + it('returns every matching supported branch', async () => { + nock(url.origin).get(url.pathname).reply(200, fixture); + + await expect(getBranchesTrackedByMain(mockOctokit, 150)).resolves.toEqual([ + '43-x-y', + '44-x-y', + '45-x-y', + ]); + }); + + it('returns no branches if the rolled major is newer than every scheduled version', async () => { + nock(url.origin).get(url.pathname).reply(200, fixture); + + await expect(getBranchesTrackedByMain(mockOctokit, 157)).resolves.toEqual([]); + }); + + it('ignores schedule entries for unsupported branches', async () => { + nock(url.origin).get(url.pathname).reply(200, fixture); + + // 41-x-y ships Chromium 146 but is no longer supported, and main is not a + // release branch - neither should be returned. + await expect(getBranchesTrackedByMain(mockOctokit, 140)).resolves.toEqual([ + '42-x-y', + '43-x-y', + '44-x-y', + '45-x-y', + ]); + }); + + it('throws if the schedule fetch fails', async () => { + nock(url.origin).get(url.pathname).reply(500); + + await expect(getBranchesTrackedByMain(mockOctokit, 154)).rejects.toThrowError( + 'Failed to fetch Electron release schedule: 500', + ); + }); +}); diff --git a/tests/utils/roll.spec.ts b/tests/utils/roll.spec.ts index 5dd69e8..d6422fe 100644 --- a/tests/utils/roll.spec.ts +++ b/tests/utils/roll.spec.ts @@ -3,15 +3,19 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { roll } from '../../src/utils/roll.js'; import { getOctokit } from '../../src/utils/octokit.js'; import { + BACKPORT_CHECK_SKIP, CHROMIUM_UPGRADE_WORKFLOW, MAIN_BRANCH, + NO_BACKPORT, REPOS, ROLL_TARGETS, } from '../../src/constants.js'; +import { getBranchesTrackedByMain } from '../../src/utils/get-target-branch-labels.js'; import { updateDepsFile } from '../../src/utils/update-deps.js'; vi.mock('../../src/utils/octokit.js'); vi.mock('../../src/utils/update-deps.js'); +vi.mock('../../src/utils/get-target-branch-labels.js'); describe('roll()', () => { let mockOctokit: any; @@ -47,6 +51,7 @@ describe('roll()', () => { }, issues: { addLabels: vi.fn(), + removeLabel: vi.fn(), listLabelsOnIssue: vi.fn().mockReturnValue({ data: [] }), }, actions: { @@ -58,6 +63,7 @@ describe('roll()', () => { previousDEPSVersion: 'v4.0.0', newDEPSVersion: 'v10.0.0', }); + vi.mocked(getBranchesTrackedByMain).mockReset().mockResolvedValue([]); }); it('takes no action if versions are identical', async () => { @@ -361,12 +367,299 @@ describe('roll()', () => { electronBranch: mainBranch, targetVersion: '120.0.0.0', }), - ).resolves.toBeUndefined(); + ).resolves.toEqual([]); expect(mockOctokit.pulls.create).toHaveBeenCalled(); }); }); + describe('backport labels', () => { + const mainBranch = { ...branch, name: MAIN_BRANCH }; + const existingMainChromiumPr = { + user: { login: 'electron-roller[bot]' }, + title: `chore: bump ${ROLL_TARGETS.chromium.name} to bar`, + number: 1, + head: { + ref: `roller/${ROLL_TARGETS.chromium.name}/${MAIN_BRANCH}`, + repo: { full_name: `${REPOS.electron.owner}/${REPOS.electron.repo}` }, + }, + body: 'Original-Version: 119.0.0.0', + labels: [], + created_at: new Date().toISOString(), + }; + + it('adds target branch labels instead of no-backport for chromium rolls on main', async () => { + mockOctokit.paginate.mockReturnValue([]); + vi.mocked(getBranchesTrackedByMain).mockResolvedValue(['44-x-y', '45-x-y']); + + await expect( + roll({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: mainBranch, + targetVersion: '152.0.0.0', + }), + ).resolves.toEqual(['44-x-y', '45-x-y']); + + expect(getBranchesTrackedByMain).toHaveBeenCalledWith(mockOctokit, 152); + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ + labels: ['target/44-x-y', 'target/45-x-y', 'semver/patch'], + }), + ); + }); + + it('adds no-backport for chromium rolls on main when no target branch applies', async () => { + mockOctokit.paginate.mockReturnValue([]); + vi.mocked(getBranchesTrackedByMain).mockResolvedValue([]); + + await expect( + roll({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: mainBranch, + targetVersion: '160.0.0.0', + }), + ).resolves.toEqual([]); + + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ + labels: [NO_BACKPORT, 'semver/patch'], + }), + ); + }); + + it('falls back to no-backport on a new PR if target branches cannot be determined', async () => { + mockOctokit.paginate.mockReturnValue([]); + vi.mocked(getBranchesTrackedByMain).mockRejectedValue(new Error('schedule unavailable')); + + await expect( + roll({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: mainBranch, + targetVersion: '152.0.0.0', + }), + ).resolves.toEqual([]); + + expect(mockOctokit.pulls.create).toHaveBeenCalled(); + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ + labels: [NO_BACKPORT, 'semver/patch'], + }), + ); + }); + + it('leaves the labels of an existing PR unchanged if target branches cannot be determined', async () => { + mockOctokit.paginate.mockReturnValue([existingMainChromiumPr]); + mockOctokit.issues.listLabelsOnIssue.mockReturnValue({ + data: [{ name: 'target/45-x-y' }], + }); + vi.mocked(getBranchesTrackedByMain).mockRejectedValue(new Error('schedule unavailable')); + + await expect( + roll({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: mainBranch, + targetVersion: '154.0.0.0', + }), + ).resolves.toEqual([]); + + // The PR keeps its existing backport labels: nothing is removed, and + // neither no-backport nor target labels are added on top of them. + expect(mockOctokit.issues.removeLabel).not.toHaveBeenCalled(); + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ + labels: ['semver/patch'], + }), + ); + }); + + it('falls back without fetching the schedule for an invalid target version', async () => { + mockOctokit.paginate.mockReturnValue([]); + + await roll({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: mainBranch, + targetVersion: 'not-a-version', + }); + + expect(getBranchesTrackedByMain).not.toHaveBeenCalled(); + expect(mockOctokit.issues.removeLabel).not.toHaveBeenCalled(); + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ + labels: [NO_BACKPORT, 'semver/patch'], + }), + ); + }); + + it('removes stale roller-managed labels that no longer apply', async () => { + mockOctokit.paginate.mockReturnValue([]); + mockOctokit.issues.listLabelsOnIssue.mockReturnValue({ + data: [ + { name: NO_BACKPORT }, + { name: 'target/43-x-y' }, + { name: 'target/4-0-x' }, + { name: 'target/45-x-y' }, + { name: 'merged/44-x-y' }, + { name: 'in-flight/45-x-y' }, + { name: 'semver/patch' }, + ], + }); + vi.mocked(getBranchesTrackedByMain).mockResolvedValue(['45-x-y']); + + await roll({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: mainBranch, + targetVersion: '154.0.0.0', + }); + + // target/43-x-y and target/4-0-x no longer qualify and no-backport + // conflicts with the target labels that do - all three are removed, + // nothing else is touched. + expect(mockOctokit.issues.removeLabel).toHaveBeenCalledTimes(3); + expect(mockOctokit.issues.removeLabel).toHaveBeenCalledWith( + expect.objectContaining({ name: 'target/43-x-y' }), + ); + expect(mockOctokit.issues.removeLabel).toHaveBeenCalledWith( + expect.objectContaining({ name: 'target/4-0-x' }), + ); + expect(mockOctokit.issues.removeLabel).toHaveBeenCalledWith( + expect.objectContaining({ name: NO_BACKPORT }), + ); + }); + + it('removes stale target labels but keeps no-backport when no target branch applies', async () => { + mockOctokit.paginate.mockReturnValue([]); + mockOctokit.issues.listLabelsOnIssue.mockReturnValue({ + data: [{ name: NO_BACKPORT }, { name: 'target/45-x-y' }], + }); + vi.mocked(getBranchesTrackedByMain).mockResolvedValue([]); + + await roll({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: mainBranch, + targetVersion: '160.0.0.0', + }); + + expect(mockOctokit.issues.removeLabel).toHaveBeenCalledTimes(1); + expect(mockOctokit.issues.removeLabel).toHaveBeenCalledWith( + expect.objectContaining({ name: 'target/45-x-y' }), + ); + }); + + it('does not transition backport labels if a stale label removal fails', async () => { + mockOctokit.paginate.mockReturnValue([existingMainChromiumPr]); + mockOctokit.issues.listLabelsOnIssue.mockReturnValue({ + data: [{ name: 'target/43-x-y' }], + }); + mockOctokit.issues.removeLabel.mockRejectedValue(new Error('server error')); + vi.mocked(getBranchesTrackedByMain).mockResolvedValue(['45-x-y']); + + // The stale target/43-x-y could not be confirmed removed, so the new + // label set must not be applied on top of it - and the failure must not + // fail the roll. + await expect( + roll({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: mainBranch, + targetVersion: '154.0.0.0', + }), + ).resolves.toEqual([]); + + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ + labels: ['semver/patch'], + }), + ); + }); + + it('treats an already-removed stale label as removed', async () => { + mockOctokit.paginate.mockReturnValue([]); + mockOctokit.issues.listLabelsOnIssue.mockReturnValue({ + data: [{ name: NO_BACKPORT }], + }); + mockOctokit.issues.removeLabel.mockRejectedValue( + Object.assign(new Error('Not Found'), { status: 404 }), + ); + vi.mocked(getBranchesTrackedByMain).mockResolvedValue(['45-x-y']); + + await expect( + roll({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: mainBranch, + targetVersion: '154.0.0.0', + }), + ).resolves.toEqual(['45-x-y']); + + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ + labels: ['target/45-x-y', 'semver/patch'], + }), + ); + }); + + it('reconciles labels on an existing PR even when the DEPS version is unchanged', async () => { + mockOctokit.paginate.mockReturnValue([existingMainChromiumPr]); + mockOctokit.issues.listLabelsOnIssue.mockReturnValue({ + data: [{ name: 'target/45-x-y' }], + }); + vi.mocked(updateDepsFile).mockResolvedValue({ + previousDEPSVersion: '154.0.0.0', + newDEPSVersion: '154.0.0.0', + }); + // A newly cut release branch appears in the schedule without any DEPS + // change - the open roll PR must still gain its target label. + vi.mocked(getBranchesTrackedByMain).mockResolvedValue(['45-x-y', '46-x-y']); + + await expect( + roll({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: mainBranch, + targetVersion: '154.0.0.0', + }), + ).resolves.toEqual(['45-x-y', '46-x-y']); + + expect(mockOctokit.pulls.update).not.toHaveBeenCalled(); + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ + labels: ['target/45-x-y', 'target/46-x-y', 'semver/patch'], + }), + ); + }); + + it('adds no-backport for node rolls on main without checking the schedule', async () => { + mockOctokit.paginate.mockReturnValue([]); + + await roll({ + rollTarget: ROLL_TARGETS.node, + electronBranch: mainBranch, + targetVersion: 'v10.0.0', + }); + + expect(getBranchesTrackedByMain).not.toHaveBeenCalled(); + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ + labels: [NO_BACKPORT, 'semver/patch'], + }), + ); + }); + + it('adds backport-check-skip for chromium rolls on a release branch', async () => { + mockOctokit.paginate.mockReturnValue([]); + + await roll({ + rollTarget: ROLL_TARGETS.chromium, + electronBranch: branch, + targetVersion: '152.0.0.0', + }); + + expect(getBranchesTrackedByMain).not.toHaveBeenCalled(); + expect(mockOctokit.issues.addLabels).toHaveBeenCalledWith( + expect.objectContaining({ + labels: [BACKPORT_CHECK_SKIP, 'semver/patch'], + }), + ); + }); + }); + it('skips PR if existing one has been paused', async () => { mockOctokit.paginate.mockReturnValue([ {