Skip to content

Commit d2bafd8

Browse files
committed
feat: auto-approve staging PRs on ok openQA tests
Motivation: To align the staging PR submission workflows with the existing package maintenance process in qem-bot (sub-approve), gitea-trigger should automatically approve a PR when all its triggered openQA tests are ok. Design Choices: - Extended PullRequest to include commit_sha, needed for review_pr - Centralized approval message and URL parsing in loader/gitea.py - Updated gitea-trigger to call the shared approve_pr when jobs pass Benefits: - Provides automatic approval for staging PRs when openQA tests are ok - Reduces code duplication between the approver and giteatrigger modules Related issue: https://progress.opensuse.org/issues/197906
1 parent 4c17832 commit d2bafd8

File tree

9 files changed

+60
-24
lines changed

9 files changed

+60
-24
lines changed

openqabot/approver.py

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
from logging import getLogger
1414
from typing import TYPE_CHECKING
1515
from urllib.error import HTTPError
16-
from urllib.parse import urlparse
1716

1817
import osc.conf
1918
import osc.core
@@ -25,7 +24,7 @@
2524
from openqabot.openqa import OpenQAInterface
2625

2726
from .commenter import Commenter
28-
from .loader.gitea import make_token_header, review_pr
27+
from .loader.gitea import approve_pr, make_token_header
2928
from .loader.qem import (
3029
JobAggr,
3130
SubReq,
@@ -441,19 +440,7 @@ def osc_approve(sub: SubReq, msg: str) -> bool:
441440

442441
def git_approve(self, sub: SubReq, msg: str) -> bool:
443442
"""Approve a submission in Gitea."""
444-
if not sub.url:
445-
log.error("Gitea API error: PR %s has no URL", sub.sub)
443+
if not sub.submission or not getattr(sub.submission, "project", None):
444+
log.error("Gitea API error: PR %s has no project (repo_name)", sub.sub)
446445
return False
447-
try:
448-
path_parts = urlparse(sub.url).path.split("/")
449-
review_pr(
450-
self.gitea_token,
451-
"/".join(path_parts[-4:-2]),
452-
sub.sub,
453-
msg,
454-
sub.scm_info or "",
455-
)
456-
except Exception:
457-
log.exception("Gitea API error: Failed to approve PR %s", sub.sub)
458-
return False
459-
return True
446+
return approve_pr(self.gitea_token, sub.submission.project, sub.sub, sub.scm_info or "", msg)

openqabot/giteatrigger.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
from .commenter import Commenter
1818
from .loader.gitea import (
19+
approve_pr,
1920
generate_repo_url,
2021
get_gitea_staging_config,
2122
get_open_prs,
@@ -136,6 +137,15 @@ def comment_on_pr(self, pullrequest: PullRequest, product: str, version: str, ar
136137

137138
if res := self.commenter.generate_comment(pullrequest, jobs):
138139
self.commenter.gitea_comment(pullrequest, *res)
140+
if res[1] == "passed":
141+
if self.dry:
142+
log.info("Dry run: Would approve PR %s", pullrequest.number)
143+
else:
144+
msg = (
145+
f"Request accepted for '{config.settings.obs_group}' "
146+
f"based on data in {config.settings.qem_dashboard_url}"
147+
)
148+
approve_pr(self.gitea_token, pullrequest.repo_name, pullrequest.number, pullrequest.commit_sha, msg)
139149

140150
def get_prs_by_label(self) -> None:
141151
"""Get all open PRs and filter them by defined label."""
@@ -160,6 +170,7 @@ def get_prs_by_label(self) -> None:
160170
repo_name=pr["base"]["repo"]["name"],
161171
branch=pr["base"]["label"],
162172
url=pr["html_url"],
173+
commit_sha=pr["head"]["sha"],
163174
)
164175
# we looking only for PRs which has ALL labels defined via '--pr-label' parameter AND
165176
# at least one for labels defined in staging.config

openqabot/loader/gitea.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,16 @@ def review_pr( # noqa: PLR0913
246246
post_json(review_url, token, review_data)
247247

248248

249+
def approve_pr(token: dict[str, str], repo_name: str, pr_number: int, commit_id: str, msg: str) -> bool:
250+
"""Approve a PR on Gitea using its repository name and commit ID."""
251+
try:
252+
review_pr(token, repo_name, pr_number, msg, commit_id, approve=True)
253+
except Exception:
254+
log.exception("Gitea API error: Failed to approve PR %s", pr_number)
255+
return False
256+
return True
257+
258+
249259
def get_name(review: dict[str, Any], of: str, via: str) -> str:
250260
"""Extract a name from a Gitea review entity."""
251261
entity = review.get(of)

openqabot/types/pullrequest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ class PullRequest:
2828
repo_name: str
2929
branch: str
3030
url: str
31+
commit_sha: str
3132
raw_labels: list[dict[str, Any]] = field(repr=False)
3233

3334
labels: set[str] = field(init=False)

tests/helpers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ def f_sub_approver(*_args: Any) -> list[SubReq]:
9393
"type": "git",
9494
"url": "https://src.suse.de/products/SLFO/pulls/124",
9595
"scm_info": "...",
96-
"project": "SLFO",
96+
"project": "products/SLFO",
9797
"inReview": True,
9898
"isActive": True,
9999
"approved": False,

tests/test_approve_git.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
import pytest
1717

1818

19-
def test_git_approve_no_url(caplog: pytest.LogCaptureFixture) -> None:
19+
def test_git_approve_no_project(caplog: pytest.LogCaptureFixture) -> None:
2020
approver_instance = Approver(args)
21-
sub = SubReq(sub=1, req=100, type="git", url=None)
21+
sub = SubReq(sub=1, req=100, type="git", url=None, submission=None)
2222
caplog.set_level(logging.ERROR)
2323
assert not approver_instance.git_approve(sub, "msg")
24-
assert "Gitea API error: PR 1 has no URL" in caplog.text
24+
assert "Gitea API error: PR 1 has no project (repo_name)" in caplog.text

tests/test_approve_obs.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ def test_osc_all_pass(caplog: pytest.LogCaptureFixture, mocker: MockerFixture) -
122122

123123
mocker.patch("openqabot.approver.dashboard.get_json", return_value=[{"job_id": 100000, "status": "passed"}])
124124
mocker.patch("osc.core.change_review_state")
125-
mock_review_pr = mocker.patch("openqabot.approver.review_pr")
125+
mock_review_pr = mocker.patch("openqabot.approver.approve_pr")
126126

127127
assert Approver(args)() == 0
128128
expected = [

tests/test_giteatrigger.py

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
# SPDX-License-Identifier: MIT
33
"""Tests GiteaTrigger class."""
44

5+
import logging
56
from argparse import Namespace
67
from typing import cast
78
from unittest.mock import MagicMock
@@ -114,18 +115,21 @@ def test_get_prs_by_label_filtering(trigger: GiteaTrigger, mocker: MockerFixture
114115
"labels": [{"name": "needs-testing"}, {"name": "qalabel1"}],
115116
"base": {"repo": {"name": "r"}, "label": "l"},
116117
"html_url": "u",
118+
"head": {"sha": "xyz"},
117119
},
118120
{
119121
"number": 2,
120122
"labels": [{"name": "wrong-label"}, {"name": "qalabel1"}],
121123
"base": {"repo": {"name": "r"}, "label": "l"},
122124
"html_url": "u",
125+
"head": {"sha": "xyz"},
123126
},
124127
{
125128
"number": 3,
126129
"labels": [{"name": "needs-testing"}],
127130
"base": {"repo": {"name": "r"}, "label": "l"},
128131
"html_url": "u",
132+
"head": {"sha": "xyz"},
129133
},
130134
],
131135
)
@@ -203,6 +207,7 @@ def test_get_prs_by_label_specific_number(mock_args: Namespace, mocker: MockerFi
203207
"labels": [{"name": "needs-testing"}, {"name": "qalabel1"}],
204208
"base": {"repo": {"name": "r"}, "label": "l"},
205209
"html_url": "u",
210+
"head": {"sha": "xyz"},
206211
}
207212
],
208213
)
@@ -237,17 +242,37 @@ def test_check_pullrequest_comments_when_no_trigger_needed(trigger: GiteaTrigger
237242
mock_comment_on_pr.assert_called_once()
238243

239244

240-
def test_comment_on_pr_build_injection(trigger: GiteaTrigger) -> None:
245+
def test_comment_on_pr_build_injection(trigger: GiteaTrigger, mocker: MockerFixture) -> None:
241246
"""Tests that the build string is injected into raw openQA jobs."""
247+
mock_approve_pr = mocker.patch("openqabot.giteatrigger.approve_pr")
242248
mock_jobs = [{"id": 1, "state": "done", "result": "passed"}]
243249
cast("MagicMock", trigger.openqa.get_jobs).return_value = mock_jobs
244250
cast("MagicMock", trigger.commenter.generate_comment).return_value = ("Summary", "passed")
245251

246-
mock_pr = MagicMock(number=123, url="http://fake.url/123")
252+
mock_pr = MagicMock(number=123, url="http://fake.url/123", commit_sha="sha123", repo_name="fake_repo")
247253
trigger.comment_on_pr(mock_pr, "product", "version", "arch", "PR-BUILD")
248254

249255
assert mock_jobs[0]["build"] == "PR-BUILD"
250256
cast("MagicMock", trigger.commenter.generate_comment).assert_called_once_with(mock_pr, mock_jobs)
257+
mock_approve_pr.assert_called_once_with(trigger.gitea_token, "fake_repo", 123, "sha123", mocker.ANY)
258+
259+
260+
def test_comment_on_pr_dry_run_approves(
261+
trigger: GiteaTrigger, mocker: MockerFixture, caplog: pytest.LogCaptureFixture
262+
) -> None:
263+
"""Tests that approve_pr is not called during a dry run."""
264+
caplog.set_level(logging.INFO)
265+
trigger.dry = True
266+
mock_approve_pr = mocker.patch("openqabot.giteatrigger.approve_pr")
267+
mock_jobs = [{"id": 1, "state": "done", "result": "passed"}]
268+
cast("MagicMock", trigger.openqa.get_jobs).return_value = mock_jobs
269+
cast("MagicMock", trigger.commenter.generate_comment).return_value = ("Summary", "passed")
270+
271+
mock_pr = MagicMock(number=123, url="http://fake.url/123", commit_sha="sha123", repo_name="fake_repo")
272+
trigger.comment_on_pr(mock_pr, "product", "version", "arch", "PR-BUILD")
273+
274+
mock_approve_pr.assert_not_called()
275+
assert "Dry run: Would approve PR 123" in caplog.text
251276

252277

253278
def test_comment_on_pr_no_jobs(trigger: GiteaTrigger) -> None:

tests/test_pullrequest.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ def test_pull_request_has_labels() -> None:
4141
repo_name="os-autoinst",
4242
branch="master",
4343
url="http://gitea/pull/124",
44+
commit_sha="abcd123",
4445
raw_labels=raw_data,
4546
)
4647

@@ -57,6 +58,7 @@ def test_pull_request_id_property() -> None:
5758
repo_name="os-autoinst",
5859
branch="master",
5960
url="http://gitea/pull/124",
61+
commit_sha="abcd123",
6062
raw_labels=[],
6163
)
6264
assert pr.id == 124

0 commit comments

Comments
 (0)