Skip to content

Commit 22f72d7

Browse files
committed
Comment on the source PR when a backport fails
Restore the pre-`--from-pr` behavior of reporting backport failures on the merged PR itself. When one or more target branches fail, port-commit now posts a best-effort comment listing each failed base and a manual retry hint. A comment failure is warned, never raised, so it can't mask the backport result.
1 parent b383598 commit 22f72d7

3 files changed

Lines changed: 118 additions & 0 deletions

File tree

ddev/src/ddev/cli/release/port_commit_workflow.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1011,6 +1011,8 @@ def run_backport_from_pr(
10111011
options=options,
10121012
)
10131013
_display_backport_summary(app, pr_number, results)
1014+
if not options.dry_run:
1015+
_comment_on_backport_failures(app, pr_number, results)
10141016
return all(result.status is not BackportStatus.FAILED for result in results)
10151017

10161018

@@ -1106,3 +1108,48 @@ def _display_backport_summary(app: Application, pr_number: int, results: list[Ba
11061108
),
11071109
stderr=True,
11081110
)
1111+
1112+
1113+
def _comment_on_backport_failures(app: Application, pr_number: int, results: list[BackportResult]) -> None:
1114+
"""Post a comment on the source PR when one or more bases failed to backport.
1115+
1116+
The pre-`--from-pr` automation commented on the merged PR when a backport failed; without this a
1117+
failure is only visible as a red workflow run, never on the PR the developer merged. Posting is
1118+
best-effort: a comment failure must not mask the backport failure it is reporting, so any error
1119+
here is warned, not raised.
1120+
"""
1121+
failures = [result for result in results if result.status is BackportStatus.FAILED]
1122+
if not failures:
1123+
return
1124+
token = app.config.github.token
1125+
if not token:
1126+
app.display_warning(f'No GitHub token configured; skipping backport-failure comment on PR #{pr_number}.')
1127+
return
1128+
1129+
owner, repo = resolve_owner_repo(app)
1130+
lines = [f'⚠️ Automatic backport of this PR failed for {len(failures)} target branch(es):', '']
1131+
for result in failures:
1132+
reason = f': {result.detail}' if result.detail else ''
1133+
lines.append(
1134+
f'- `{result.base}`{reason} — retry manually with '
1135+
f'`ddev release port-commit PR-{pr_number} --target-branch {result.base}`.'
1136+
)
1137+
body = '\n'.join(lines)
1138+
1139+
try:
1140+
_post_issue_comment(token, owner, repo, pr_number, body)
1141+
except Exception as e: # noqa: BLE001 - best-effort comment; never mask the backport result
1142+
app.display_warning(f'Could not post backport-failure comment on PR #{pr_number}: {e}')
1143+
1144+
1145+
def _post_issue_comment(token: str, owner: str, repo: str, issue_number: int, body: str) -> None:
1146+
"""Create a comment on the given issue or pull request."""
1147+
import asyncio
1148+
1149+
from ddev.utils.github_async import async_github_client
1150+
1151+
async def _post() -> None:
1152+
async with async_github_client(token=token) as client:
1153+
await client.create_issue_comment(owner=owner, repo=repo, issue_number=issue_number, body=body)
1154+
1155+
asyncio.run(_post())

