From dd958dc0a2a1ed985a5d0a5e7cab6d8ae4863425 Mon Sep 17 00:00:00 2001 From: Santiago Zarate Date: Wed, 31 Jan 2024 22:34:36 +0100 Subject: [PATCH] Handle job status and review comments properly - 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 softfailed and passed results without questioning, anything else, will need to have an acceptable_for --- openqabot/types/incident.py | 26 ++++++++++++++++++-- tests/test_incident.py | 47 +++++++++++++++++++++++++++++++++---- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/openqabot/types/incident.py b/openqabot/types/incident.py index 1272372..67c3534 100644 --- a/openqabot/types/incident.py +++ b/openqabot/types/incident.py @@ -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 @@ -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): @@ -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", "softfailed") + 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 diff --git a/tests/test_incident.py b/tests/test_incident.py index 86e103f..786d84d 100644 --- a/tests/test_incident.py +++ b/tests/test_incident.py @@ -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"" + -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", + "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) @@ -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