Skip to content

Commit

Permalink
Require copr build job definition for tests (#2012)
Browse files Browse the repository at this point in the history
Require copr build job definition for tests

Add pre-check for TestingFarmHandler that checks whether there is a copr build job definition in the config, if not, report it to user and do not run the tests.
Related to #1775
Do not merge this before the configurations in affected repos are updated (or at least wait some time after PRs are opened)

RELEASE NOTES BEGIN
Packit will now additionally require for each test job requiring build a build job definition to be present in the Packit configuration file.
RELEASE NOTES END

Reviewed-by: Jiri Popelka
Reviewed-by: František Lachman
  • Loading branch information
softwarefactory-project-zuul[bot] authored May 17, 2023
2 parents 3ab678d + 36a2ebc commit 7add044
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 459 deletions.
25 changes: 25 additions & 0 deletions packit_service/worker/checker/testing_farm.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,28 @@ def _pre_check(self) -> bool:
)
return False
return True


class IsCoprBuildDefined(Checker, GetTestingFarmJobHelperMixin):
"""
If the test job doesn't have enabled skip_build option, check whether
there is matching build job present and report if there is no.
"""

def pre_check(self) -> bool:
if (
not self.testing_farm_job_helper.skip_build
and not self.testing_farm_job_helper.job_build
):
logger.info(
"Build required and no build job found in the configuration, "
"reporting and skipping."
)
self.testing_farm_job_helper.report_status_to_tests(
description="Test job requires build job definition in the configuration.",
state=BaseCommitStatus.neutral,
url="",
)
return False

return True
31 changes: 0 additions & 31 deletions packit_service/worker/handlers/abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,37 +82,6 @@ def _add_to_mapping(kls: Type["JobHandler"]):
return _add_to_mapping


def required_for(job_type: JobType):
"""
[class decorator]
Specify a job_type for which this handler is the prerequisite.
E.g. for test, we need to run build first.
If there is a matching job_type defined by @configured_as,
we don't use the decorated handler with the job-config using this job_type.
If there is none, we use the job-config with this job_type.
Example:
- When there is a build and test defined, we run build only once
with the build job-config.
- When there is only test defined,
we run build with the test job-configuration.
```
@configured_as(job_type=JobType.copr_build)
@configured_as(job_type=JobType.build)
@required_for(job_type=JobType.tests)
class CoprBuildHandler(JobHandler):
```
"""

def _add_to_mapping(kls: Type["JobHandler"]):
MAP_REQUIRED_JOB_TYPE_TO_HANDLER[job_type].add(kls)
return kls

return _add_to_mapping


