Skip to content

Commit 09dd1f9

Browse files
committed
Turn existing functions into two new Checker sub-classes
The two static methods `is_project_in_tests_namespace` and `is_fmf_configured` have been removed and replaced with two new checkers `IsFMFConfigPresent` and `IsProjectOutsideOfTestsNamespace` in order to improve consistency of the code. Additionally, the `@implements_fedora_ci_test` decorator no longer expects a single optional callable `skipif`. Instead, it is possible to specify a list of Checkers, making it possible to specify multiple "skipif" conditions. Changes have been made to `testing_farm_mixin.py` in order to resolve the circular dependency introduced because of the aforementioned changes.
1 parent f861082 commit 09dd1f9

4 files changed

Lines changed: 93 additions & 63 deletions

File tree

packit_service/worker/checker/testing_farm.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,3 +330,31 @@ def pre_check(self) -> bool:
330330
return False
331331

332332
return True
333+
334+
335+
class IsFMFConfigPresent(_TestingFarmTestTypeChecker):
336+
"""
337+
Check whether FMF configuration is missing in the given project.
338+
"""
339+
340+
def pre_check(self) -> bool:
341+
try:
342+
commit_sha = self.data.event_dict.get("commit_sha")
343+
344+
self.project.get_file_content(
345+
path=".fmf/version",
346+
ref=commit_sha,
347+
)
348+
except FileNotFoundError:
349+
logger.debug("FMF configuration not found in repository.")
350+
return False
351+
return True
352+
353+
354+
class IsProjectOutsideOfTestsNamespace(_TestingFarmTestTypeChecker):
355+
"""
356+
Check whether the current project is located inside the tests namespace.
357+
"""
358+
359+
def pre_check(self) -> bool:
360+
return self.project.namespace != "tests"

packit_service/worker/helpers/testing_farm.py

Lines changed: 51 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@
4545
get_packit_commands_from_comment,
4646
)
4747
from packit_service.worker.celery_task import CeleryTask
48+
from packit_service.worker.checker.abstract import Checker
49+
from packit_service.worker.checker.testing_farm import (
50+
IsFMFConfigPresent,
51+
IsProjectOutsideOfTestsNamespace,
52+
)
4853
from packit_service.worker.helpers.build import CoprBuildJobHelper
4954
from packit_service.worker.helpers.fedora_ci import FedoraCIHelper
5055
from packit_service.worker.helpers.testing_farm_client import TestingFarmClient
@@ -685,14 +690,9 @@ def is_fmf_configured(self) -> bool:
685690
if self.custom_fmf:
686691
return True
687692

688-
try:
689-
self.project.get_file_content(
690-
path=f"{self.fmf_path}/.fmf/version",
691-
ref=self.metadata.commit_sha,
692-
)
693-
return True
694-
except FileNotFoundError:
695-
return False
693+
return IsFMFConfigPresent(
694+
package_config=None, job_config=None, event=self.metadata.event_dict, task_name=None
695+
).pre_check()
696696

