From d9be0842171bc6b952532315dea2cd48e33c8ec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Forr=C3=B3?= Date: Mon, 23 Mar 2026 20:08:46 +0100 Subject: [PATCH 1/2] Cancel running jobs by event rather than previous commit SHA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some triggers such as comment commands have no relation to a commit SHA, so canceling doesn't work for them. We can use project event instead to identify running jobs that should be canceled, but event IDs are reused and we need to exclude current jobs, i.e. those that have been started by our trigger. To make that possible, before starting any tasks for a job record datetime of the most recent pipeline of the triggering event and use that as a cutoff time when filtering running jobs to cancel. Signed-off-by: Nikola Forró --- packit_service/events/event.py | 3 + packit_service/events/event_data.py | 9 ++ packit_service/models.py | 107 ++++++++++++++---- packit_service/worker/handlers/copr.py | 10 ++ .../worker/helpers/build/copr_build.py | 18 ++- .../worker/helpers/build/koji_build.py | 12 +- packit_service/worker/helpers/testing_farm.py | 19 +++- packit_service/worker/jobs.py | 8 ++ 8 files changed, 156 insertions(+), 30 deletions(-) diff --git a/packit_service/events/event.py b/packit_service/events/event.py index 86bec953e..dcc33dc7e 100644 --- a/packit_service/events/event.py +++ b/packit_service/events/event.py @@ -51,6 +51,7 @@ def _add_to_mapping(kls: type["Event"]): class Event(ABC): task_accepted_time: Optional[datetime] = None + cancel_cutoff_time: Optional[datetime] = None actor: Optional[str] def __init__(self, created_at: Optional[Union[int, float, str]] = None): @@ -139,6 +140,8 @@ def get_dict(self, default_dict: Optional[dict] = None) -> dict: d["task_accepted_time"] = ( int(task_accepted_time.timestamp()) if task_accepted_time else None ) + cancel_cutoff_time = d.get("cancel_cutoff_time") + d["cancel_cutoff_time"] = cancel_cutoff_time.timestamp() if cancel_cutoff_time else None d["project_url"] = d.get("project_url") or ( self.db_project_object.project.project_url if ( diff --git a/packit_service/events/event_data.py b/packit_service/events/event_data.py index 7ceca2124..c1834b242 100644 --- a/packit_service/events/event_data.py +++ b/packit_service/events/event_data.py @@ -40,6 +40,7 @@ def __init__( event_dict: Optional[dict], issue_id: Optional[int], task_accepted_time: Optional[datetime], + cancel_cutoff_time: Optional[datetime], build_targets_override: Optional[set[tuple[str, str]]], tests_targets_override: Optional[set[tuple[str, str]]], branches_override: Optional[list[str]], @@ -58,6 +59,7 @@ def __init__( self.event_dict = event_dict self.issue_id = issue_id self.task_accepted_time = task_accepted_time + self.cancel_cutoff_time = cancel_cutoff_time self.build_targets_override = ( set(build_targets_override) if build_targets_override else None ) @@ -91,6 +93,9 @@ def from_event_dict(cls, event: dict): time = event.get("task_accepted_time") task_accepted_time = datetime.fromtimestamp(time, timezone.utc) if time else None + cutoff = event.get("cancel_cutoff_time") + cancel_cutoff_time = datetime.fromtimestamp(cutoff, timezone.utc) if cutoff else None + build_targets_override = ( {(target, identifier_) for [target, identifier_] in event.get("build_targets_override")} if event.get("build_targets_override") @@ -118,6 +123,7 @@ def from_event_dict(cls, event: dict): event_dict=event, issue_id=issue_id, task_accepted_time=task_accepted_time, + cancel_cutoff_time=cancel_cutoff_time, build_targets_override=build_targets_override, tests_targets_override=tests_targets_override, branches_override=branches_override, @@ -138,6 +144,7 @@ def to_event(self) -> "Event": kwargs.pop("event_type", None) kwargs.pop("event_id", None) kwargs.pop("task_accepted_time", None) + kwargs.pop("cancel_cutoff_time", None) kwargs.pop("build_targets_override", None) kwargs.pop("tests_targets_override", None) kwargs.pop("branches_override", None) @@ -321,6 +328,8 @@ def get_dict(self) -> dict: d["task_accepted_time"] = ( int(task_accepted_time.timestamp()) if task_accepted_time else None ) + cancel_cutoff_time = d.get("cancel_cutoff_time") + d["cancel_cutoff_time"] = cancel_cutoff_time.timestamp() if cancel_cutoff_time else None if self.build_targets_override: d["build_targets_override"] = list(self.build_targets_override) if self.tests_targets_override: diff --git a/packit_service/models.py b/packit_service/models.py index f36ee359a..476e5d2f0 100644 --- a/packit_service/models.py +++ b/packit_service/models.py @@ -5,6 +5,7 @@ Data layer on top of PSQL using sqlalch """ +import datetime as dt import enum import logging import re @@ -1943,6 +1944,25 @@ def create( def get_project_event_object(self) -> AbstractProjectObjectDbType: return self.project_event.get_project_event_object() + @classmethod + def get_latest_datetime_for_event( + cls, + project_event_type: ProjectEventModelType, + event_id: int, + ) -> Optional[dt.datetime]: + """Return the datetime of the most recent pipeline for the given + project object (e.g. a PR or branch), or None if no pipelines exist.""" + with sa_session_transaction() as session: + return ( + session.query(func.max(PipelineModel.datetime)) + .join(ProjectEventModel) + .filter( + ProjectEventModel.type == project_event_type, + ProjectEventModel.event_id == event_id, + ) + .scalar() + ) + def __repr__(self): return ( f"PipelineModel(id={self.id}, datetime='{datetime}', " @@ -2175,30 +2195,43 @@ def get_by_id(cls, group_id: int) -> Optional["CoprBuildGroupModel"]: return session.query(CoprBuildGroupModel).filter_by(id=group_id).first() @classmethod - def get_running(cls, commit_sha: str) -> Iterable["CoprBuildTargetModel"]: - """Get list of currently running Copr builds matching the passed - arguments. + def get_running( + cls, + project_event_type: ProjectEventModelType, + event_id: int, + created_before: Optional[datetime] = None, + ) -> Iterable["CoprBuildTargetModel"]: + """Get list of currently running Copr builds for a given project object + (e.g. a PR or branch). Args: - commit_sha: Commit hash that is used for filtering the running jobs. + project_event_type: Type of the project event (e.g. pull_request). + event_id: ID of the project object (e.g. PullRequestModel.id). + created_before: If set, only return builds whose pipeline was + created at or before this datetime (used to avoid cancelling + builds from the current trigger batch). Returns: - An iterable over Copr target models that are curently in queue + An iterable over Copr target models that are currently in queue (running) or waiting for an SRPM. """ with sa_session_transaction() as session: - return ( + q = ( session.query(CoprBuildTargetModel) .join(CoprBuildGroupModel) .join(PipelineModel) .join(ProjectEventModel) .filter( - ProjectEventModel.commit_sha == commit_sha, + ProjectEventModel.type == project_event_type, + ProjectEventModel.event_id == event_id, CoprBuildTargetModel.status.in_( (BuildStatus.pending, BuildStatus.waiting_for_srpm) ), ) ) + if created_before is not None: + q = q.filter(PipelineModel.datetime <= created_before) + return q class BuildStatus(str, enum.Enum): @@ -2629,18 +2662,22 @@ def get_running( cls, project_event_type: ProjectEventModelType, event_id: int, + created_before: Optional[datetime] = None, ) -> Iterable["KojiBuildTargetModel"]: """Get running Koji builds for a given project object (e.g. a PR or branch). Args: project_event_type: Type of the project event (e.g. pull_request). event_id: ID of the project object (e.g. PullRequestModel.id). + created_before: If set, only return builds whose pipeline was + created at or before this datetime (used to avoid cancelling + builds from the current trigger batch). Returns: An iterable over KojiBuildTargetModels in non-final states. """ with sa_session_transaction() as session: - return ( + q = ( session.query(KojiBuildTargetModel) .join(KojiBuildGroupModel) .join(PipelineModel) @@ -2651,6 +2688,9 @@ def get_running( KojiBuildTargetModel.status.in_(("pending", "queued", "running")), ) ) + if created_before is not None: + q = q.filter(PipelineModel.datetime <= created_before) + return q class BodhiUpdateTargetModel(GroupAndTargetModelConnector, Base): @@ -3620,26 +3660,37 @@ def get_by_id(cls, group_id: int) -> Optional["TFTTestRunGroupModel"]: return session.query(TFTTestRunGroupModel).filter_by(id=group_id).first() @classmethod - def get_running(cls, commit_sha: str, ranch: str) -> Iterable["TFTTestRunTargetModel"]: - """Get list of currently running Testing Farm runs matching the passed - arguments. + def get_running( + cls, + project_event_type: ProjectEventModelType, + event_id: int, + ranch: str, + created_before: Optional[datetime] = None, + ) -> Iterable["TFTTestRunTargetModel"]: + """Get list of currently running Testing Farm runs for a given project + object (e.g. a PR or branch). Args: - commit_sha: Commit hash that is used for filtering the running jobs. + project_event_type: Type of the project event (e.g. pull_request). + event_id: ID of the project object (e.g. PullRequestModel.id). ranch: Testing Farm ranch where the tests are supposed to be run. + created_before: If set, only return test runs whose pipeline was + created at or before this datetime (used to avoid cancelling + tests from the current trigger batch). Returns: An iterable over TFT target models that reprepresent matching TF runs that are _queued_ (already submitted to the TF) or _running_. """ with sa_session_transaction() as session: - return ( + q = ( session.query(TFTTestRunTargetModel) .join(TFTTestRunGroupModel) .join(PipelineModel) .join(ProjectEventModel) .filter( - ProjectEventModel.commit_sha == commit_sha, + ProjectEventModel.type == project_event_type, + ProjectEventModel.event_id == event_id, TFTTestRunGroupModel.ranch == ranch, TFTTestRunTargetModel.status.in_( ( @@ -3649,6 +3700,9 @@ def get_running(cls, commit_sha: str, ranch: str) -> Iterable["TFTTestRunTargetM ), ) ) + if created_before is not None: + q = q.filter(PipelineModel.datetime <= created_before) + return q class TFTTestRunTargetModel(GroupAndTargetModelConnector, Base): @@ -4953,28 +5007,41 @@ def get_by_id(cls, group_id: int) -> Optional["LogDetectiveRunGroupModel"]: return session.query(LogDetectiveRunGroupModel).filter_by(id=group_id).first() @classmethod - def get_running(cls, commit_sha: str) -> Iterable[LogDetectiveRunModel]: - """Get list of currently running Log Detective runs matching the passed - arguments. + def get_running( + cls, + project_event_type: ProjectEventModelType, + event_id: int, + created_before: Optional[datetime] = None, + ) -> Iterable[LogDetectiveRunModel]: + """Get list of currently running Log Detective runs for a given project + object (e.g. a PR or branch). Args: - commit_sha: Commit hash that is used for filtering the running jobs. + project_event_type: Type of the project event (e.g. pull_request). + event_id: ID of the project object (e.g. PullRequestModel.id). + created_before: If set, only return runs whose pipeline was + created at or before this datetime (used to avoid cancelling + runs from the current trigger batch). Returns: An iterable over Log Detective run models representing Log Detective runs runs that are running. """ with sa_session_transaction() as session: - return ( + q = ( session.query(LogDetectiveRunModel) .join(LogDetectiveRunGroupModel) .join(PipelineModel) .join(ProjectEventModel) .filter( - ProjectEventModel.commit_sha == commit_sha, + ProjectEventModel.type == project_event_type, + ProjectEventModel.event_id == event_id, LogDetectiveRunModel.status == LogDetectiveResult.running, ) ) + if created_before is not None: + q = q.filter(PipelineModel.datetime <= created_before) + return q @cached(cache=TTLCache(maxsize=2048, ttl=(60 * 60 * 24))) diff --git a/packit_service/worker/handlers/copr.py b/packit_service/worker/handlers/copr.py index a4d649c67..d273c7d0b 100644 --- a/packit_service/worker/handlers/copr.py +++ b/packit_service/worker/handlers/copr.py @@ -477,6 +477,16 @@ def handle_testing_farm(self): event_dict = self.data.get_dict() + if ( + self.build + and self.build.group_of_targets.runs + and (cutoff := self.build.group_of_targets.runs[0].datetime) + ): + # use the pipeline datetime of the current build rather than + # recomputing via `get_latest_datetime_for_event()`, to avoid + # canceling tests triggered by sibling builds from the same batch + event_dict["cancel_cutoff_time"] = cutoff.timestamp() + for job_config in self.copr_build_helper.job_tests_all: if ( not job_config.skip_build diff --git a/packit_service/worker/helpers/build/copr_build.py b/packit_service/worker/helpers/build/copr_build.py index 52d022726..aa1ab9645 100644 --- a/packit_service/worker/helpers/build/copr_build.py +++ b/packit_service/worker/helpers/build/copr_build.py @@ -1024,10 +1024,20 @@ def get_configured_targets(self) -> set[str]: def get_running_jobs( self, ) -> Union[Iterable["CoprBuildTargetModel"], Iterable["TFTTestRunTargetModel"]]: - if sha := self.metadata.commit_sha_before: - yield from CoprBuildGroupModel.get_running(commit_sha=sha) - - # [SAFETY] When there's no previous commit hash, yields nothing + if not self.db_project_event: + logger.warning("No db_project_event, cannot query running Copr builds.") + return + if not self.metadata.cancel_cutoff_time: + logger.warning( + "No cancel_cutoff_time, skipping running Copr builds query " + "to avoid canceling unrelated jobs." + ) + return + yield from CoprBuildGroupModel.get_running( + project_event_type=self.db_project_event.type, + event_id=self.db_project_event.event_id, + created_before=self.metadata.cancel_cutoff_time, + ) def cancel_running_builds(self): running_builds = list(self.get_running_jobs()) diff --git a/packit_service/worker/helpers/build/koji_build.py b/packit_service/worker/helpers/build/koji_build.py index 167f2b93f..38667e727 100644 --- a/packit_service/worker/helpers/build/koji_build.py +++ b/packit_service/worker/helpers/build/koji_build.py @@ -229,9 +229,19 @@ def run_build( return get_koji_task_id_and_url_from_stdout(out) def get_running_jobs(self) -> Iterable[KojiBuildTargetModel]: - return KojiBuildGroupModel.get_running( + if not self.db_project_event: + logger.warning("No db_project_event, cannot query running Koji builds.") + return + if not self.metadata.cancel_cutoff_time: + logger.warning( + "No cancel_cutoff_time, skipping running Koji builds query " + "to avoid canceling unrelated jobs." + ) + return + yield from KojiBuildGroupModel.get_running( project_event_type=self.db_project_event.type, event_id=self.db_project_event.event_id, + created_before=self.metadata.cancel_cutoff_time, ) def cancel_running_builds( diff --git a/packit_service/worker/helpers/testing_farm.py b/packit_service/worker/helpers/testing_farm.py index ba537d60a..a3e13fb97 100644 --- a/packit_service/worker/helpers/testing_farm.py +++ b/packit_service/worker/helpers/testing_farm.py @@ -1234,12 +1234,21 @@ def report_status_to_configured_job( ) def get_running_jobs(self) -> Iterable["TFTTestRunTargetModel"]: - if sha := self.metadata.commit_sha_before: - yield from TFTTestRunGroupModel.get_running( - commit_sha=sha, ranch=self.tft_client.default_ranch + if not self.db_project_event: + logger.warning("No db_project_event, cannot query running TF runs.") + return + if not self.metadata.cancel_cutoff_time: + logger.warning( + "No cancel_cutoff_time, skipping running TF runs query " + "to avoid canceling unrelated jobs." ) - - # [SAFETY] When there's no previous commit hash, yields nothing + return + yield from TFTTestRunGroupModel.get_running( + project_event_type=self.db_project_event.type, + event_id=self.db_project_event.event_id, + ranch=self.tft_client.default_ranch, + created_before=self.metadata.cancel_cutoff_time, + ) def cancel_running_tests(self): running_tests = list(self.get_running_jobs()) diff --git a/packit_service/worker/jobs.py b/packit_service/worker/jobs.py index 537282db5..5b5ae185d 100644 --- a/packit_service/worker/jobs.py +++ b/packit_service/worker/jobs.py @@ -38,6 +38,7 @@ from packit_service.events.event import Event from packit_service.events.event_data import EventData from packit_service.fedora_ci_config import FedoraCIConfig +from packit_service.models import PipelineModel from packit_service.package_config_getter import PackageConfigGetter from packit_service.utils import ( elapsed_seconds, @@ -801,6 +802,13 @@ def create_tasks( """ processing_results: list[TaskResults] = [] signatures = [] + + if self.event.db_project_event: + self.event.cancel_cutoff_time = PipelineModel.get_latest_datetime_for_event( + project_event_type=self.event.db_project_event.type, + event_id=self.event.db_project_event.event_id, + ) + # we want to run handlers for all possible jobs, not just the first one for job_config in job_configs: if self.should_task_be_created_for_job_config_and_handler( From 8125abad49d42a9216236f57383b950604279e46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikola=20Forr=C3=B3?= Date: Tue, 24 Mar 2026 17:41:24 +0100 Subject: [PATCH 2/2] Update and add new tests for job canceling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nikola Forró Assisted-by: Claude Opus 4.6 via Claude Code --- packit_service/worker/handlers/bodhi.py | 2 +- tests/conftest.py | 15 +- tests/integration/conftest.py | 10 +- tests/integration/test_bodhi_update.py | 30 ++-- tests/integration/test_commit_comment.py | 2 +- tests/integration/test_dg_commit.py | 41 ++++-- tests/integration/test_issue_comment.py | 7 +- tests/integration/test_koji_build.py | 11 +- tests/integration/test_koji_build_cancel.py | 128 +++++++++++++++++- tests/integration/test_listen_to_fedmsg.py | 6 +- tests/integration/test_new_hotness_update.py | 20 ++- tests/integration/test_pr_comment.py | 51 +++++-- tests/integration/test_pr_comment_monorepo.py | 2 +- tests/integration/test_release_event.py | 5 +- tests/integration/test_vm_image_build.py | 2 +- tests/unit/test_jobs.py | 5 + tests_openshift/database/test_models.py | 128 ++++++++++++++++-- 17 files changed, 399 insertions(+), 66 deletions(-) diff --git a/packit_service/worker/handlers/bodhi.py b/packit_service/worker/handlers/bodhi.py index c5bc2f330..f3026283b 100644 --- a/packit_service/worker/handlers/bodhi.py +++ b/packit_service/worker/handlers/bodhi.py @@ -365,7 +365,7 @@ def _get_or_create_bodhi_update_group_model(self) -> BodhiUpdateGroupModel: group = None for koji_build_data in self: koji_build_target = KojiBuildTargetModel.get_by_task_id( - koji_build_data.task_id, + task_id=koji_build_data.task_id, ) if koji_build_target: run_model = koji_build_target.group_of_targets.runs[-1] diff --git a/tests/conftest.py b/tests/conftest.py index f66501a35..c4092cea5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,6 +18,7 @@ from packit_service.fedora_ci_config import FedoraCIConfig from packit_service.models import ( BuildStatus, + PipelineModel, ProjectEventModel, ProjectEventModelType, PullRequestModel, @@ -60,6 +61,13 @@ def _reset_fedora_ci_config(): FedoraCIConfig._instance = None +@pytest.fixture(autouse=True) +def _mock_pipeline_get_latest_datetime_for_event(): + """Mock PipelineModel.get_latest_datetime_for_event so tests don't + need a real database connection for the cancel-cutoff-time lookup.""" + flexmock(PipelineModel).should_receive("get_latest_datetime_for_event").and_return(None) + + @pytest.fixture() def dump_http_com(): """ @@ -259,6 +267,7 @@ def mock_set_built_packages(built_packages): srpm_build=srpm_build, copr_build_group=copr_group, test_run_group=None, + datetime=datetime.now(), ) runs.append(run_model) @@ -409,7 +418,7 @@ def add_pull_request_event_with_sha_123456(): id=123, ) db_project_event = ( - flexmock(type=ProjectEventModelType.pull_request, commit_sha="123456") + flexmock(type=ProjectEventModelType.pull_request, event_id=123, commit_sha="123456") .should_receive("get_project_event_object") .and_return(db_project_object) .mock() @@ -425,7 +434,7 @@ def add_pull_request_event_with_pr_id_9(): project_event_model_type=ProjectEventModelType.pull_request, ) db_project_event = ( - flexmock() + flexmock(type=ProjectEventModelType.pull_request, event_id=9) .should_receive("get_project_event_object") .and_return(db_project_object) .mock() @@ -457,7 +466,7 @@ def add_pull_request_event_with_sha_0011223344(): project_event_model_type=ProjectEventModelType.pull_request, ) db_project_event = ( - flexmock(id=123) + flexmock(id=123, type=ProjectEventModelType.pull_request, event_id=9) .should_receive("get_project_event_object") .and_return(db_project_object) .mock() diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 5ddc5edc6..8cfc0e323 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -49,7 +49,7 @@ def mock_pr_functionality(request): .mock() ) project_event = ( - flexmock(ProjectEventModel(type=JobConfigTriggerType.pull_request, id=123456)) + flexmock(ProjectEventModel(type=JobConfigTriggerType.pull_request, id=123456, event_id=123)) .should_receive("get_project_event_object") .and_return(pr_model) .mock() @@ -94,7 +94,7 @@ def mock_push_functionality(request): .mock() ) project_event = ( - flexmock(ProjectEventModel(type=JobConfigTriggerType.commit, id=123456)) + flexmock(ProjectEventModel(type=JobConfigTriggerType.commit, id=123456, event_id=123)) .should_receive("get_project_event_object") .and_return(branch_model) .mock() @@ -140,7 +140,7 @@ def mock_release_functionality(request): .mock() ) project_event = ( - flexmock(ProjectEventModel(type=JobConfigTriggerType.release, id=123456)) + flexmock(ProjectEventModel(type=JobConfigTriggerType.release, id=123456, event_id=123)) .should_receive("get_project_event_object") .and_return(release_model) .mock() @@ -175,6 +175,8 @@ def _setup(package_name: str, project_url: str): job_config_trigger_type=JobConfigTriggerType.pull_request, project=flexmock(project_url=None), id=123, + type=ProjectEventModelType.pull_request, + event_id=12, ) flexmock(AddPullRequestEventToDb).should_receive("db_project_object").and_return( project_event, @@ -189,7 +191,7 @@ def _setup(package_name: str, project_url: str): job_config_trigger_type=JobConfigTriggerType.pull_request, ) flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").with_args( - 79721403, + task_id=79721403, ).and_return(None) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.pull_request, diff --git a/tests/integration/test_bodhi_update.py b/tests/integration/test_bodhi_update.py index 346eebdc5..7a2d3ac81 100644 --- a/tests/integration/test_bodhi_update.py +++ b/tests/integration/test_bodhi_update.py @@ -98,7 +98,7 @@ def test_bodhi_update_for_unknown_koji_build(koji_build_completed_old_format): project_event_model_type=ProjectEventModelType.branch_push, ) flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").with_args( - 79721403, + task_id=79721403, ).and_return(None) flexmock(GitBranchModel).should_receive("get_or_create").and_return( git_branch_model_flexmock, @@ -208,7 +208,7 @@ def test_bodhi_update_for_unknown_koji_build_failed(koji_build_completed_old_for project_event_model_type=ProjectEventModelType.branch_push, ) flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").with_args( - 79721403, + task_id=79721403, ).and_return(None) flexmock(GitBranchModel).should_receive("get_or_create").and_return( git_branch_model_flexmock, @@ -321,7 +321,7 @@ def test_bodhi_update_for_unknown_koji_build_failed_issue_created( project_event_model_type=ProjectEventModelType.branch_push, ) flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").with_args( - 79721403, + task_id=79721403, ).and_return(None) flexmock(GitBranchModel).should_receive("get_or_create").and_return( git_branch_model_flexmock, @@ -448,7 +448,7 @@ def test_bodhi_update_for_unknown_koji_build_failed_issue_comment( project_event_model_type=ProjectEventModelType.branch_push, ) flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").with_args( - 79721403, + task_id=79721403, ).and_return(None) flexmock(GitBranchModel).should_receive("get_or_create").and_return( git_branch_model_flexmock, @@ -587,13 +587,17 @@ def test_bodhi_update_build_not_tagged_yet( project_event_model_type=ProjectEventModelType.branch_push, ) flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").with_args( - 79721403, + task_id=79721403, ).and_return( flexmock( get_project_event_object=lambda: flexmock( id=1, job_config_trigger_type=JobConfigTriggerType.commit, ), + get_project_event_model=lambda: flexmock( + type=ProjectEventModelType.branch_push, + event_id=1, + ), group_of_targets=flexmock(runs=[flexmock()]), ), ) @@ -709,7 +713,7 @@ def test_bodhi_update_for_unknown_koji_build_not_for_unfinished( project_event_model_type=ProjectEventModelType.branch_push, ) flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").with_args( - 79721403, + task_id=79721403, ).and_return(None) flexmock(GitBranchModel).should_receive("get_or_create").and_return( git_branch_model_flexmock, @@ -795,13 +799,17 @@ def test_bodhi_update_for_known_koji_build(koji_build_completed_old_format): # Database structure flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").with_args( - 79721403, + task_id=79721403, ).and_return( flexmock( get_project_event_object=lambda: flexmock( id=1, job_config_trigger_type=JobConfigTriggerType.commit, ), + get_project_event_model=lambda: flexmock( + type=ProjectEventModelType.branch_push, + event_id=1, + ), group_of_targets=flexmock(runs=[flexmock()]), ), ) @@ -865,7 +873,7 @@ def test_bodhi_update_for_not_configured_branch(koji_build_completed_old_format) project_event_model_type=ProjectEventModelType.branch_push, ) flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").with_args( - 79721403, + task_id=79721403, ).and_return(None) flexmock(GitBranchModel).should_receive("get_or_create").and_return( git_branch_model_flexmock, @@ -950,13 +958,17 @@ def test_bodhi_update_fedora_stable_by_default(koji_build_completed_f36): bodhi_update_group=group_model, ).and_return() flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").with_args( - 80860789, + task_id=80860789, ).and_return( flexmock( get_project_event_object=lambda: flexmock( id=1, job_config_trigger_type=JobConfigTriggerType.commit, ), + get_project_event_model=lambda: flexmock( + type=ProjectEventModelType.branch_push, + event_id=1, + ), group_of_targets=flexmock(runs=[flexmock()]), ), ) diff --git a/tests/integration/test_commit_comment.py b/tests/integration/test_commit_comment.py index 7bc128cde..fa9b888ba 100644 --- a/tests/integration/test_commit_comment.py +++ b/tests/integration/test_commit_comment.py @@ -62,7 +62,7 @@ def mock_commit_comment_functionality(request): name="main", ) db_project_event = ( - flexmock() + flexmock(type=ProjectEventModelType.branch_push, event_id=9) .should_receive("get_project_event_object") .and_return(db_project_object) .mock() diff --git a/tests/integration/test_dg_commit.py b/tests/integration/test_dg_commit.py index 5b7ed38a8..c3b93d664 100644 --- a/tests/integration/test_dg_commit.py +++ b/tests/integration/test_dg_commit.py @@ -84,7 +84,7 @@ def test_sync_from_downstream(): type=ProjectEventModelType.branch_push, event_id=9, commit_sha="abcd", - ).and_return(flexmock()) + ).and_return(flexmock(type=ProjectEventModelType.branch_push, event_id=9)) flexmock(GitBranchModel).should_receive("get_or_create").with_args( branch_name="main", namespace="rpms", @@ -170,7 +170,7 @@ def test_do_not_sync_from_downstream_on_a_different_branch(): type=ProjectEventModelType.branch_push, event_id=9, commit_sha="abcd", - ).and_return(flexmock()) + ).and_return(flexmock(type=ProjectEventModelType.branch_push, event_id=9)) flexmock(GitBranchModel).should_receive("get_or_create").with_args( branch_name="main", namespace="rpms", @@ -249,13 +249,16 @@ def test_downstream_koji_build(sidetag_group): project_event_model_type=ProjectEventModelType.branch_push, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.branch_push, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.branch_push, event_id=9, commit_sha="abcd", - ).and_return(flexmock()) + ).and_return(flexmock(type=ProjectEventModelType.branch_push, event_id=9)) flexmock(GitBranchModel).should_receive("get_or_create").with_args( branch_name="main", namespace="rpms", @@ -396,13 +399,16 @@ def test_downstream_koji_build_failure_no_issue(): project_event_model_type=ProjectEventModelType.branch_push, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.branch_push, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.branch_push, event_id=9, commit_sha="abcd", - ).and_return(flexmock()) + ).and_return(flexmock(type=ProjectEventModelType.branch_push, event_id=9)) flexmock(GitBranchModel).should_receive("get_or_create").with_args( branch_name="main", namespace="rpms", @@ -502,13 +508,16 @@ def test_downstream_koji_build_failure_issue_created(): project_event_model_type=ProjectEventModelType.branch_push, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.branch_push, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.branch_push, event_id=9, commit_sha="abcd", - ).and_return(flexmock()) + ).and_return(flexmock(type=ProjectEventModelType.branch_push, event_id=9)) flexmock(GitBranchModel).should_receive("get_or_create").with_args( branch_name="main", namespace="rpms", @@ -616,13 +625,16 @@ def test_downstream_koji_build_failure_issue_comment(): project_event_model_type=ProjectEventModelType.branch_push, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.branch_push, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.branch_push, event_id=9, commit_sha="abcd", - ).and_return(flexmock()) + ).and_return(flexmock(type=ProjectEventModelType.branch_push, event_id=9)) flexmock(GitBranchModel).should_receive("get_or_create").with_args( branch_name="main", namespace="rpms", @@ -812,13 +824,16 @@ def test_downstream_koji_build_where_multiple_branches_defined(jobs_config): project_event_model_type=ProjectEventModelType.branch_push, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.branch_push, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.branch_push, event_id=9, commit_sha="abcd", - ).and_return(flexmock()) + ).and_return(flexmock(type=ProjectEventModelType.branch_push, event_id=9)) flexmock(GitBranchModel).should_receive("get_or_create").with_args( branch_name="main", namespace="rpms", @@ -950,7 +965,7 @@ def test_do_not_run_downstream_koji_build_for_a_different_branch(jobs_config): type=ProjectEventModelType.branch_push, event_id=9, commit_sha="abcd", - ).and_return(flexmock()) + ).and_return(flexmock(type=ProjectEventModelType.branch_push, event_id=9)) flexmock(GitBranchModel).should_receive("get_or_create").with_args( branch_name="main", namespace="rpms", diff --git a/tests/integration/test_issue_comment.py b/tests/integration/test_issue_comment.py index d9b83f128..4a8277ff7 100644 --- a/tests/integration/test_issue_comment.py +++ b/tests/integration/test_issue_comment.py @@ -263,6 +263,8 @@ def test_issue_comment_propose_downstream_handler( ) db_project_event = ( flexmock( + type=ProjectEventModelType.issue, + event_id=123, id=123456, project_event_model_type=ProjectEventModelType.issue, ) @@ -409,7 +411,10 @@ def mock_repository_issue_retriggering(): ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.issue, event_id=123) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(IssueModel).should_receive("get_or_create").and_return(db_project_object) diff --git a/tests/integration/test_koji_build.py b/tests/integration/test_koji_build.py index cb33ae3d9..a91a2ff40 100644 --- a/tests/integration/test_koji_build.py +++ b/tests/integration/test_koji_build.py @@ -91,6 +91,10 @@ def test_downstream_koji_build_report_known_build(koji_build_fixture, request): id=1, job_config_trigger_type=JobConfigTriggerType.commit, ), + get_project_event_model=lambda: flexmock( + type=ProjectEventModelType.branch_push, + event_id=9, + ), set_status=lambda x: None, set_build_start_time=lambda x: None, set_build_finished_time=lambda x: None, @@ -99,7 +103,7 @@ def test_downstream_koji_build_report_known_build(koji_build_fixture, request): .with_args({}) .and_return() .mock(), - ).once() # only when running a handler + ).twice() # once from create_tasks() db_project_event access, once from handler processing_results = SteveJobs().process_message(koji_build_event) # 1*KojiBuildReportHandler @@ -149,7 +153,10 @@ def test_koji_build_error_msg(distgit_push_packit): project_event_model_type=ProjectEventModelType.branch_push, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.branch_push, event_id=123) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(pagure.push.Commit).should_receive("db_project_object").and_return( db_project_object, diff --git a/tests/integration/test_koji_build_cancel.py b/tests/integration/test_koji_build_cancel.py index e467120a2..d8a03f523 100644 --- a/tests/integration/test_koji_build_cancel.py +++ b/tests/integration/test_koji_build_cancel.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: MIT import json +from datetime import datetime, timezone import pytest from celery.canvas import Signature, group @@ -102,7 +103,10 @@ def mock_distgit_pr_functionality(): project=flexmock(project_url="https://src.fedoraproject.org/rpms/optee_os"), ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.pull_request, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.pull_request, @@ -341,7 +345,10 @@ def test_downstream_koji_build_cancel_running(monkeypatch): project_event_model_type=ProjectEventModelType.branch_push, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.branch_push, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.branch_push, @@ -392,3 +399,120 @@ def test_downstream_koji_build_cancel_running(monkeypatch): ) assert first_dict_value(results["job"])["success"] + + +def test_downstream_koji_build_cancel_uses_event_based_filtering(monkeypatch): + """Test that cancel_running_builds passes event-based parameters to get_running. + + Verifies the end-to-end flow introduced by commit e4cde978: + 1. SteveJobs.create_tasks() queries PipelineModel.get_latest_datetime_for_event + and sets cancel_cutoff_time on the event + 2. cancel_cutoff_time is serialized into the event dict + 3. The handler's get_running_jobs() passes project_event_type, event_id, + and created_before (not commit_sha) to KojiBuildGroupModel.get_running + + This ensures that cancellation targets only builds from the same event + (e.g. the same PR), not builds from other PRs that happen to share a commit SHA. + """ + monkeypatch.setenv("CANCEL_RUNNING_JOBS", "1") + + cutoff_time = datetime(2025, 1, 15, 10, 30, 0, tzinfo=timezone.utc) + + # Override the autouse fixture mock — return a specific cutoff time + flexmock(PipelineModel).should_receive("get_latest_datetime_for_event").with_args( + project_event_type=ProjectEventModelType.branch_push, + event_id=9, + ).and_return(cutoff_time).once() + + # Pagure project with a koji_build job in .packit.yaml + packit_yaml = ( + "{'specfile_path': 'buildah.spec'," + "'jobs': [{'trigger': 'commit', 'job': 'koji_build', 'allowed_committers':" + " ['rhcontainerbot']}]," + "'downstream_package_name': 'buildah'}" + ) + pagure_project = flexmock( + PagureProject, + full_repo_name="rpms/buildah", + get_web_url=lambda: "https://src.fedoraproject.org/rpms/buildah", + default_branch="main", + ) + pagure_project.should_receive("get_files").with_args( + ref="main", + filter_regex=r".+\.spec$", + ).and_return(["buildah.spec"]) + pagure_project.should_receive("get_file_content").with_args( + path=".packit.yaml", + ref="main", + headers=dict, + ).and_return(packit_yaml) + pagure_project.should_receive("get_files").with_args( + ref="main", + recursive=False, + ).and_return(["buildah.spec", ".packit.yaml"]) + + # Database models for branch push event + db_project_object = flexmock( + id=9, + job_config_trigger_type=JobConfigTriggerType.commit, + project_event_model_type=ProjectEventModelType.branch_push, + ) + db_project_event = ( + flexmock(type=ProjectEventModelType.branch_push, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() + ) + flexmock(ProjectEventModel).should_receive("get_or_create").with_args( + type=ProjectEventModelType.branch_push, + event_id=9, + commit_sha="abcd", + ).and_return(flexmock()) + flexmock(GitBranchModel).should_receive("get_or_create").with_args( + branch_name="main", + namespace="rpms", + repo_name="buildah", + project_url="https://src.fedoraproject.org/rpms/buildah", + ).and_return(db_project_object) + flexmock(ProjectEventModel).should_receive("get_or_create").and_return( + db_project_event, + ) + + # Infrastructure no-ops + flexmock(PipelineModel).should_receive("create") + flexmock(LocalProjectBuilder, _refresh_the_state=lambda *args: None) + flexmock(group).should_receive("apply_async") + flexmock(Pushgateway).should_receive("push").and_return() + flexmock(IsRunConditionSatisfied).should_receive("pre_check").and_return(True) + + # Short-circuit _run() right after cancel_running_builds + flexmock(DownstreamKojiBuildHandler).should_receive( + "_get_or_create_koji_group_model", + ).and_raise(PackitException, "mock error") + + # The key assertion: get_running must be called with event-based parameters + # (project_event_type + event_id + created_before), not commit_sha + flexmock(KojiBuildGroupModel).should_receive("get_running").with_args( + project_event_type=ProjectEventModelType.branch_push, + event_id=9, + created_before=cutoff_time, + ).and_return([]).once() + + distgit_commit = json.loads( + (DATA_DIR / "fedmsg" / "distgit_commit.json").read_text(), + ) + + processing_results = SteveJobs().process_message(distgit_commit) + event_dict, _, job_config, package_config = get_parameters_from_results( + processing_results, + ) + + # Verify that cancel_cutoff_time was serialized into the event dict + assert event_dict.get("cancel_cutoff_time") == cutoff_time.timestamp() + + results = run_downstream_koji_build( + package_config=package_config, + event=event_dict, + job_config=job_config, + ) + assert first_dict_value(results["job"])["success"] diff --git a/tests/integration/test_listen_to_fedmsg.py b/tests/integration/test_listen_to_fedmsg.py index 52d107415..8fad93496 100644 --- a/tests/integration/test_listen_to_fedmsg.py +++ b/tests/integration/test_listen_to_fedmsg.py @@ -3096,6 +3096,7 @@ def test_koji_build_tag( flexmock(Signature).should_receive("apply_async").twice() flexmock(celery_group).should_receive("apply_async").once() + flexmock(KojiBuildTargetModel).should_receive("get_by_task_id").and_return(None) processing_results = SteveJobs().process_message(koji_build_tagged) event_dict, _, job_config, package_config = get_parameters_from_results( @@ -3355,7 +3356,10 @@ def test_pagure_pr_updated(pagure_pr_updated, project_namespace, project_repo): project=dg_project, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.pull_request, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.pull_request, diff --git a/tests/integration/test_new_hotness_update.py b/tests/integration/test_new_hotness_update.py index c19e4eeba..00ddbd287 100644 --- a/tests/integration/test_new_hotness_update.py +++ b/tests/integration/test_new_hotness_update.py @@ -55,7 +55,10 @@ def sync_release_model(): project=flexmock(AnityaProjectModel), ) anitya_event = ( - flexmock().should_receive("get_project_event_object").and_return(anitya_db_object).mock() + flexmock(type=ProjectEventModelType.anitya_multiple_versions, event_id=12) + .should_receive("get_project_event_object") + .and_return(anitya_db_object) + .mock() ) flexmock(AnityaMultipleVersionsModel).should_receive("get_or_create").with_args( versions=["7.0.3"], @@ -115,7 +118,10 @@ class AnityaTestProjectModel(AnityaProjectModel): project=AnityaTestProjectModel(), ) project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.release, event_id=12) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) run_model = flexmock(PipelineModel) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( @@ -334,7 +340,10 @@ def test_new_hotness_update_pre_check_fail(new_hotness_update): project=flexmock(AnityaProjectModel), ) anitya_event = ( - flexmock().should_receive("get_project_event_object").and_return(anitya_db_object).mock() + flexmock(type=ProjectEventModelType.anitya_multiple_versions, event_id=12) + .should_receive("get_project_event_object") + .and_return(anitya_db_object) + .mock() ) flexmock(AnityaMultipleVersionsModel).should_receive("get_or_create").with_args( versions=["7.0.3"], @@ -386,7 +395,10 @@ class AnityaTestProjectModel(AnityaProjectModel): project=AnityaTestProjectModel(), ) project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.anitya_multiple_versions, event_id=12) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) run_model = flexmock(PipelineModel) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( diff --git a/tests/integration/test_pr_comment.py b/tests/integration/test_pr_comment.py index 3598daced..25cf8495c 100644 --- a/tests/integration/test_pr_comment.py +++ b/tests/integration/test_pr_comment.py @@ -197,7 +197,7 @@ def mock_pr_comment_functionality(request): project_event_model_type=ProjectEventModelType.pull_request, ) db_project_event = ( - flexmock() + flexmock(type=ProjectEventModelType.pull_request, event_id=9) .should_receive("get_project_event_object") .and_return(db_project_object) .mock() @@ -524,7 +524,7 @@ def test_pr_comment_production_build_handler(pr_production_build_comment_event): project_event_model_type=ProjectEventModelType.pull_request, ) project_event = ( - flexmock() + flexmock(type=ProjectEventModelType.pull_request, event_id=9) .should_receive("get_project_event_object") .and_return(db_project_object) .mock() @@ -1813,7 +1813,7 @@ def test_retest_failed( project_event_model_type=ProjectEventModelType.pull_request, ) db_project_event = ( - flexmock() + flexmock(type=ProjectEventModelType.pull_request, event_id=9) .should_receive("get_project_event_object") .and_return(db_project_object) .mock() @@ -2476,7 +2476,10 @@ def test_koji_build_retrigger_via_dist_git_pr_comment(pagure_pr_comment_added): job_config_trigger_type=JobConfigTriggerType.pull_request, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.pull_request, event_id=12) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.pull_request, @@ -2632,7 +2635,10 @@ def test_downstream_koji_scratch_build_retrigger_via_dist_git_pr_comment( project=flexmock(project_url="https://src.fedoraproject.org/rpms/python-teamcity-messages"), ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.pull_request, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.pull_request, @@ -3002,7 +3008,10 @@ def _get_project(url, *_, **__): job_config_trigger_type=JobConfigTriggerType.pull_request, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.pull_request, event_id=12) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) run_model = flexmock(PipelineModel) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( @@ -3172,7 +3181,10 @@ def _get_project(url, *_, **__): job_config_trigger_type=JobConfigTriggerType.pull_request, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.pull_request, event_id=12) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) run_model = flexmock(PipelineModel) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( @@ -3368,7 +3380,10 @@ def _get_project(url, *_, **__): job_config_trigger_type=JobConfigTriggerType.pull_request, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.pull_request, event_id=12) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) run_model = flexmock(PipelineModel) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( @@ -3518,7 +3533,10 @@ def test_koji_build_tag_via_dist_git_pr_comment(pagure_pr_comment_added, all_bra job_config_trigger_type=JobConfigTriggerType.pull_request, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.pull_request, event_id=12) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.pull_request, @@ -3712,7 +3730,10 @@ def _test_downstream_tf_retrigger_common( project=flexmock(project_url="https://src.fedoraproject.org/rpms/python-teamcity-messages"), ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.pull_request, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.pull_request, @@ -3987,7 +4008,10 @@ def test_downstream_testing_farm_retrigger_rawhide_pr_eln_package_fedora_ci( project=flexmock(project_url="https://src.fedoraproject.org/rpms/python-teamcity-messages"), ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.pull_request, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.pull_request, @@ -4173,7 +4197,10 @@ def test_downstream_build_retrigger_rawhide_pr_eln_package_fedora_ci( project=flexmock(project_url="https://src.fedoraproject.org/rpms/python-teamcity-messages"), ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.pull_request, event_id=9) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( type=ProjectEventModelType.pull_request, diff --git a/tests/integration/test_pr_comment_monorepo.py b/tests/integration/test_pr_comment_monorepo.py index 727a61577..1e59c8d8f 100644 --- a/tests/integration/test_pr_comment_monorepo.py +++ b/tests/integration/test_pr_comment_monorepo.py @@ -65,7 +65,7 @@ def mock_pr_comment_monorepo_functionality(request): project_event_model_type=ProjectEventModelType.pull_request, ) db_project_event = ( - flexmock() + flexmock(type=ProjectEventModelType.pull_request, event_id=1418) .should_receive("get_project_event_object") .and_return(db_project_object) .mock() diff --git a/tests/integration/test_release_event.py b/tests/integration/test_release_event.py index 113f6e943..bf9e3bbc2 100644 --- a/tests/integration/test_release_event.py +++ b/tests/integration/test_release_event.py @@ -66,7 +66,10 @@ def propose_downstream_model(sync_release_pr_model): job_config_trigger_type=JobConfigTriggerType.release, ) db_project_event = ( - flexmock().should_receive("get_project_event_object").and_return(db_project_object).mock() + flexmock(type=ProjectEventModelType.release, event_id=12) + .should_receive("get_project_event_object") + .and_return(db_project_object) + .mock() ) run_model = flexmock(PipelineModel) flexmock(ProjectEventModel).should_receive("get_or_create").with_args( diff --git a/tests/integration/test_vm_image_build.py b/tests/integration/test_vm_image_build.py index 3e22a2944..c32581c01 100644 --- a/tests/integration/test_vm_image_build.py +++ b/tests/integration/test_vm_image_build.py @@ -80,7 +80,7 @@ def test_vm_image_build(github_vm_image_build_comment): type=ProjectEventModelType.pull_request, event_id=1, commit_sha="123456", - ).and_return(flexmock()) + ).and_return(flexmock(type=ProjectEventModelType.pull_request, event_id=1)) flexmock(PullRequestModel).should_receive("get_or_create").and_return( flexmock( job_config_trigger_type=JobConfigTriggerType.pull_request, diff --git a/tests/unit/test_jobs.py b/tests/unit/test_jobs.py index c292dd550..098a2065f 100644 --- a/tests/unit/test_jobs.py +++ b/tests/unit/test_jobs.py @@ -27,6 +27,7 @@ testing_farm, vm_image, ) +from packit_service.models import TFTTestRunTargetModel from packit_service.worker.allowlist import Allowlist from packit_service.worker.handlers import ( CoprBuildEndHandler, @@ -3422,6 +3423,8 @@ def test_create_tasks_tf_identifier( class Event(event_kls): def __init__(self): self._db_project_object = None + self._db_project_event = None + self.pipeline_id = None @property def packages_config(self): @@ -3443,6 +3446,8 @@ def actor(self): SteveJobs, report_task_accepted=lambda handler_kls, job_config, update_feedback_time: None, ) + # Mock DB lookup for testing farm result events + flexmock(TFTTestRunTargetModel).should_receive("get_by_pipeline_id").and_return(None) # We are testing the number of tasks, the exact signatures are not important flexmock(handler_kls).should_receive("get_signature").and_return(None) flexmock(TaskResults, create_from=lambda *args, **kwargs: object()) diff --git a/tests_openshift/database/test_models.py b/tests_openshift/database/test_models.py index 76a241a39..1644bae0f 100644 --- a/tests_openshift/database/test_models.py +++ b/tests_openshift/database/test_models.py @@ -1274,7 +1274,12 @@ def test_create_koji_tag_request(clean_before_and_after, a_koji_tag_request): assert a_koji_tag_request.get_project().project_url == SampleValues.project_url -def test_copr_get_running(clean_before_and_after, pr_model, srpm_build_model_with_new_run_for_pr): +def test_copr_get_running( + clean_before_and_after, + pr_model, + pr_project_event_model, + srpm_build_model_with_new_run_for_pr, +): _, run_model = srpm_build_model_with_new_run_for_pr group, _ = CoprBuildGroupModel.create(run_model=run_model) @@ -1294,7 +1299,12 @@ def test_copr_get_running(clean_before_and_after, pr_model, srpm_build_model_wit copr_build_group=group, ) - running = list(CoprBuildGroupModel.get_running(commit_sha=SampleValues.commit_sha)) + running = list( + CoprBuildGroupModel.get_running( + project_event_type=pr_project_event_model.type, + event_id=pr_project_event_model.event_id, + ) + ) assert running, "There are some running builds present" assert len(running) == 3, "There are exactly 3 builds running" assert {build.build_id for build in running} == {"1", "2"}, ( @@ -1302,7 +1312,12 @@ def test_copr_get_running(clean_before_and_after, pr_model, srpm_build_model_wit ) -def test_tmt_get_running(clean_before_and_after, pr_model, srpm_build_model_with_new_run_for_pr): +def test_tmt_get_running( + clean_before_and_after, + pr_model, + pr_project_event_model, + srpm_build_model_with_new_run_for_pr, +): _, run_model = srpm_build_model_with_new_run_for_pr group = TFTTestRunGroupModel.create(run_model, ranch="public") @@ -1320,7 +1335,11 @@ def test_tmt_get_running(clean_before_and_after, pr_model, srpm_build_model_with ) running = list( - TFTTestRunGroupModel.get_running(commit_sha=SampleValues.commit_sha, ranch="public") + TFTTestRunGroupModel.get_running( + project_event_type=pr_project_event_model.type, + event_id=pr_project_event_model.event_id, + ranch="public", + ) ) assert running, "There are some running tests present" assert len(running) == 2, "There are exactly 2 tests running" @@ -1330,7 +1349,10 @@ def test_tmt_get_running(clean_before_and_after, pr_model, srpm_build_model_with def test_tmt_get_running_different_ranches( - clean_before_and_after, pr_model, srpm_build_model_with_new_run_for_pr + clean_before_and_after, + pr_model, + pr_project_event_model, + srpm_build_model_with_new_run_for_pr, ): _, run_model = srpm_build_model_with_new_run_for_pr @@ -1359,7 +1381,11 @@ def test_tmt_get_running_different_ranches( ) running = list( - TFTTestRunGroupModel.get_running(commit_sha=SampleValues.commit_sha, ranch="public") + TFTTestRunGroupModel.get_running( + project_event_type=pr_project_event_model.type, + event_id=pr_project_event_model.event_id, + ranch="public", + ) ) assert running, "There are some running tests present" assert len(running) == 2, "There are exactly 2 tests running in the public ranch" @@ -1368,7 +1394,11 @@ def test_tmt_get_running_different_ranches( ) running = list( - TFTTestRunGroupModel.get_running(commit_sha=SampleValues.commit_sha, ranch="redhat") + TFTTestRunGroupModel.get_running( + project_event_type=pr_project_event_model.type, + event_id=pr_project_event_model.event_id, + ranch="redhat", + ) ) assert running, "There are some running tests present" assert len(running) == 2, "There are exactly 2 tests running in the redhat ranch" @@ -1508,7 +1538,9 @@ def test_log_detective_run_group_targets( assert group.grouped_targets[0] == run_target -def test_log_detective_get_running(clean_before_and_after, srpm_build_model_with_new_run_for_pr): +def test_log_detective_get_running( + clean_before_and_after, pr_project_event_model, srpm_build_model_with_new_run_for_pr +): _, run_model = srpm_build_model_with_new_run_for_pr group = LogDetectiveRunGroupModel.create([run_model]) @@ -1542,8 +1574,12 @@ def test_log_detective_get_running(clean_before_and_after, srpm_build_model_with target="", ) - # The fixture uses SampleValues.commit_sha ("80201a74d96c") - running = list(LogDetectiveRunGroupModel.get_running(SampleValues.commit_sha)) + running = list( + LogDetectiveRunGroupModel.get_running( + project_event_type=pr_project_event_model.type, + event_id=pr_project_event_model.event_id, + ) + ) assert running, "There should be running analysis present" assert len(running) == 1, "There is exactly 1 analysis running" @@ -1553,6 +1589,78 @@ def test_log_detective_get_running(clean_before_and_after, srpm_build_model_with assert run_model.analysis_id == "uuid-1" +def test_get_latest_datetime_for_event( + clean_before_and_after, + pr_project_event_model, + srpm_build_model_with_new_run_for_pr, +): + """PipelineModel.get_latest_datetime_for_event returns the datetime of the + most recent pipeline for a given project event.""" + _, run_model = srpm_build_model_with_new_run_for_pr + + result = PipelineModel.get_latest_datetime_for_event( + project_event_type=pr_project_event_model.type, + event_id=pr_project_event_model.event_id, + ) + assert result is not None + assert result == run_model.datetime + + +def test_get_latest_datetime_for_event_no_pipelines( + clean_before_and_after, +): + """Returns None when there are no pipelines for the given event.""" + result = PipelineModel.get_latest_datetime_for_event( + project_event_type=ProjectEventModelType.pull_request, + event_id=999999, + ) + assert result is None + + +def test_copr_get_running_with_created_before( + clean_before_and_after, + pr_model, + pr_project_event_model, + srpm_build_model_with_new_run_for_pr, +): + """CoprBuildGroupModel.get_running with created_before filters out builds + whose pipeline was created after the cutoff time.""" + _, run_model = srpm_build_model_with_new_run_for_pr + group, _ = CoprBuildGroupModel.create(run_model=run_model) + + CoprBuildTargetModel.create( + build_id="1", + project_name="something", + owner="hello", + web_url=None, + target="fedora-rawhide-x86_64", + status=BuildStatus.pending, + copr_build_group=group, + ) + + # With created_before set to BEFORE the pipeline datetime, no builds returned + cutoff = run_model.datetime - timedelta(seconds=1) + running = list( + CoprBuildGroupModel.get_running( + project_event_type=pr_project_event_model.type, + event_id=pr_project_event_model.event_id, + created_before=cutoff, + ) + ) + assert len(running) == 0, "No running builds before cutoff" + + # With created_before set to AFTER the pipeline datetime, builds are returned + cutoff = run_model.datetime + timedelta(seconds=1) + running = list( + CoprBuildGroupModel.get_running( + project_event_type=pr_project_event_model.type, + event_id=pr_project_event_model.event_id, + created_before=cutoff, + ) + ) + assert len(running) == 1, "Running build found after cutoff" + + @pytest.mark.parametrize( "build_system", [LogDetectiveBuildSystem.copr, LogDetectiveBuildSystem.koji] )