Remove hardcoded list of supported Fedora CI tests (#3039)

Remove hardcoded list of supported Fedora CI tests

The Fedora CI comment parser no longer uses a hardcoded list supported test targets. Now it relies on the get_fedora_ci_tests to retrieve all available test targets in the given context. The implements_fedora_ci_test has been overhauled to accept a list of Checkers rather than a singular Callable, making the code easier to read and easy to add additional Checkers. Two new Checkers IsFMFConfigPresent and IsProjectOutsideOfTestsNamespace have been added to replace previously used methods.
Fixes #3038

Reviewed-by: gemini-code-assist[bot]
Reviewed-by: Nikola Forró
Reviewed-by: Alžběta Kučerová

Author: centosinfra-prod-github-app[bot]

packit_service/utils.py

@@ -341,6 +341,7 @@ def get_comment_parser_fedora_ci(
     prog: str | None = None,
     description: str | None = None,
     epilog: str | None = None,
+    supported_test_types: list[str] | None = None,
 ) -> argparse.ArgumentParser:
     parser = _create_base_parser(prog, description, epilog)
 
@@ -357,7 +358,7 @@ def get_comment_parser_fedora_ci(
     test_parser.add_argument(
         "test_identifier",
         nargs="?",
-        choices=["installability", "rpmlint", "rpminspect", "custom"],
+        choices=supported_test_types,
         help="specific type of tests to run",
     )
 

packit_service/worker/checker/testing_farm.py

@@ -330,3 +330,31 @@ def pre_check(self) -> bool:
             return False
 
         return True
+
+
+class IsFMFConfigPresent(_TestingFarmTestTypeChecker):
+    """
+    Check whether FMF configuration is missing in the given project.
+    """
+
+    def pre_check(self) -> bool:
+        try:
+            commit_sha = self.data.event_dict.get("commit_sha")
+
+            self.project.get_file_content(
+                path=".fmf/version",
+                ref=commit_sha,
+            )
+        except FileNotFoundError:
+            logger.debug("FMF configuration not found in repository.")
+            return False
+        return True
+
+
+class IsProjectOutsideOfTestsNamespace(_TestingFarmTestTypeChecker):
+    """
+    Check whether the current project is located inside the tests namespace.
+    """
+
+    def pre_check(self) -> bool:
+        return self.project.namespace != "tests"

packit_service/worker/handlers/testing_farm.py

@@ -430,20 +430,14 @@ def get_checkers() -> tuple[type[Checker], ...]:
             PermissionOnDistgitForFedoraCI,
         )
 
-    @classmethod
-    def filter_ci_tests(cls, tests: list[str]) -> list[str]:
-        return tests
-
     @classmethod
     def get_all_check_names(
         cls, service_config: ServiceConfig, project: GitProject, metadata: EventData
     ) -> list[str]:
         return [
             DownstreamTestingFarmJobHelper.get_check_name_from_config(t, service_config)
-            for t in cls.filter_ci_tests(
-                DownstreamTestingFarmJobHelper.get_fedora_ci_tests(
-                    service_config, project, metadata
-                )
+            for t in DownstreamTestingFarmJobHelper.get_fedora_ci_tests(
+                service_config, project, metadata
             )
         ]
 
@@ -546,10 +540,8 @@ def run_for_fedora_ci_test(
     def _run(self) -> TaskResults:
         failed: dict[str, str] = {}
 
-        fedora_ci_tests = self.filter_ci_tests(
-            self.downstream_testing_farm_job_helper.get_fedora_ci_tests(
-                self.service_config, self.project, self.data
-            )
+        fedora_ci_tests = self.downstream_testing_farm_job_helper.get_fedora_ci_tests(
+            self.service_config, self.project, self.data
         )
 
         if not fedora_ci_tests:
@@ -615,10 +607,6 @@ def get_checkers() -> tuple[type[Checker], ...]:
             PermissionOnDistgitForFedoraCI,
         )
 
-    @classmethod
-    def filter_ci_tests(cls, tests: list[str]) -> list[str]:
-        return [t for t in tests if t == "custom"]
-
 
 @configured_as(job_type=JobType.tests)
 @reacts_to(event=testing_farm.Result)

packit_service/worker/helpers/testing_farm.py

@@ -45,6 +45,11 @@
     get_packit_commands_from_comment,
 )
 from packit_service.worker.celery_task import CeleryTask
+from packit_service.worker.checker.abstract import Checker
+from packit_service.worker.checker.testing_farm import (
+    IsFMFConfigPresent,
+    IsProjectOutsideOfTestsNamespace,
+)
 from packit_service.worker.helpers.build import CoprBuildJobHelper
 from packit_service.worker.helpers.fedora_ci import FedoraCIHelper
 from packit_service.worker.helpers.testing_farm_client import TestingFarmClient
@@ -685,14 +690,9 @@ def is_fmf_configured(self) -> bool:
         if self.custom_fmf:
             return True
 
-        try:
-            self.project.get_file_content(
-                path=f"{self.fmf_path}/.fmf/version",
-                ref=self.metadata.commit_sha,
-            )
-            return True
-        except FileNotFoundError:
-            return False
+        return IsFMFConfigPresent(
+            package_config=None, job_config=None, event=self.metadata.event_dict, task_name=None
+        ).pre_check()
 
     def report_missing_build_chroot(self, chroot: str):
         self.report_status_to_tests_for_chroot(
@@ -1303,9 +1303,11 @@ def cancel_running_tests(self):
 FEDORA_CI_TESTS = {}
 
 
-def implements_fedora_ci_test(test_name: str, skipif: Optional[Callable] = None) -> Callable:
+def implements_fedora_ci_test(
+    test_name: str, checkers: Optional[list[type[Checker]]] = None
+) -> Callable:
     def _update_mapping(function: Callable) -> Callable:
-        FEDORA_CI_TESTS[test_name] = (function, skipif)
+        FEDORA_CI_TESTS[test_name] = (function, checkers)
         return function
 
     return _update_mapping
@@ -1339,7 +1341,10 @@ def koji_helper(self):
 
     @staticmethod
     def get_fedora_ci_tests(
-        service_config: ServiceConfig, project: GitProject, metadata: EventData
+        service_config: ServiceConfig,
+        project: GitProject,
+        metadata: EventData,
+        filter_specific_tests: bool = True,
     ) -> list[str]:
         """
         Gets relevant Fedora CI tests registered using the `@implements_fedora_ci_test()` decorator.
@@ -1351,31 +1356,47 @@ def get_fedora_ci_tests(
             service_config: Service config.
             project: Git project.
             metadata: Event metadata.
+            filter_specific_tests: Whether to filter tests based on the command in user's comment.
 
         Returns:
             List of registered Fedora CI test names.
         """
+
+        def filter_tests(tests):
+            if metadata.event_type != pagure.pr.Comment.event_type():
+                return tests
+            # TODO: remove this once Fedora CI has its own instances and comment_command_prefixes
+            # comment_command_prefixes for Fedora CI are /packit-ci and /packit-ci-stg
+            comment_command_prefix = (
+                "/packit-ci-stg"
+                if service_config.comment_command_prefix.endswith("-stg")
+                else "/packit-ci"
+            )
+            commands = get_packit_commands_from_comment(
+                metadata.event_dict.get("comment"), comment_command_prefix
+            )
+            if not commands:
+                return []
+            if len(commands) > 1 and commands[1] in tests:
+                return [commands[1]]
+            return tests
+
         all_tests = [
             name
-            for name, (_, skipif) in FEDORA_CI_TESTS.items()
-            if not skipif or not skipif(service_config, project, metadata)
+            for name, (_, checkers) in FEDORA_CI_TESTS.items()
+            if all(
+                checker(
+                    package_config=None,
+                    job_config=None,
+                    event=metadata.event_dict,
+                    task_name=None,
+                ).pre_check()
+                for checker in checkers
+            )
         ]
-        if metadata.event_type != pagure.pr.Comment.event_type():
-            return all_tests
-        # TODO: remove this once Fedora CI has its own instances and comment_command_prefixes
-        # comment_command_prefixes for Fedora CI are /packit-ci and /packit-ci-stg
-        comment_command_prefix = (
-            "/packit-ci-stg"
-            if service_config.comment_command_prefix.endswith("-stg")
-            else "/packit-ci"
-        )
-        commands = get_packit_commands_from_comment(
-            metadata.event_dict.get("comment"), comment_command_prefix
-        )
-        if not commands:
-            return []
-        if len(commands) > 1 and commands[1] in all_tests:
-            return [commands[1]]
+
+        if filter_specific_tests:
+            return filter_tests(all_tests)
         return all_tests
 
     @property
@@ -1495,7 +1516,10 @@ def prepare_and_send_tf_request(
             response=response,
         )
 
-    @implements_fedora_ci_test("installability")
+    @implements_fedora_ci_test(
+        "installability",
+        checkers=[IsProjectOutsideOfTestsNamespace],
+    )
     def _payload_installability(self, distro: str, compose: str) -> dict:
         git_repo = "https://github.com/fedora-ci/installability-pipeline.git"
         git_ref = (
@@ -1542,7 +1566,10 @@ def _payload_installability(self, distro: str, compose: str) -> dict:
             },
         }
 
-    @implements_fedora_ci_test("rpminspect")
+    @implements_fedora_ci_test(
+        "rpminspect",
+        checkers=[IsProjectOutsideOfTestsNamespace],
+    )
     def _payload_rpminspect(self, distro: str, compose: str) -> dict:
         git_repo = "https://github.com/fedora-ci/rpminspect-pipeline.git"
         git_ref = "master"
@@ -1557,7 +1584,10 @@ def _payload_rpminspect(self, distro: str, compose: str) -> dict:
         }
         return payload
 
-    @implements_fedora_ci_test("rpmlint")
+    @implements_fedora_ci_test(
+        "rpmlint",
+        checkers=[IsProjectOutsideOfTestsNamespace],
+    )
     def _payload_rpmlint(self, distro: str, compose: str) -> dict:
         git_repo = "https://github.com/packit/tmt-plans.git"
         git_ref = "main"
@@ -1573,22 +1603,9 @@ def _payload_rpmlint(self, distro: str, compose: str) -> dict:
         }
         return payload
 
