Skip to content

Commit

Permalink
Handle job status and review comments properly
Browse files Browse the repository at this point in the history
- While incidents are less likely to have an exception comment, there are
  cases where a failing aggregate from day before, might impact an
  incident on its own to be scheduled

- We want to accept only passed results without questioning,
  anything else, will need to have an acceptable_for, following the
  discussion in [1]

[1] https://github.com/openSUSE/qem-bot/pull/154/files#r1474042954
  • Loading branch information
foursixnine committed Feb 1, 2024
1 parent 90ef423 commit 62cb3d2
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 7 deletions.
26 changes: 24 additions & 2 deletions openqabot/types/incident.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from openqabot.utils import retry3 as requests

# Make sure we have the right dashboard URL
from .. import QEM_DASHBOARD
from .. import QEM_DASHBOARD, OPENQA_URL
from . import ArchVer, Repos
from ..errors import EmptyChannels, EmptyPackagesError, NoRepoFoundError, NoResultsError
from ..loader.repohash import get_max_revision
Expand Down Expand Up @@ -154,7 +154,7 @@ def has_failures(self, token) -> bool:
QEM_DASHBOARD + "/api/jobs/incident/" + str(self.id), headers=token
).json()

failed_jobs = [res for res in results if res["status"] != "passed"]
failed_jobs = self.filter_failures(results)
if failed_jobs:
log.info("Found %s failed jobs for incident %s:", len(failed_jobs), self.id)
if log.isEnabledFor(LOG_DEBUG_LEVEL):
Expand All @@ -176,3 +176,25 @@ def log_debug_incident(self, job_id):
job_id,
self.id,
)

def filter_failures(self, results):
return [
res
for res in results
if res["status"] not in ("passed")
and not has_ignored_comment(res["job_id"], self.id)
]


@staticmethod
def has_ignored_comment(job_id: int, inc: int):
ret = []
ret = requests.get(OPENQA_URL + "/api/v1/jobs/%s/comments" % job_id).json()
regex = re.compile(r"\@review\:acceptable_for\:incident_%s\:(.+)" % inc)
for comment in ret:
if regex.match(comment["text"]):
# leave comment for future debugging purposes
# log.debug("matched comment incident %s: with comment %s", inc, comment)
return True

return False
47 changes: 42 additions & 5 deletions tests/test_incident.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,23 +131,60 @@ def test_inc_revisions(mock_good):
class MockResponse:
# TODO: collect all instances where the same pattern is used and refactor,
# see openSUSE/qem-bot/issues/161
def __init__(self, json_data):
def __init__(self, url, json_data, extra_data=None):
self.url = url # the url helps us mock different responses
self.json_data = json_data
self.extra_data = extra_data

def mock_comments(self, job=1777, incident=24618):
return [
{
"bugrefs": [],
"created": "2024-01-30 16:04:56 +0000",
"id": job,
"renderedMarkdown": None,
"text": "label:linked Job mentioned in https://progress.opensuse.org/issues/154156",
"text": "@review:acceptable_for:incident_%s:openqa#1337" % incident,
"updated": "2024-01-30 16:04:56 +0000",
"userName": "system",
}
]

def json(self):
if "openqa" in self.url:
self.json_data = []
if "1777" in self.url:
self.json_data = self.mock_comments()
if "qam" in self.url:
pass # right now we don't need to mock anything else for requests to the dashboard
# leave comment for future debugging purposes
# logger.debug("Mocking json: %s", self.json_data)
return self.json_data

def __repr__(self):
return f"<MockResponse for {self.url}>"


def mock_get(url, headers):
def mock_get(url, extra_data=None, headers=None):
return MockResponse(
url=url,
json_data=[
{"status": "passed", "job_id": 1},
{"status": "failed", "job_id": 2},
{"status": "failed", "job_id": 1777}, # Accept the turk
{
"status": "softfailed",

This comment has been minimized.

Copy link
@okurz

okurz Feb 2, 2024

Member

@kraih this looks like it's mocking a response from qem-dashboard that can actually never happen in reality as the dashboard would not return a status "softfailed", am I right?

This comment has been minimized.

Copy link
@okurz

okurz Feb 2, 2024

Member

Let's better move that discussion to #154 (comment) in the pull request, not the commit

"job_id": 2020,
}, # 2020 is the genesys of dark fate
{"status": "failed", "job_id": 2042}, # This one has a dark fate
{"status": "passed", "job_id": 3},
]
],
extra_data=extra_data,
)


logger = logging.getLogger("bot.types.incident")


def test_inc_has_failures(caplog, mock_good, monkeypatch):

monkeypatch.setattr(requests, "get", mock_get)
Expand All @@ -164,7 +201,7 @@ def test_inc_has_failures(caplog, mock_good, monkeypatch):
assert caplog.records[0].message == "Found 1 failed jobs for incident 24618:"
assert (
caplog.records[1].message
== "Job 2 is not marked as acceptable for incident 24618"
== "Job 2042 is not marked as acceptable for incident 24618"
)
assert len(caplog.records) == 2

Expand Down

0 comments on commit 62cb3d2

Please sign in to comment.