From cf3108190e64b2abaa6bf4612d21ef3f16b6b6cc Mon Sep 17 00:00:00 2001
From: Maja Massarini <mmassari@redhat.com>
Date: Fri, 29 Nov 2024 11:50:47 +0100
Subject: [PATCH] Add failing builds to override targets when retriggering
 tests

Otherwise there is no override and everything is retriggered.
---
 packit_service/worker/events/comment.py       | 11 ++++++++-
 .../worker/helpers/build/build_helper.py      |  9 ++++---
 tests/integration/test_pr_comment.py          | 24 +++++++++++++++----
 3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/packit_service/worker/events/comment.py b/packit_service/worker/events/comment.py
index a7400f952..aa03c5581 100644
--- a/packit_service/worker/events/comment.py
+++ b/packit_service/worker/events/comment.py
@@ -94,7 +94,16 @@ def comment_object(self) -> Optional[Comment]:
 
     @property
     def build_targets_override(self) -> Optional[set[tuple[str, str]]]:
-        if not self._build_targets_override and "rebuild-failed" in self.comment:
+        # If we do not override the failing builds for the retest-failed comment
+        # we will later submit all tests.
+        # Overriding builds for the retest-failed comment will let the test jobs
+        # see that something has failed and only for those targets the
+        # tests will be submitted.
+        if (
+            not self._build_targets_override
+            and "rebuild-failed" in self.comment
+            or "retest-failed" in self.comment
+        ):
             self._build_targets_override = (
                 super().get_all_build_targets_by_status(
                     statuses_to_filter_with=[BuildStatus.failure],
diff --git a/packit_service/worker/helpers/build/build_helper.py b/packit_service/worker/helpers/build/build_helper.py
index 7dd9ccb39..062bf8310 100644
--- a/packit_service/worker/helpers/build/build_helper.py
+++ b/packit_service/worker/helpers/build/build_helper.py
@@ -10,13 +10,13 @@
 
 from kubernetes.client.rest import ApiException
 from ogr.abstract import GitProject
+from sandcastle import SandcastleTimeoutReached
+
 from packit.config import JobConfig, JobConfigTriggerType, JobType
 from packit.config.aliases import DEFAULT_VERSION
 from packit.config.package_config import PackageConfig
 from packit.exceptions import PackitMergeException
 from packit.utils import PackitFormatter
-from sandcastle import SandcastleTimeoutReached
-
 from packit_service import sentry_integration
 from packit_service.config import ServiceConfig
 from packit_service.constants import FAILURE_COMMENT_MESSAGE_VARIABLES
@@ -325,7 +325,7 @@ def tests_targets_for_test_job(self, test_job_config: JobConfig) -> set[str]:
         if self.build_targets_override:
             logger.debug(f"Build targets override: {self.build_targets_override}")
             for target, identifier in self.build_targets_override:
-                if identifier == (test_job_config.identifier):
+                if identifier == test_job_config.identifier:
                     targets_override.update(
                         self.build_target2test_targets_for_test_job(target, test_job_config),
                     )
@@ -336,8 +336,7 @@ def tests_targets_for_test_job(self, test_job_config: JobConfig) -> set[str]:
                 [
                     target
                     for target, identifier in self.tests_targets_override
-                    if identifier
-                    == (test_job_config.identifier)
+                    if identifier == test_job_config.identifier
                 ]
             )
 
diff --git a/tests/integration/test_pr_comment.py b/tests/integration/test_pr_comment.py
index 5c4cb4655..1bf833aed 100644
--- a/tests/integration/test_pr_comment.py
+++ b/tests/integration/test_pr_comment.py
@@ -12,6 +12,9 @@
 from ogr.services.github import GithubProject, GithubService
 from ogr.services.pagure import PagureProject
 from ogr.utils import RequestResponse
+
+import packit_service.models
+import packit_service.service.urls as urls
 from packit.api import PackitAPI
 from packit.config import (
     JobConfigTriggerType,
@@ -22,9 +25,6 @@
 from packit.local_project import LocalProject, LocalProjectBuilder
 from packit.upstream import GitUpstream
 from packit.utils.koji_helper import KojiHelper
-
-import packit_service.models
-import packit_service.service.urls as urls
 from packit_service.config import ServiceConfig
 from packit_service.constants import (
     COMMENT_REACTION,
@@ -1787,21 +1787,37 @@ def test_retest_failed(
     )
     flexmock(GithubProject).should_receive("is_private").and_return(False)
     flexmock(celery_group).should_receive("apply_async").once()
-    flexmock(CoprHelper).should_receive("get_valid_build_targets").times(3).and_return(
+    flexmock(CoprHelper).should_receive("get_valid_build_targets").times(4).and_return(
         {"test-target"},
     )
     flexmock(TestingFarmJobHelper).should_receive("get_latest_copr_build").and_return(
         flexmock(status=BuildStatus.success),
     )
 
+    build_model = flexmock(
+        CoprBuildTargetModel,
+        status=TestingFarmResult.failed,
+        target="some_build_target",
+    )
     model = flexmock(
         TFTTestRunTargetModel,
         status=TestingFarmResult.failed,
         target="some_tf_target",
     )
+    flexmock(build_model).should_receive("get_all_by_commit_target").with_args(
+        commit_sha="12345",
+    ).and_return(build_model)
     flexmock(model).should_receive("get_all_by_commit_target").with_args(
         commit_sha="12345",
     ).and_return(model)
+    flexmock(AbstractForgeIndependentEvent).should_receive(
+        "get_all_build_targets_by_status",
+    ).with_args(
+        statuses_to_filter_with=[BuildStatus.failure],
+    ).and_return(
+        {("some_build_target", None)},
+    )
+
     flexmock(AbstractForgeIndependentEvent).should_receive(
         "get_all_tf_targets_by_status",
     ).with_args(