-    @staticmethod
-    def is_fmf_configured(project: GitProject, metadata: EventData) -> bool:
-        try:
-            project.get_file_content(
-                path=".fmf/version",
-                ref=metadata.commit_sha,
-            )
-        except FileNotFoundError:
-            return False
-        return True
-
     @implements_fedora_ci_test(
         "custom",
-        skipif=lambda _, project, metadata: not DownstreamTestingFarmJobHelper.is_fmf_configured(
-            project, metadata
-        ),
+        checkers=[IsFMFConfigPresent],
     )
     def _payload_custom(self, distro: str, compose: str) -> dict:
         payload = self._get_tf_base_payload(distro, compose)

packit_service/worker/helpers/testing_farm_mixin.py

@@ -9,7 +9,7 @@
 from __future__ import annotations
 
 from abc import abstractmethod
-from typing import Protocol
+from typing import TYPE_CHECKING, Protocol
 
 from packit.config import JobConfig, PackageConfig
 
@@ -18,12 +18,14 @@
     GetCoprBuildMixin,
     GetKojiBuildFromTaskOrPullRequestMixin,
 )
-from packit_service.worker.helpers.testing_farm import (
-    DownstreamTestingFarmJobHelper,
-    TestingFarmJobHelper,
-)
 from packit_service.worker.mixin import ConfigFromEventMixin
 
