Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1567,6 +1567,21 @@ export class GitHub {
.toString()
.slice(0, MAX_ISSUE_BODY_SIZE);

// re-fetch before trying to recreate the PR, in case an existing one
// was merged while this was running. this avoids modifying a PR that's
// already closed.
const currentPr = await this.octokit.pulls.get({
owner: this.repository.owner,
repo: this.repository.repo,
pull_number: number,
});
if (currentPr.data.state !== 'open') {
this.logger.warn(
`Pull request #${number} is no longer open (state: ${currentPr.data.state}) - skipping update`
);
return this.getPullRequest(number);
}

await this.createPullRequest(
{
headBranchName: releasePullRequest.headRefName,
Expand Down
114 changes: 90 additions & 24 deletions test/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1128,18 +1128,25 @@ describe('GitHub', () => {
files: [],
});

req = req.patch('/repos/fake/fake/pulls/123').reply(200, {
number: 123,
title: 'updated-title',
body: 'updated body',
labels: [],
head: {
ref: 'release-please--branches--main--changes--next',
},
base: {
ref: 'main',
},
});
req = req
.get('/repos/fake/fake/pulls/123')
.reply(200, {
number: 123,
state: 'open',
})
.patch('/repos/fake/fake/pulls/123')
.reply(200, {
number: 123,
title: 'updated-title',
body: 'updated body',
labels: [],
head: {
ref: 'release-please--branches--main--changes--next',
},
base: {
ref: 'main',
},
});

const pullRequest: ReleasePullRequest = {
title: PullRequestTitle.ofTargetBranch('main', 'next'),
Expand All @@ -1164,18 +1171,25 @@ describe('GitHub', () => {
.stub(github, <any>'upsertReleaseBranch') // eslint-disable-line @typescript-eslint/no-explicit-any
.resolves('the-pull-request-branch-sha');

req = req.patch('/repos/fake/fake/pulls/123').reply(200, {
number: 123,
title: 'updated-title',
body: 'updated body',
labels: [],
head: {
ref: 'release-please--branches--main',
},
base: {
ref: 'main',
},
});
req = req
.get('/repos/fake/fake/pulls/123')
.reply(200, {
number: 123,
state: 'open',
})
.patch('/repos/fake/fake/pulls/123')
.reply(200, {
number: 123,
title: 'updated-title',
body: 'updated body',
labels: [],
head: {
ref: 'release-please--branches--main',
},
base: {
ref: 'main',
},
});
const getPullRequestStub = sandbox
.stub(github, 'getPullRequest')
.withArgs(123)
Expand Down Expand Up @@ -1209,6 +1223,58 @@ describe('GitHub', () => {
sinon.assert.calledOnce(getPullRequestStub);
req.done();
});

it('skips update if the PR was merged between query and update', async () => {
const upsertReleaseBranch = sandbox
.stub(github, <any>'upsertReleaseBranch') // eslint-disable-line @typescript-eslint/no-explicit-any
.resolves('the-pull-request-branch-sha');

const getPullRequestStub = sandbox
.stub(github, 'getPullRequest')
.withArgs(123)
.resolves({
title: 'release: 1.9.1',
headBranchName: 'release-please--branches--main--changes--next',
baseBranchName: 'main',
number: 123,
body: 'release body',
labels: ['autorelease: tagged'],
files: [],
});

// PR was merged between the initial open-PR query and this update
req = req.get('/repos/fake/fake/pulls/123').reply(200, {
number: 123,
state: 'closed',
merged: true,
});

const pullRequest: ReleasePullRequest = {
title: PullRequestTitle.ofTargetBranch('main', 'next'),
body: new PullRequestBody(mockReleaseData(1000), {
useComponents: true,
}),
labels: ['autorelease: pending'],
headRefName: 'release-please--branches--main--changes--next',
draft: false,
updates: [],
conventionalCommits: [],
};

const result = await github.updatePullRequest(
123,
pullRequest,
'main',
'next'
);

// Should return the current PR state without modifying it
sinon.assert.notCalled(upsertReleaseBranch);
sinon.assert.calledOnce(getPullRequestStub);
expect(result.number).to.equal(123);
expect(result.labels).to.include('autorelease: tagged');
req.done();
});
});

describe('createPullRequest', () => {
Expand Down
Loading