697697
def report_missing_build_chroot(self, chroot: str):
698698
self.report_status_to_tests_for_chroot(
@@ -1303,9 +1303,11 @@ def cancel_running_tests(self):
13031303
FEDORA_CI_TESTS = {}
13041304

13051305

1306-
def implements_fedora_ci_test(test_name: str, skipif: Optional[Callable] = None) -> Callable:
1306+
def implements_fedora_ci_test(
1307+
test_name: str, checkers: Optional[list[type[Checker]]] = None
1308+
) -> Callable:
13071309
def _update_mapping(function: Callable) -> Callable:
1308-
FEDORA_CI_TESTS[test_name] = (function, skipif)
1310+
FEDORA_CI_TESTS[test_name] = (function, checkers)
13091311
return function
13101312

13111313
return _update_mapping
@@ -1342,7 +1344,7 @@ def get_fedora_ci_tests(
13421344
service_config: ServiceConfig,
13431345
project: GitProject,
13441346
metadata: EventData,
1345-
filter_specific_test: bool = True,
1347+
filter_specific_tests: bool = True,
13461348
) -> list[str]:
13471349
"""
13481350
Gets relevant Fedora CI tests registered using the `@implements_fedora_ci_test()` decorator.
@@ -1354,32 +1356,47 @@ def get_fedora_ci_tests(
13541356
service_config: Service config.
13551357
project: Git project.
13561358
metadata: Event metadata.
1357-
filter_specific_test: Whether to filter tests based on the command in user's comment.
1359+
filter_specific_tests: Whether to filter tests based on the command in user's comment.
13581360
13591361
Returns:
13601362
List of registered Fedora CI test names.
13611363
"""
1364+
1365+
def filter_tests(tests):
1366+
if metadata.event_type != pagure.pr.Comment.event_type():
1367+
return tests
1368+
# TODO: remove this once Fedora CI has its own instances and comment_command_prefixes
1369+
# comment_command_prefixes for Fedora CI are /packit-ci and /packit-ci-stg
1370+
comment_command_prefix = (
1371+
"/packit-ci-stg"
1372+
if service_config.comment_command_prefix.endswith("-stg")
1373+
else "/packit-ci"
1374+
)
1375+
commands = get_packit_commands_from_comment(
1376+
metadata.event_dict.get("comment"), comment_command_prefix
1377+
)
1378+
if not commands:
1379+
return []
1380+
if len(commands) > 1 and commands[1] in tests:
1381+
return [commands[1]]
1382+
return tests
1383+
13621384
all_tests = [
13631385
name
1364-
for name, (_, skipif) in FEDORA_CI_TESTS.items()
1365-
if not skipif or not skipif(service_config, project, metadata)
1386+
for name, (_, checkers) in FEDORA_CI_TESTS.items()
1387+
if all(
1388+
checker(
1389+
package_config=None,
1390+
job_config=None,
1391+
event=metadata.event_dict,
1392+
task_name=None,
1393+
).pre_check()
1394+
for checker in checkers
1395+
)
13661396
]
1367-
if metadata.event_type != pagure.pr.Comment.event_type() or not filter_specific_test:
1368-
return all_tests
1369-
# TODO: remove this once Fedora CI has its own instances and comment_command_prefixes
1370-
# comment_command_prefixes for Fedora CI are /packit-ci and /packit-ci-stg
1371-
comment_command_prefix = (
1372-
"/packit-ci-stg"
1373-
if service_config.comment_command_prefix.endswith("-stg")
1374-
else "/packit-ci"
1375-
)
1376-
commands = get_packit_commands_from_comment(
1377-
metadata.event_dict.get("comment"), comment_command_prefix
1378-
)
1379-
if not commands:
1380-
return []
1381-
if len(commands) > 1 and commands[1] in all_tests:
1382-
return [commands[1]]
1397+
1398+
if filter_specific_tests:
1399+
return filter_tests(all_tests)
13831400
return all_tests
13841401

13851402
@property
@@ -1501,9 +1518,7 @@ def prepare_and_send_tf_request(
15011518

15021519
@implements_fedora_ci_test(
15031520
"installability",
1504-
skipif=lambda _, project, __: DownstreamTestingFarmJobHelper.is_project_in_tests_namespace(
1505-
project
1506-
),
1521+
checkers=[IsProjectOutsideOfTestsNamespace],
15071522
)
15081523
def _payload_installability(self, distro: str, compose: str) -> dict:
15091524
git_repo = "https://github.com/fedora-ci/installability-pipeline.git"
@@ -1553,9 +1568,7 @@ def _payload_installability(self, distro: str, compose: str) -> dict:
15531568

15541569
@implements_fedora_ci_test(
15551570
"rpminspect",
1556-
skipif=lambda _, project, __: DownstreamTestingFarmJobHelper.is_project_in_tests_namespace(
1557-
project
1558-
),
1571+
checkers=[IsProjectOutsideOfTestsNamespace],
15591572
)
15601573
def _payload_rpminspect(self, distro: str, compose: str) -> dict:
15611574
git_repo = "https://github.com/fedora-ci/rpminspect-pipeline.git"
@@ -1573,9 +1586,7 @@ def _payload_rpminspect(self, distro: str, compose: str) -> dict:
15731586

15741587
@implements_fedora_ci_test(
15751588
"rpmlint",
1576-
skipif=lambda _, project, __: DownstreamTestingFarmJobHelper.is_project_in_tests_namespace(
1577-
project
1578-
),
1589+
checkers=[IsProjectOutsideOfTestsNamespace],
15791590
)
15801591
def _payload_rpmlint(self, distro: str, compose: str) -> dict:
15811592
git_repo = "https://github.com/packit/tmt-plans.git"
@@ -1592,26 +1603,9 @@ def _payload_rpmlint(self, distro: str, compose: str) -> dict:
15921603
}
15931604
return payload
15941605

1595-
@staticmethod
1596-
def is_fmf_configured(project: GitProject, metadata: EventData) -> bool:
1597-
try:
1598-
project.get_file_content(
1599-
path=".fmf/version",
1600-
ref=metadata.commit_sha,
1601-
)
1602-
except FileNotFoundError:
1603-
return False
1604-
return True
1605-
1606-
@staticmethod
1607-
def is_project_in_tests_namespace(project: GitProject) -> bool:
1608-
return project.namespace == "tests"
1609-
16101606
@implements_fedora_ci_test(
16111607
"custom",
1612-
skipif=lambda _, project, metadata: not DownstreamTestingFarmJobHelper.is_fmf_configured(
1613-
project, metadata
1614-
),
1608+
checkers=[IsFMFConfigPresent],
16151609
)
16161610
def _payload_custom(self, distro: str, compose: str) -> dict:
16171611
payload = self._get_tf_base_payload(distro, compose)

packit_service/worker/helpers/testing_farm_mixin.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from __future__ import annotations
1010

1111
from abc import abstractmethod
12-
from typing import Protocol
12+
from typing import TYPE_CHECKING, Protocol
1313

1414
from packit.config import JobConfig, PackageConfig
1515

@@ -18,12 +18,14 @@
1818
GetCoprBuildMixin,
1919
GetKojiBuildFromTaskOrPullRequestMixin,
2020
)
21-
from packit_service.worker.helpers.testing_farm import (
22-
DownstreamTestingFarmJobHelper,
23-
TestingFarmJobHelper,
24-
)
2521
from packit_service.worker.mixin import ConfigFromEventMixin
2622

