diff --git a/packit_service/utils.py b/packit_service/utils.py index 61be2883d..6e27eea8d 100644 --- a/packit_service/utils.py +++ b/packit_service/utils.py @@ -335,6 +335,7 @@ def get_comment_parser_fedora_ci( prog: Optional[str] = None, description: Optional[str] = None, epilog: Optional[str] = None, + supported_test_types: Optional[list[str]] = None, ) -> argparse.ArgumentParser: parser = _create_base_parser(prog, description, epilog) @@ -351,7 +352,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", ) diff --git a/packit_service/worker/checker/testing_farm.py b/packit_service/worker/checker/testing_farm.py index ede0e11b1..4601b9582 100644 --- a/packit_service/worker/checker/testing_farm.py +++ b/packit_service/worker/checker/testing_farm.py @@ -328,3 +328,31 @@ def pre_check(self) -> bool: return False return True + + +class IsFMFConfigMissing(_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 True + return False + + +class IsProjectInTestsNamespace(_TestingFarmTestTypeChecker): + """ + Check whether the current project is located inside the tests namespace. + """ + + def pre_check(self) -> bool: + return self.project.namespace == "tests" diff --git a/packit_service/worker/handlers/mixin.py b/packit_service/worker/handlers/mixin.py index 33dae1044..ed650f4c8 100644 --- a/packit_service/worker/handlers/mixin.py +++ b/packit_service/worker/handlers/mixin.py @@ -5,7 +5,7 @@ from abc import abstractmethod from collections.abc import Iterator from dataclasses import dataclass -from typing import Any, Optional, Protocol, Union +from typing import TYPE_CHECKING, Any, Optional, Protocol, Union from packit.config import JobConfig, PackageConfig from packit.exceptions import PackitException @@ -35,13 +35,15 @@ from packit_service.worker.helpers.build.copr_build import CoprBuildJobHelper from packit_service.worker.helpers.build.koji_build import KojiBuildJobHelper from packit_service.worker.helpers.sidetag import Sidetag, SidetagHelper -from packit_service.worker.helpers.testing_farm import ( - DownstreamTestingFarmJobHelper, - TestingFarmJobHelper, -) from packit_service.worker.mixin import Config, ConfigFromEventMixin, GetBranches from packit_service.worker.monitoring import Pushgateway +if TYPE_CHECKING: + from packit_service.worker.helpers.testing_farm import ( + DownstreamTestingFarmJobHelper, + TestingFarmJobHelper, + ) + logger = logging.getLogger(__name__) @@ -615,7 +617,7 @@ class GetTestingFarmJobHelper(Protocol): @property @abstractmethod - def testing_farm_job_helper(self) -> TestingFarmJobHelper: ... + def testing_farm_job_helper(self) -> "TestingFarmJobHelper": ... class GetTestingFarmJobHelperMixin( @@ -623,11 +625,13 @@ class GetTestingFarmJobHelperMixin( GetCoprBuildMixin, ConfigFromEventMixin, ): - _testing_farm_job_helper: Optional[TestingFarmJobHelper] = None + _testing_farm_job_helper: Optional["TestingFarmJobHelper"] = None @property - def testing_farm_job_helper(self) -> TestingFarmJobHelper: + 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, @@ -647,7 +651,7 @@ class GetDownstreamTestingFarmJobHelper(Protocol): @property @abstractmethod - def downstream_testing_farm_job_helper(self) -> DownstreamTestingFarmJobHelper: ... + def downstream_testing_farm_job_helper(self) -> "DownstreamTestingFarmJobHelper": ... class GetDownstreamTestingFarmJobHelperMixin( @@ -655,11 +659,13 @@ class GetDownstreamTestingFarmJobHelperMixin( GetKojiBuildFromTaskOrPullRequestMixin, ConfigFromEventMixin, ): - _downstream_testing_farm_job_helper: Optional[DownstreamTestingFarmJobHelper] = None + _downstream_testing_farm_job_helper: Optional["DownstreamTestingFarmJobHelper"] = None @property - def downstream_testing_farm_job_helper(self) -> DownstreamTestingFarmJobHelper: + 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, diff --git a/packit_service/worker/handlers/testing_farm.py b/packit_service/worker/handlers/testing_farm.py index 3b18eda42..a6dc693c1 100644 --- a/packit_service/worker/handlers/testing_farm.py +++ b/packit_service/worker/handlers/testing_farm.py @@ -400,20 +400,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 ) ] @@ -493,10 +487,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: @@ -562,10 +554,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) diff --git a/packit_service/worker/helpers/testing_farm.py b/packit_service/worker/helpers/testing_farm.py index ba537d60a..1fc3a98e4 100644 --- a/packit_service/worker/helpers/testing_farm.py +++ b/packit_service/worker/helpers/testing_farm.py @@ -41,6 +41,11 @@ from packit_service.service.urls import get_testing_farm_info_url from packit_service.utils import get_package_nvrs, 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 ( + IsFMFConfigMissing, + IsProjectInTestsNamespace, +) from packit_service.worker.handlers.abstract import FedoraCIJobHandler from packit_service.worker.helpers.build import CoprBuildJobHelper from packit_service.worker.helpers.fedora_ci import FedoraCIHelper @@ -667,14 +672,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 not IsFMFConfigMissing( + 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( @@ -1255,9 +1255,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 @@ -1291,7 +1293,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. @@ -1303,31 +1308,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 not any( + 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 @@ -1447,7 +1468,10 @@ def prepare_and_send_tf_request( response=response, ) - @implements_fedora_ci_test("installability") + @implements_fedora_ci_test( + "installability", + checkers=[IsProjectInTestsNamespace], + ) def _payload_installability(self, distro: str, compose: str) -> dict: git_repo = "https://github.com/fedora-ci/installability-pipeline.git" git_ref = ( @@ -1494,7 +1518,10 @@ def _payload_installability(self, distro: str, compose: str) -> dict: }, } - @implements_fedora_ci_test("rpminspect") + @implements_fedora_ci_test( + "rpminspect", + checkers=[IsProjectInTestsNamespace], + ) def _payload_rpminspect(self, distro: str, compose: str) -> dict: git_repo = "https://github.com/fedora-ci/rpminspect-pipeline.git" git_ref = "master" @@ -1509,7 +1536,10 @@ def _payload_rpminspect(self, distro: str, compose: str) -> dict: } return payload - @implements_fedora_ci_test("rpmlint") + @implements_fedora_ci_test( + "rpmlint", + checkers=[IsProjectInTestsNamespace], + ) def _payload_rpmlint(self, distro: str, compose: str) -> dict: git_repo = "https://github.com/packit/tmt-plans.git" git_ref = "main" @@ -1525,22 +1555,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=[IsFMFConfigMissing], ) def _payload_custom(self, distro: str, compose: str) -> dict: payload = self._get_tf_base_payload(distro, compose) diff --git a/packit_service/worker/jobs.py b/packit_service/worker/jobs.py index 537282db5..cfe41cb2f 100644 --- a/packit_service/worker/jobs.py +++ b/packit_service/worker/jobs.py @@ -90,7 +90,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 @@ -265,7 +268,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() diff --git a/tests/integration/test_pr_comment.py b/tests/integration/test_pr_comment.py index 3598daced..befacf384 100644 --- a/tests/integration/test_pr_comment.py +++ b/tests/integration/test_pr_comment.py @@ -2603,6 +2603,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( @@ -2691,6 +2694,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, diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 9585c185a..82e1d91cf 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -9,12 +9,6 @@ from packit.config.aliases import Distro from packit.config.job_config import JobConfigTriggerType, JobType -from packit_service.constants import ( - HELP_COMMENT_DESCRIPTION, - HELP_COMMENT_EPILOG, - HELP_COMMENT_PROG, - HELP_COMMENT_PROG_FEDORA_CI, -) from packit_service.events.logdetective import Result as LogDetectiveResultEvent from packit_service.models import ( LogDetectiveRunModel, @@ -27,20 +21,14 @@ @pytest.fixture(scope="module") def comment_parser() -> argparse.ArgumentParser: - return get_comment_parser( - prog=HELP_COMMENT_PROG, - description=HELP_COMMENT_DESCRIPTION, - epilog=HELP_COMMENT_EPILOG, - ) + return get_comment_parser() @pytest.fixture(scope="module") def comment_parser_fedora_ci() -> argparse.ArgumentParser: - return get_comment_parser_fedora_ci( - prog=HELP_COMMENT_PROG_FEDORA_CI, - description=HELP_COMMENT_DESCRIPTION, - epilog=HELP_COMMENT_EPILOG, - ) + supported_test_types = ["installability", "rpmlint", "rpminspect", "custom"] + + return get_comment_parser_fedora_ci(supported_test_types=supported_test_types) @pytest.fixture()