ddev/tests/cli/release/test_port_commit.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,6 +1159,55 @@ def test_command_from_pr_aggregates_failures_and_continues(
11591159
assert bases == ['7.62.x', '7.61.x']
11601160

11611161

1162+
def test_command_from_pr_comments_on_source_pr_when_a_base_fails(
1163+
ddev: CliRunner, mocker: MockerFixture, fake_async_github: FakeAsyncGitHubClient
1164+
) -> None:
1165+
"""A failed base posts a comment on the merged PR so the failure is visible off the workflow run."""
1166+
import httpx
1167+
1168+
_setup_command_mocks(mocker, commit_sha=FULL_SHA_FOR_TESTS)
1169+
fake_async_github.mock_response(
1170+
'get_pull_request',
1171+
_merged_pr(number=23703, backport_bases=['7.62.x', '7.61.x']),
1172+
)
1173+
fake_async_github.mock_response(
1174+
'create_pull_request',
1175+
httpx.HTTPStatusError('boom', request=httpx.Request('POST', 'https://x'), response=httpx.Response(500)),
1176+
base='7.62.x',
1177+
)
1178+
mocker.patch.dict('os.environ', {'DD_GITHUB_USER': 'alice'})
1179+
1180+
result = ddev('release', 'port-commit', '--from-pr', '23703')
1181+
1182+
assert result.exit_code == 1, result.output
1183+
comment_calls = fake_async_github.calls_to('create_issue_comment')
1184+
assert len(comment_calls) == 1
1185+
comment_call = comment_calls[0]
1186+
assert comment_call.kwargs['issue_number'] == 23703
1187+
body = comment_call.kwargs['body']
1188+
assert '7.62.x' in body
1189+
assert 'retry manually' in body
1190+
# Only the failed base is named; the base that ported cleanly is not mentioned.
1191+
assert '7.61.x' not in body
1192+
1193+
1194+
def test_command_from_pr_does_not_comment_when_all_bases_succeed(
1195+
ddev: CliRunner, mocker: MockerFixture, fake_async_github: FakeAsyncGitHubClient
1196+
) -> None:
1197+
"""The happy path leaves no comment on the source PR."""
1198+
_setup_command_mocks(mocker, commit_sha=FULL_SHA_FOR_TESTS)
1199+
fake_async_github.mock_response(
1200+
'get_pull_request',
1201+
_merged_pr(number=23703, backport_bases=['7.62.x', '7.61.x']),
1202+
)
1203+
mocker.patch.dict('os.environ', {'DD_GITHUB_USER': 'alice'})
1204+
1205+
result = ddev('release', 'port-commit', '--from-pr', '23703')
1206+
1207+
assert result.exit_code == 0, result.output
1208+
fake_async_github.assert_not_called('create_issue_comment')
1209+
1210+
11621211
def test_command_from_pr_summary_reports_every_status(
11631212
ddev: CliRunner, mocker: MockerFixture, fake_async_github: FakeAsyncGitHubClient
11641213
) -> None:

ddev/tests/helpers/github_async.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ def test_thing(fake_async_github):
4646
from ddev.utils.github_async.models import (
4747
ArtifactsList,
4848
CheckRun,
49+
IssueComment,
4950
Label,
5051
PullRequest,
5152
WorkflowDispatchResult,
@@ -84,6 +85,10 @@ def _default_response_factories() -> dict[str, Callable[[], Any]]:
8485
headers={},
8586
),
8687
'add_labels_to_issue': lambda: GitHubResponse.model_validate({'data': [], 'headers': {}}),
88+
'create_issue_comment': lambda: GitHubResponse(
89+
data=IssueComment(id=1, body='', html_url='https://github.com/test/repo/issues/1#issuecomment-1'),
90+
headers={},
91+
),
8792
# Default to "PR not found" so tests that don't care about PR lookup auto-fall-through
8893
# to commit resolution. Tests that need a specific PR register their own mock_response.
8994
'get_pull_request': lambda: httpx.HTTPStatusError(
@@ -284,6 +289,23 @@ async def add_labels_to_issue(
284289
timeout=timeout,
285290
)
286291

292+
async def create_issue_comment(
293+
self,
294+
owner: str,
295+
repo: str,
296+
issue_number: int,
297+
body: str,
298+
timeout: float | None = None,
299+
) -> GitHubResponse[IssueComment]:
300+
return self._call(
301+
'create_issue_comment',
302+
owner=owner,
303+
repo=repo,
304+
issue_number=issue_number,
305+
body=body,
306+
timeout=timeout,
307+
)
308+
287309
async def create_workflow_dispatch(
288310
self,
289311
owner: str,

0 commit comments

Comments
 (0)