+if TYPE_CHECKING:
+    from packit_service.worker.helpers.testing_farm import (
+        DownstreamTestingFarmJobHelper,
+        TestingFarmJobHelper,
+    )
+
 
 class GetTestingFarmJobHelper(Protocol):
     package_config: PackageConfig
@@ -45,6 +47,8 @@ class GetTestingFarmJobHelperMixin(
     @property
     def testing_farm_job_helper(self) -> TestingFarmJobHelper:
         if not self._testing_farm_job_helper:
+            from packit_service.worker.helpers.testing_farm import TestingFarmJobHelper
+
             self._testing_farm_job_helper = TestingFarmJobHelper(
                 service_config=self.service_config,
                 package_config=self.package_config,
@@ -77,6 +81,10 @@ class GetDownstreamTestingFarmJobHelperMixin(
     @property
     def downstream_testing_farm_job_helper(self) -> DownstreamTestingFarmJobHelper:
         if not self._downstream_testing_farm_job_helper:
+            from packit_service.worker.helpers.testing_farm import (
+                DownstreamTestingFarmJobHelper,
+            )
+
             self._downstream_testing_farm_job_helper = DownstreamTestingFarmJobHelper(
                 service_config=self.service_config,
                 project=self.project,

packit_service/worker/jobs.py

@@ -91,7 +91,10 @@
 from packit_service.worker.helpers.sync_release.propose_downstream import (
     ProposeDownstreamJobHelper,
 )
-from packit_service.worker.helpers.testing_farm import TestingFarmJobHelper
+from packit_service.worker.helpers.testing_farm import (
+    DownstreamTestingFarmJobHelper,
+    TestingFarmJobHelper,
+)
 from packit_service.worker.monitoring import Pushgateway
 from packit_service.worker.parser import Parser
 from packit_service.worker.reporting import BaseCommitStatus
@@ -266,7 +269,13 @@ def parse_comment(
             return ParsedComment()
 
         if comment.startswith("/packit-ci"):
-            parser = get_comment_parser_fedora_ci()
+            supported_test_types = DownstreamTestingFarmJobHelper.get_fedora_ci_tests(
+                self.service_config,
+                self.event.project,
+                EventData.from_event_dict(self.event.get_dict()),
+                filter_specific_tests=False,
+            )
+            parser = get_comment_parser_fedora_ci(supported_test_types=supported_test_types)
         else:
             parser = get_comment_parser()
 

tests/integration/test_pr_comment.py

@@ -2649,6 +2649,9 @@ def test_downstream_koji_scratch_build_retrigger_via_dist_git_pr_comment(
         .should_receive("get_web_url")
         .and_return("https://src.fedoraproject.org/rpms/python-teamcity-messages")
         .mock()
+        .should_receive("get_file_content")
+        .and_raise(FileNotFoundError)
+        .mock()
     )
     service_config = (
         flexmock(
@@ -2740,6 +2743,10 @@ def test_downstream_koji_scratch_build_retrigger_via_dist_git_pr_comment(
         (123, "koji-web-url")
     ).once()
 
+    flexmock(DownstreamTestingFarmJobHelper).should_receive("get_fedora_ci_tests").and_return(
+        ["installability", "rpmlint", "rpminspect", "custom"]
+    )
+
     processing_results = SteveJobs().process_message(pagure_pr_comment_added)
     event_dict, _, job_config, package_config = get_parameters_from_results(
         processing_results,