From 9318a6658aff99963f089c8128a58cdf22702010 Mon Sep 17 00:00:00 2001 From: Jayashree Huttanagoudar Date: Fri, 3 Oct 2025 10:41:13 +0530 Subject: [PATCH 1/2] Send drop-bug MR notification to ART team on slack --- oar/cli/cmd_drop_bugs.py | 11 +++++ oar/core/notification.py | 94 +++++++++++++++++++++++++------------- oar/core/shipment.py | 16 +++++++ tests/test_notification.py | 66 +++++++++++++++++++++++++- 4 files changed, 155 insertions(+), 32 deletions(-) diff --git a/oar/cli/cmd_drop_bugs.py b/oar/cli/cmd_drop_bugs.py index 6cf666189ae7..240c3064cf91 100644 --- a/oar/cli/cmd_drop_bugs.py +++ b/oar/cli/cmd_drop_bugs.py @@ -40,6 +40,17 @@ def drop_bugs(ctx): logger.info("updating test report") report.update_bug_list(operator.get_jira_issues(), dropped_bugs) nm.share_dropped_bugs(dropped_bugs) + # Notify ART using MR url directly from ShipmentData cache (no extra GitLab lookup) + try: + from oar.core.shipment import ShipmentData + sd = ShipmentData(cs) + mr_url = sd.get_last_drop_bugs_mr_url() + if mr_url: + nm.share_drop_bugs_mr_for_approval(mr_url, dropped_count=len(dropped_bugs)) + else: + logger.warning("drop-bugs MR url is not cached; skipping ART notification") + except Exception: + logger.warning("Failed to obtain MR url for drop-bugs; skipping ART notification") if len(approved_doc_ads): for ad in approved_doc_ads: ad.refresh() diff --git a/oar/core/notification.py b/oar/core/notification.py index d5e9ba5f0dd6..eab412a4d6b8 100644 --- a/oar/core/notification.py +++ b/oar/core/notification.py @@ -31,8 +31,30 @@ def __init__(self, cs: ConfigStore): # self.mc = MailClient( # self.cs.get_email_contact("trt"), self.cs.get_google_app_passwd() # ) - self.sc = SlackClient(self.cs.get_slack_bot_token()) - self.mh = MessageHelper(self.cs) + # Lazy init Slack client and helpers to avoid unnecessary external setup on hot paths + self._sc = None + self._mh = None + # Cache resolved slack channels to avoid repeated config lookups and API calls + self._channel_cache = {} + + @property + def sc(self): + if self._sc is None: + self._sc = SlackClient(self.cs.get_slack_bot_token()) + return self._sc + + @property + def mh(self): + if self._mh is None: + self._mh = MessageHelper(self.cs) + return self._mh + + def _get_channel(self, contact_key: str) -> str: + if contact_key in self._channel_cache: + return self._channel_cache[contact_key] + ch = self.cs.get_slack_channel_from_contact(contact_key) + self._channel_cache[contact_key] = ch + return ch def share_new_report(self, report: TestReport): """ @@ -45,16 +67,9 @@ def share_new_report(self, report: TestReport): NotificationException: error when share this info """ try: - # Send email - # mail_subject = self.cs.release + " z-stream errata test status" - # mail_content = self.mh.get_mail_content_for_new_report(report) - # self.mc.send_email( - # self.cs.get_email_contact("qe"), mail_subject, mail_content - # ) - # Send slack message slack_msg = self.mh.get_slack_message_for_new_report(report) self.sc.post_message( - self.cs.get_slack_channel_from_contact("qe-release"), slack_msg + self._get_channel("qe-release"), slack_msg ) except Exception as e: raise NotificationException("share new report failed") from e @@ -76,19 +91,18 @@ def share_ownership_change_result( """ try: - # Send slack message only slack_msg = self.mh.get_slack_message_for_ownership_change( updated_ads, abnormal_ads, updated_subtasks, new_owner ) self.sc.post_message( - self.cs.get_slack_channel_from_contact("qe-release"), slack_msg + self._get_channel("qe-release"), slack_msg ) if len(abnormal_ads): slack_msg = self.mh.get_slack_message_for_abnormal_advisory( abnormal_ads ) self.sc.post_message( - self.cs.get_slack_channel_from_contact("art"), slack_msg + self._get_channel("art"), slack_msg ) except Exception as e: raise NotificationException( @@ -109,7 +123,7 @@ def share_bugs_to_be_verified(self, jira_issues): jira_issues) if len(slack_msg): self.sc.post_message( - self.cs.get_slack_channel_from_contact( + self._get_channel( "qe-forum"), slack_msg ) except Exception as e: @@ -131,7 +145,7 @@ def share_high_severity_bugs(self, jira_issues): jira_issues) if len(slack_msg): self.sc.post_message( - self.cs.get_slack_channel_from_contact( + self._get_channel( "qe-forum"), slack_msg ) except Exception as e: @@ -153,7 +167,7 @@ def share_new_cve_tracker_bugs(self, cve_tracker_bugs): cve_tracker_bugs) if len(slack_msg): self.sc.post_message( - self.cs.get_slack_channel_from_contact("art"), slack_msg + self._get_channel("art"), slack_msg ) except Exception as e: raise NotificationException( @@ -174,7 +188,7 @@ def share_unhealthy_advisories(self, unhealthy_advisories): unhealthy_advisories) if len(slack_msg): self.sc.post_message( - self.cs.get_slack_channel_from_contact("qe-release"), slack_msg + self._get_channel("qe-release"), slack_msg ) except Exception as e: raise NotificationException( @@ -195,13 +209,12 @@ def share_dropped_bugs(self, dropped_bugs): slack_msg = self.mh.get_slack_message_for_dropped_bugs(dropped_bugs) if len(slack_msg): self.sc.post_message( - self.cs.get_slack_channel_from_contact( + self._get_channel( "qe-release"), slack_msg ) except Exception as e: raise NotificationException( - "share dropped bugs failed" - ) from e + "share dropped bugs failed") from e def share_dropped_and_high_severity_bugs(self, dropped_bugs, high_severity_bugs): @@ -221,13 +234,12 @@ def share_dropped_and_high_severity_bugs(self, dropped_bugs, high_severity_bugs) ) if len(slack_msg): self.sc.post_message( - self.cs.get_slack_channel_from_contact( + self._get_channel( "qe-release"), slack_msg ) except Exception as e: raise NotificationException( - "share dropped and high severity bugs failed" - ) from e + "share dropped and high severity bugs failed") from e def share_doc_prodsec_approval_result(self, doc_appr, prodsec_appr): """ @@ -239,7 +251,7 @@ def share_doc_prodsec_approval_result(self, doc_appr, prodsec_appr): ) if len(slack_msg): self.sc.post_message( - self.cs.get_slack_channel_from_contact( + self._get_channel( "approver"), slack_msg ) except Exception as e: @@ -254,7 +266,7 @@ def share_jenkins_build_url(self, job_name, build_url): slack_msg = self.mh.get_slack_message_for_jenkins_build( job_name, build_url) if len(slack_msg): - self.sc.post_message(self.cs.get_slack_channel_from_contact( + self.sc.post_message(self._get_channel( "qe-release"), slack_msg) except Exception as e: raise NotificationException( @@ -269,7 +281,7 @@ def share_greenwave_cvp_failures(self, jira_key): """ try: slack_msg = self.mh.get_slack_message_for_failed_cvp(jira_key) - self.sc.post_message(self.cs.get_slack_channel_from_contact( + self.sc.post_message(self._get_channel( "qe-release"), slack_msg) except Exception as e: raise NotificationException( @@ -289,7 +301,7 @@ def share_shipment_mr(self, mr, new_owner): try: slack_msg = self.mh.get_slack_message_for_shipment_mr(mr, new_owner) self.sc.post_message( - self.cs.get_slack_channel_from_contact("qe-release"), slack_msg + self._get_channel("qe-release"), slack_msg ) except Exception as e: raise NotificationException("share shipment MR failed") from e @@ -313,18 +325,38 @@ def share_shipment_mr_and_ad_info(self, mr, updated_ads, abnormal_ads, updated_s mr, updated_ads, abnormal_ads, updated_subtasks, new_owner ) self.sc.post_message( - self.cs.get_slack_channel_from_contact("qe-release"), slack_msg + self._get_channel("qe-release"), slack_msg ) if len(abnormal_ads): slack_msg = self.mh.get_slack_message_for_abnormal_advisory( abnormal_ads ) self.sc.post_message( - self.cs.get_slack_channel_from_contact("art"), slack_msg + self._get_channel("art"), slack_msg ) except Exception as e: raise NotificationException("share shipment MR and AD info failed") from e + def share_drop_bugs_mr_for_approval(self, mr_url: str, dropped_count: int = 0): + """ + Notify ART forum channel to approve the drop-bugs merge request + + Args: + mr_url (str): URL of the drop-bugs merge request + dropped_count (int): Number of bugs dropped in this MR (optional) + + Raises: + NotificationException: error when posting message + """ + try: + count_text = f" (dropped {dropped_count} bugs)" if dropped_count else "" + slack_msg = f"ART team, please review and approve the drop-bugs MR: {mr_url}{count_text}" + # Use forum channel mapping from config + channel = self._get_channel("qe-forum") + self.sc.post_message(channel, slack_msg) + except Exception as e: + raise NotificationException("share drop-bugs MR for approval failed") from e + def share_unverified_cve_issues_to_managers(self, unverified_cve_issues): """ Share unverified CVE issues to managers of QA contacts @@ -340,7 +372,7 @@ def share_unverified_cve_issues_to_managers(self, unverified_cve_issues): unverified_cve_issues) if len(slack_msg): self.sc.post_message( - self.cs.get_slack_channel_from_contact( + self._get_channel( "qe-forum"), slack_msg ) except Exception as e: @@ -383,7 +415,7 @@ def share_release_approval_completion(self, release: str, success: bool, error: summary_message = f"Hello {gid}, Release approval timeout for {release}. Payload metadata URL still not accessible" # Always send to default channel - default_channel = self.cs.get_slack_channel_from_contact("qe-release") + default_channel = self._get_channel("qe-release") self.sc.post_message(default_channel, summary_message) logger.info(f"Sent completion notification to default channel {default_channel}") diff --git a/oar/core/shipment.py b/oar/core/shipment.py index b196244d3694..13a86c3ba713 100644 --- a/oar/core/shipment.py +++ b/oar/core/shipment.py @@ -817,6 +817,8 @@ def __init__(self, config_store: ConfigStore): """ self._cs = config_store self._mr = self._initialize_mr() + # cache the last created/used drop-bugs MR url for downstream notifications + self._last_drop_bugs_mr_url: str | None = None def _initialize_mr(self) -> GitLabMergeRequest: """Initialize GitLabMergeRequest objects from shipment MRs @@ -858,6 +860,10 @@ def get_mr(self) -> GitLabMergeRequest: """ return self._mr + def get_last_drop_bugs_mr_url(self) -> str | None: + """Return the last drop-bugs MR URL detected/created during drop_bugs().""" + return self._last_drop_bugs_mr_url + def get_jira_issues(self) -> List[str]: """Get Jira issue IDs from shipment YAML files where source is issues.redhat.com @@ -993,6 +999,11 @@ def drop_bugs(self) -> list[str]: repo_dir = gh.checkout_repo(source_project.http_url_to_repo, mr.get_source_branch()) # configure credential for remote origin, just need to update this branch gh.configure_remotes("origin", f"https://group_143087_bot_e4ed5153eb7e7dfa7eb3d7901a95a6a7:{self._cs.get_gitlab_token()}@gitlab.cee.redhat.com/rioliu/ocp-shipment-data.git") + # cache existing MR url for notification + try: + self._last_drop_bugs_mr_url = mr.get_web_url() + except Exception: + self._last_drop_bugs_mr_url = None else: # if mr does not exist, check out release branch and create new branch based on it repo_dir = gh.checkout_repo(branch=self._mr.get_source_branch()) @@ -1049,6 +1060,11 @@ def drop_bugs(self) -> list[str]: gh.push_changes("ert-release-bot") # create new MR on gitlab server new_mr = gl.create_merge_request("rioliu/ocp-shipment-data", branch, self._mr.get_source_branch(), mr_title, target_project_name="hybrid-platforms/art/ocp-shipment-data", auto_merge=True) + # cache new MR url for notification + try: + self._last_drop_bugs_mr_url = new_mr.get_web_url() + except Exception: + self._last_drop_bugs_mr_url = None self._mr.add_comment(f"Drop bug in MR: {new_mr.get_web_url()}") return unverified_issues diff --git a/tests/test_notification.py b/tests/test_notification.py index af13b07ce83f..76b8096b080d 100644 --- a/tests/test_notification.py +++ b/tests/test_notification.py @@ -14,7 +14,7 @@ class TestNotificationManager(unittest.TestCase): def setUp(self): - self.cs = ConfigStore("4.13.6") + self.cs = ConfigStore("4.19.14") self.nm = NotificationManager(self.cs) @unittest.skip @@ -174,3 +174,67 @@ def test_share_unverified_cve_issues_to_managers_error(self): self.nm.share_unverified_cve_issues_to_managers([test_issue]) self.nm.sc.post_message.assert_called_once() + + def test_share_drop_bugs_mr_for_approval_success(self): + """Ensure MR approval request is posted to forum channel with URL and count""" + mr_url = "https://gitlab.example.com/mygroup/ocp-shipment-data/-/merge_requests/136" + dropped_count = 1 + # Mock channel mapping and Slack posting + self.nm.cs.get_slack_channel_from_contact = unittest.mock.Mock(return_value="#forum-ocp-release") + self.nm.sc.post_message = unittest.mock.Mock() + + self.nm.share_drop_bugs_mr_for_approval(mr_url, dropped_count) + + # Validate channel and message contents + self.nm.sc.post_message.assert_called_once() + args, kwargs = self.nm.sc.post_message.call_args + self.assertEqual(args[0], "#forum-ocp-release") + self.assertIn(mr_url, args[1]) + self.assertIn(str(dropped_count), args[1]) + + def test_share_drop_bugs_mr_for_approval_error(self): + """Ensure NotificationException is raised when Slack post fails""" + mr_url = "https://gitlab.example.com/mygroup/ocp-shipment-data/-/merge_requests/999" + self.nm.cs.get_slack_channel_from_contact = unittest.mock.Mock(return_value="#forum-ocp-release") + self.nm.sc.post_message = unittest.mock.Mock(side_effect=Exception("Slack failure")) + + with self.assertRaises(NotificationException): + self.nm.share_drop_bugs_mr_for_approval(mr_url, dropped_count=0) + + def test_lazy_init_and_channel_cache(self): + """Validate Slack client lazy init and channel caching reduce repeated lookups""" + # Prepare NotificationManager with a fake MessageHelper to avoid external calls + class _FakeMH: + def get_slack_message_for_new_report(self, _): + return "new report msg" + def get_slack_message_for_bug_verification(self, _): + return "verify bugs msg" + + # Ensure lazy state before first use + self.nm._mh = _FakeMH() + self.nm._sc = None + + # Mock channel resolution and Slack post + self.nm.cs.get_slack_channel_from_contact = unittest.mock.Mock(side_effect=lambda k: { + "qe-release": "#qe-release", + "qe-forum": "#forum-ocp-release" + }[k]) + self.nm.sc.post_message = unittest.mock.Mock() + + # First call should initialize Slack client and resolve channel once + self.nm.share_new_report(report=unittest.mock.Mock()) + self.assertIsNotNone(self.nm._sc) + self.nm.cs.get_slack_channel_from_contact.assert_called_once_with("qe-release") + self.nm.sc.post_message.assert_called_once_with("#qe-release", "new report msg") + + # Subsequent call using same contact should use cached channel (no extra get_slack_channel_from_contact) + self.nm.share_new_report(report=unittest.mock.Mock()) + # Still only one resolution for qe-release + self.nm.cs.get_slack_channel_from_contact.assert_called_once() + self.assertEqual(self.nm.sc.post_message.call_count, 2) + + # Now exercise a different contact key and verify each key is cached independently + self.nm.share_bugs_to_be_verified(["OCPBUGS-1"]) # uses qe-forum + # Two unique calls total: one for qe-release and one for qe-forum + self.assertEqual(self.nm.cs.get_slack_channel_from_contact.call_count, 2) + self.nm.sc.post_message.assert_any_call("#forum-ocp-release", "verify bugs msg") From bac158f21dc237014880687f241b34af62671f21 Mon Sep 17 00:00:00 2001 From: Jayashree Huttanagoudar Date: Sat, 11 Oct 2025 16:57:38 +0530 Subject: [PATCH 2/2] Address review comments --- oar/cli/cmd_drop_bugs.py | 21 ++++++++------ oar/core/notification.py | 58 +++++++++++--------------------------- oar/core/shipment.py | 27 ++++-------------- tests/test_notification.py | 46 ++---------------------------- 4 files changed, 38 insertions(+), 114 deletions(-) diff --git a/oar/cli/cmd_drop_bugs.py b/oar/cli/cmd_drop_bugs.py index 240c3064cf91..f72a1417e4bc 100644 --- a/oar/cli/cmd_drop_bugs.py +++ b/oar/cli/cmd_drop_bugs.py @@ -6,6 +6,7 @@ from oar.core.notification import NotificationManager from oar.core.operators import BugOperator from oar.core.worksheet import WorksheetManager +from oar.core.shipment import GitLabServer logger = logging.getLogger(__name__) @@ -40,17 +41,19 @@ def drop_bugs(ctx): logger.info("updating test report") report.update_bug_list(operator.get_jira_issues(), dropped_bugs) nm.share_dropped_bugs(dropped_bugs) - # Notify ART using MR url directly from ShipmentData cache (no extra GitLab lookup) + # Get drop-bugs MR to notify ART team try: - from oar.core.shipment import ShipmentData - sd = ShipmentData(cs) - mr_url = sd.get_last_drop_bugs_mr_url() - if mr_url: - nm.share_drop_bugs_mr_for_approval(mr_url, dropped_count=len(dropped_bugs)) + gl = GitLabServer(cs.get_gitlab_url(), cs.get_gitlab_token()) + mr_title = f"{cs.release} drop bugs" + # Try to scope to shipment project when available + project_name = operator._sd.get_mr().project_name if hasattr(operator, '_sd') and operator._sd.get_mr() else None + mr = gl.get_mr_by_title(mr_title, project_name) + if mr: + nm.share_drop_bugs_mr_for_approval(mr.get_web_url()) else: - logger.warning("drop-bugs MR url is not cached; skipping ART notification") - except Exception: - logger.warning("Failed to obtain MR url for drop-bugs; skipping ART notification") + logger.warning("drop-bugs MR url not found by title; skipping ART notification") + except Exception as e: + logger.warning("Failed to locate MR url for drop-bugs; skipping ART notification: %s", e) if len(approved_doc_ads): for ad in approved_doc_ads: ad.refresh() diff --git a/oar/core/notification.py b/oar/core/notification.py index eab412a4d6b8..afab73face62 100644 --- a/oar/core/notification.py +++ b/oar/core/notification.py @@ -31,30 +31,8 @@ def __init__(self, cs: ConfigStore): # self.mc = MailClient( # self.cs.get_email_contact("trt"), self.cs.get_google_app_passwd() # ) - # Lazy init Slack client and helpers to avoid unnecessary external setup on hot paths - self._sc = None - self._mh = None - # Cache resolved slack channels to avoid repeated config lookups and API calls - self._channel_cache = {} - - @property - def sc(self): - if self._sc is None: - self._sc = SlackClient(self.cs.get_slack_bot_token()) - return self._sc - - @property - def mh(self): - if self._mh is None: - self._mh = MessageHelper(self.cs) - return self._mh - - def _get_channel(self, contact_key: str) -> str: - if contact_key in self._channel_cache: - return self._channel_cache[contact_key] - ch = self.cs.get_slack_channel_from_contact(contact_key) - self._channel_cache[contact_key] = ch - return ch + self.sc = SlackClient(self.cs.get_slack_bot_token()) + self.mh = MessageHelper(self.cs) def share_new_report(self, report: TestReport): """ @@ -69,7 +47,7 @@ def share_new_report(self, report: TestReport): try: slack_msg = self.mh.get_slack_message_for_new_report(report) self.sc.post_message( - self._get_channel("qe-release"), slack_msg + self.cs.get_slack_channel_from_contact("qe-release"), slack_msg ) except Exception as e: raise NotificationException("share new report failed") from e @@ -95,14 +73,14 @@ def share_ownership_change_result( updated_ads, abnormal_ads, updated_subtasks, new_owner ) self.sc.post_message( - self._get_channel("qe-release"), slack_msg + self.cs.get_slack_channel_from_contact("qe-release"), slack_msg ) if len(abnormal_ads): slack_msg = self.mh.get_slack_message_for_abnormal_advisory( abnormal_ads ) self.sc.post_message( - self._get_channel("art"), slack_msg + self.cs.get_slack_channel_from_contact("art"), slack_msg ) except Exception as e: raise NotificationException( @@ -123,7 +101,7 @@ def share_bugs_to_be_verified(self, jira_issues): jira_issues) if len(slack_msg): self.sc.post_message( - self._get_channel( + self.cs.get_slack_channel_from_contact( "qe-forum"), slack_msg ) except Exception as e: @@ -145,7 +123,7 @@ def share_high_severity_bugs(self, jira_issues): jira_issues) if len(slack_msg): self.sc.post_message( - self._get_channel( + self.cs.get_slack_channel_from_contact( "qe-forum"), slack_msg ) except Exception as e: @@ -188,7 +166,7 @@ def share_unhealthy_advisories(self, unhealthy_advisories): unhealthy_advisories) if len(slack_msg): self.sc.post_message( - self._get_channel("qe-release"), slack_msg + self.cs.get_slack_channel_from_contact("qe-release"), slack_msg ) except Exception as e: raise NotificationException( @@ -266,7 +244,7 @@ def share_jenkins_build_url(self, job_name, build_url): slack_msg = self.mh.get_slack_message_for_jenkins_build( job_name, build_url) if len(slack_msg): - self.sc.post_message(self._get_channel( + self.sc.post_message(self.cs.get_slack_channel_from_contact( "qe-release"), slack_msg) except Exception as e: raise NotificationException( @@ -281,7 +259,7 @@ def share_greenwave_cvp_failures(self, jira_key): """ try: slack_msg = self.mh.get_slack_message_for_failed_cvp(jira_key) - self.sc.post_message(self._get_channel( + self.sc.post_message(self.cs.get_slack_channel_from_contact( "qe-release"), slack_msg) except Exception as e: raise NotificationException( @@ -301,7 +279,7 @@ def share_shipment_mr(self, mr, new_owner): try: slack_msg = self.mh.get_slack_message_for_shipment_mr(mr, new_owner) self.sc.post_message( - self._get_channel("qe-release"), slack_msg + self.cs.get_slack_channel_from_contact("qe-release"), slack_msg ) except Exception as e: raise NotificationException("share shipment MR failed") from e @@ -325,34 +303,32 @@ def share_shipment_mr_and_ad_info(self, mr, updated_ads, abnormal_ads, updated_s mr, updated_ads, abnormal_ads, updated_subtasks, new_owner ) self.sc.post_message( - self._get_channel("qe-release"), slack_msg + self.cs.get_slack_channel_from_contact("qe-release"), slack_msg ) if len(abnormal_ads): slack_msg = self.mh.get_slack_message_for_abnormal_advisory( abnormal_ads ) self.sc.post_message( - self._get_channel("art"), slack_msg + self.cs.get_slack_channel_from_contact("art"), slack_msg ) except Exception as e: raise NotificationException("share shipment MR and AD info failed") from e - def share_drop_bugs_mr_for_approval(self, mr_url: str, dropped_count: int = 0): + def share_drop_bugs_mr_for_approval(self, mr_url: str): """ Notify ART forum channel to approve the drop-bugs merge request Args: mr_url (str): URL of the drop-bugs merge request - dropped_count (int): Number of bugs dropped in this MR (optional) Raises: NotificationException: error when posting message """ try: - count_text = f" (dropped {dropped_count} bugs)" if dropped_count else "" - slack_msg = f"ART team, please review and approve the drop-bugs MR: {mr_url}{count_text}" + slack_msg = f"ART team, please review and approve the drop-bugs MR: {mr_url}" # Use forum channel mapping from config - channel = self._get_channel("qe-forum") + channel = self.cs.get_slack_channel_from_contact("qe-forum") self.sc.post_message(channel, slack_msg) except Exception as e: raise NotificationException("share drop-bugs MR for approval failed") from e @@ -415,7 +391,7 @@ def share_release_approval_completion(self, release: str, success: bool, error: summary_message = f"Hello {gid}, Release approval timeout for {release}. Payload metadata URL still not accessible" # Always send to default channel - default_channel = self._get_channel("qe-release") + default_channel = self.cs.get_slack_channel_from_contact("qe-release") self.sc.post_message(default_channel, summary_message) logger.info(f"Sent completion notification to default channel {default_channel}") diff --git a/oar/core/shipment.py b/oar/core/shipment.py index 13a86c3ba713..b45d633e22d0 100644 --- a/oar/core/shipment.py +++ b/oar/core/shipment.py @@ -817,8 +817,6 @@ def __init__(self, config_store: ConfigStore): """ self._cs = config_store self._mr = self._initialize_mr() - # cache the last created/used drop-bugs MR url for downstream notifications - self._last_drop_bugs_mr_url: str | None = None def _initialize_mr(self) -> GitLabMergeRequest: """Initialize GitLabMergeRequest objects from shipment MRs @@ -860,10 +858,6 @@ def get_mr(self) -> GitLabMergeRequest: """ return self._mr - def get_last_drop_bugs_mr_url(self) -> str | None: - """Return the last drop-bugs MR URL detected/created during drop_bugs().""" - return self._last_drop_bugs_mr_url - def get_jira_issues(self) -> List[str]: """Get Jira issue IDs from shipment YAML files where source is issues.redhat.com @@ -986,24 +980,20 @@ def drop_bugs(self) -> list[str]: # need to check if MR with above title exists or not gl = GitLabServer(self._cs.get_gitlab_url(), self._cs.get_gitlab_token()) gh = GitHelper() - mr = gl.get_mr_by_title(mr_title, self._mr.project_name) + # Look up an existing drop-bugs MR by title in the same project as the shipment MR + drop_bugs_mr = gl.get_mr_by_title(mr_title, self._mr.project_name) # if mr already exists, don't need to create new mr timestamp = datetime.now().strftime("%Y%m%d%H%M%S") repo_dir = "" branch = f"drop-bugs-for-{self._cs.release}-{timestamp}" - if mr: + if drop_bugs_mr: # get source project info from existing mr metadata # check out the branch from source project - source_project = mr.gl.projects.get(mr.mr.source_project_id) - repo_dir = gh.checkout_repo(source_project.http_url_to_repo, mr.get_source_branch()) + source_project = drop_bugs_mr.gl.projects.get(drop_bugs_mr.mr.source_project_id) + repo_dir = gh.checkout_repo(source_project.http_url_to_repo, drop_bugs_mr.get_source_branch()) # configure credential for remote origin, just need to update this branch gh.configure_remotes("origin", f"https://group_143087_bot_e4ed5153eb7e7dfa7eb3d7901a95a6a7:{self._cs.get_gitlab_token()}@gitlab.cee.redhat.com/rioliu/ocp-shipment-data.git") - # cache existing MR url for notification - try: - self._last_drop_bugs_mr_url = mr.get_web_url() - except Exception: - self._last_drop_bugs_mr_url = None else: # if mr does not exist, check out release branch and create new branch based on it repo_dir = gh.checkout_repo(branch=self._mr.get_source_branch()) @@ -1053,18 +1043,13 @@ def drop_bugs(self) -> list[str]: return [] - if mr: + if drop_bugs_mr: gh.push_changes() else: # push the local change to forked repo gh.push_changes("ert-release-bot") # create new MR on gitlab server new_mr = gl.create_merge_request("rioliu/ocp-shipment-data", branch, self._mr.get_source_branch(), mr_title, target_project_name="hybrid-platforms/art/ocp-shipment-data", auto_merge=True) - # cache new MR url for notification - try: - self._last_drop_bugs_mr_url = new_mr.get_web_url() - except Exception: - self._last_drop_bugs_mr_url = None self._mr.add_comment(f"Drop bug in MR: {new_mr.get_web_url()}") return unverified_issues diff --git a/tests/test_notification.py b/tests/test_notification.py index 76b8096b080d..b7ad3ea5a4e7 100644 --- a/tests/test_notification.py +++ b/tests/test_notification.py @@ -176,21 +176,19 @@ def test_share_unverified_cve_issues_to_managers_error(self): self.nm.sc.post_message.assert_called_once() def test_share_drop_bugs_mr_for_approval_success(self): - """Ensure MR approval request is posted to forum channel with URL and count""" + """Ensure MR approval request is posted to forum channel with URL""" mr_url = "https://gitlab.example.com/mygroup/ocp-shipment-data/-/merge_requests/136" - dropped_count = 1 # Mock channel mapping and Slack posting self.nm.cs.get_slack_channel_from_contact = unittest.mock.Mock(return_value="#forum-ocp-release") self.nm.sc.post_message = unittest.mock.Mock() - self.nm.share_drop_bugs_mr_for_approval(mr_url, dropped_count) + self.nm.share_drop_bugs_mr_for_approval(mr_url) # Validate channel and message contents self.nm.sc.post_message.assert_called_once() args, kwargs = self.nm.sc.post_message.call_args self.assertEqual(args[0], "#forum-ocp-release") self.assertIn(mr_url, args[1]) - self.assertIn(str(dropped_count), args[1]) def test_share_drop_bugs_mr_for_approval_error(self): """Ensure NotificationException is raised when Slack post fails""" @@ -199,42 +197,4 @@ def test_share_drop_bugs_mr_for_approval_error(self): self.nm.sc.post_message = unittest.mock.Mock(side_effect=Exception("Slack failure")) with self.assertRaises(NotificationException): - self.nm.share_drop_bugs_mr_for_approval(mr_url, dropped_count=0) - - def test_lazy_init_and_channel_cache(self): - """Validate Slack client lazy init and channel caching reduce repeated lookups""" - # Prepare NotificationManager with a fake MessageHelper to avoid external calls - class _FakeMH: - def get_slack_message_for_new_report(self, _): - return "new report msg" - def get_slack_message_for_bug_verification(self, _): - return "verify bugs msg" - - # Ensure lazy state before first use - self.nm._mh = _FakeMH() - self.nm._sc = None - - # Mock channel resolution and Slack post - self.nm.cs.get_slack_channel_from_contact = unittest.mock.Mock(side_effect=lambda k: { - "qe-release": "#qe-release", - "qe-forum": "#forum-ocp-release" - }[k]) - self.nm.sc.post_message = unittest.mock.Mock() - - # First call should initialize Slack client and resolve channel once - self.nm.share_new_report(report=unittest.mock.Mock()) - self.assertIsNotNone(self.nm._sc) - self.nm.cs.get_slack_channel_from_contact.assert_called_once_with("qe-release") - self.nm.sc.post_message.assert_called_once_with("#qe-release", "new report msg") - - # Subsequent call using same contact should use cached channel (no extra get_slack_channel_from_contact) - self.nm.share_new_report(report=unittest.mock.Mock()) - # Still only one resolution for qe-release - self.nm.cs.get_slack_channel_from_contact.assert_called_once() - self.assertEqual(self.nm.sc.post_message.call_count, 2) - - # Now exercise a different contact key and verify each key is cached independently - self.nm.share_bugs_to_be_verified(["OCPBUGS-1"]) # uses qe-forum - # Two unique calls total: one for qe-release and one for qe-forum - self.assertEqual(self.nm.cs.get_slack_channel_from_contact.call_count, 2) - self.nm.sc.post_message.assert_any_call("#forum-ocp-release", "verify bugs msg") + self.nm.share_drop_bugs_mr_for_approval(mr_url)