23+
if TYPE_CHECKING:
24+
from packit_service.worker.helpers.testing_farm import (
25+
DownstreamTestingFarmJobHelper,
26+
TestingFarmJobHelper,
27+
)
28+
2729

2830
class GetTestingFarmJobHelper(Protocol):
2931
package_config: PackageConfig
@@ -45,6 +47,8 @@ class GetTestingFarmJobHelperMixin(
4547
@property
4648
def testing_farm_job_helper(self) -> TestingFarmJobHelper:
4749
if not self._testing_farm_job_helper:
50+
from packit_service.worker.helpers.testing_farm import TestingFarmJobHelper
51+
4852
self._testing_farm_job_helper = TestingFarmJobHelper(
4953
service_config=self.service_config,
5054
package_config=self.package_config,
@@ -77,6 +81,10 @@ class GetDownstreamTestingFarmJobHelperMixin(
7781
@property
7882
def downstream_testing_farm_job_helper(self) -> DownstreamTestingFarmJobHelper:
7983
if not self._downstream_testing_farm_job_helper:
84+
from packit_service.worker.helpers.testing_farm import (
85+
DownstreamTestingFarmJobHelper,
86+
)
87+
8088
self._downstream_testing_farm_job_helper = DownstreamTestingFarmJobHelper(
8189
service_config=self.service_config,
8290
project=self.project,

packit_service/worker/jobs.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ def parse_comment(
273273
self.service_config,
274274
self.event.project,
275275
EventData.from_event_dict(self.event.get_dict()),
276-
filter_specific_test=False,
276+
filter_specific_tests=False,
277277
)
278278
parser = get_comment_parser_fedora_ci(supported_test_types=supported_test_types)
279279
else:

0 commit comments

Comments
 (0)