def reacts_to(event: Type["Event"]):
"""
[class decorator]
Expand Down
3 changes: 0 additions & 3 deletions packit_service/worker/handlers/copr.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
TaskName,
configured_as,
reacts_to,
required_for,
run_for_comment,
run_for_check_rerun,
RetriableJobHandler,
Expand Down Expand Up @@ -135,7 +134,6 @@ def get_checkers() -> Tuple[Type[Checker], ...]:

@configured_as(job_type=JobType.copr_build)
@configured_as(job_type=JobType.build)
@required_for(job_type=JobType.tests)
@reacts_to(event=CoprBuildStartEvent)
class CoprBuildStartHandler(AbstractCoprBuildReportHandler):
topic = "org.fedoraproject.prod.copr.build.start"
Expand Down Expand Up @@ -206,7 +204,6 @@ def run(self):

@configured_as(job_type=JobType.copr_build)
@configured_as(job_type=JobType.build)
@required_for(job_type=JobType.tests)
@reacts_to(event=CoprBuildEndEvent)
class CoprBuildEndHandler(AbstractCoprBuildReportHandler):
topic = "org.fedoraproject.prod.copr.build.end"
Expand Down
9 changes: 4 additions & 5 deletions packit_service/worker/handlers/testing_farm.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
IsEventForJob,
IsEventOk,
IsJobConfigTriggerMatching,
IsCoprBuildDefined,
)
from packit_service.worker.events import (
TestingFarmResultsEvent,
Expand Down Expand Up @@ -119,6 +120,7 @@ def get_checkers() -> Tuple[Type[Checker], ...]:
return (
IsJobConfigTriggerMatching,
IsEventOk,
IsCoprBuildDefined,
CanActorRunJob,
)

Expand Down Expand Up @@ -301,11 +303,8 @@ def run(self) -> TaskResults:
if self.testing_farm_job_helper.job_build:
msg = "Build required, already handled by build job."
else:
msg = "Build required, CoprBuildHandler task sent."
self.run_copr_build_handler(
self.data.get_dict(),
len(self.testing_farm_job_helper.build_targets),
)
# this should not happen as there is the IsCoprBuildDefined pre-check
msg = "Build required, no build job defined in config."
logger.info(msg)
return TaskResults(
success=True,
Expand Down
17 changes: 15 additions & 2 deletions tests/integration/test_check_rerun.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,12 @@ def mock_release_functionality(request):
"trigger": "pull_request",
"job": "tests",
"metadata": {"targets": "fedora-all"},
}
},
{
"trigger": "pull_request",
"job": "copr_build",
"metadata": {"targets": "fedora-all"},
},
]
]
),
Expand Down Expand Up @@ -436,7 +441,15 @@ def test_check_rerun_pr_koji_build_handler_old_job_name(
"targets": [
"fedora-all",
],
}
},
{
"trigger": "commit",
"job": "copr_build",
"branch": "main",
"targets": [
"fedora-all",
],
},
]
]
),
Expand Down
87 changes: 0 additions & 87 deletions tests/integration/test_listen_to_fedmsg.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,29 +211,6 @@ def pc_build_release():
)


@pytest.fixture(scope="module")
def pc_tests():
return PackageConfig(
jobs=[
JobConfig(
type=JobType.tests,
trigger=JobConfigTriggerType.pull_request,
packages={
"package": CommonPackageConfig(
_targets=["fedora-all"],
specfile_path="test.spec",
)
},
)
],
packages={
"package": CommonPackageConfig(
specfile_path="test.spec",
)
},
)


@pytest.fixture(scope="module")
def copr_build_branch_push():
return copr_build_model(
Expand Down Expand Up @@ -1568,70 +1545,6 @@ def test_copr_build_start(copr_build_start, pc_build_pr, copr_build_pr):
)


def test_copr_build_just_tests_defined(copr_build_start, pc_tests, copr_build_pr):
flexmock(GithubProject).should_receive("is_private").and_return(False)
flexmock(GithubProject).should_receive("get_pr").and_return(
flexmock(source_project=flexmock())
)
flexmock(AbstractCoprBuildEvent).should_receive("get_packages_config").and_return(
pc_tests
)
flexmock(CoprHelper).should_receive("get_copr_client").and_return(
Client(config={"username": "packit", "copr_url": "https://dummy.url"})
)
flexmock(TestingFarmJobHelper).should_receive("get_build_check").and_return(
EXPECTED_BUILD_CHECK_NAME
)
flexmock(TestingFarmJobHelper).should_receive("get_test_check").and_return(
EXPECTED_TESTING_FARM_CHECK_NAME
)

flexmock(CoprBuildTargetModel).should_receive("get_by_build_id").and_return(
copr_build_pr
)
url = get_copr_build_info_url(1)
flexmock(requests).should_receive("get").and_return(requests.Response())
flexmock(requests.Response).should_receive("raise_for_status").and_return(None)
copr_build_pr.should_receive("set_start_time").once()
copr_build_pr.should_call("set_status").with_args(BuildStatus.pending).once()
copr_build_pr.should_receive("set_build_logs_url")

# check if packit-service sets the correct PR status
flexmock(StatusReporter).should_receive("report").with_args(
state=BaseCommitStatus.running,
description="RPM build is in progress...",
url=url,
check_names=EXPECTED_BUILD_CHECK_NAME,
markdown_content=None,
links_to_external_services=None,
update_feedback_time=object,
).never()

flexmock(StatusReporter).should_receive("report").with_args(
state=BaseCommitStatus.running,
description="RPM build is in progress...",
url=url,
check_names=TestingFarmJobHelper.get_test_check(copr_build_start["chroot"]),
markdown_content=None,
links_to_external_services=None,
update_feedback_time=object,
).once()
flexmock(Signature).should_receive("apply_async").once()
flexmock(Pushgateway).should_receive("push").once().and_return()

processing_results = SteveJobs().process_message(copr_build_start)
event_dict, job, job_config, package_config = get_parameters_from_results(
processing_results
)
assert json.dumps(event_dict)

run_copr_build_start_handler(
package_config=package_config,
event=event_dict,
job_config=job_config,
)


def test_copr_build_not_comment_on_success(copr_build_end, pc_build_pr, copr_build_pr):
flexmock(GithubProject).should_receive("is_private").and_return(False)
flexmock(GithubProject).should_receive("get_pr").and_return(
Expand Down
Loading

0 comments on commit 7add044

Please sign in to comment.