Skip to content

Commit

Permalink
Hide manual test statuses only when build handler is required (#2389)
Browse files Browse the repository at this point in the history 
Hide manual test statuses only when build handler is required

We should not check the labels and identifiers, this should be handled by pre-checks and results in no reporting for TF notifications.
Fixes #2386
TODO:

 testcase

RELEASE NOTES BEGIN
We have fixed reporting of tests with manual_trigger: True.
RELEASE NOTES END

Reviewed-by: Maja Massarini
  • Loading branch information
@softwarefactory-project-zuul
softwarefactory-project-zuul[bot] authored Apr 4, 2024
2 parents 45b7e50 + a37db92 commit cef6943
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 209 deletions.
15 changes: 8 additions & 7 deletions packit_service/worker/handlers/testing_farm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,14 @@ def run_with_copr_builds(self, targets: List[str], failed: Dict):
"The latest build has not finished yet, "
"waiting until it finishes before running tests for it."
)
self.testing_farm_job_helper.report_status_to_tests_for_test_target(
state=BaseCommitStatus.pending,
description="The latest build has not finished yet, "
"waiting until it finishes before running tests for it.",
target=test_run.target,
url=get_copr_build_info_url(copr_build.id),
)
if not self.job_config.manual_trigger:
self.testing_farm_job_helper.report_status_to_tests_for_test_target(
state=BaseCommitStatus.pending,
description="The latest build has not finished yet, "
"waiting until it finishes before running tests for it.",
target=test_run.target,
url=get_copr_build_info_url(copr_build.id),
)
continue

# Only retry what's needed
Expand Down
61 changes: 4 additions & 57 deletions packit_service/worker/helpers/testing_farm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1298,45 +1298,6 @@ def test_target2build_target(self, test_target: str) -> str:
"""
return self.test_target2build_target_for_test_job(test_target, self.job_config)

def is_manual_job_being_triggered_by_label(self) -> bool:
"""
Is this job meant to be triggered manually
and is it being triggered by a matching
/packit test --labels label
command or has it one of the default_labels?
"""
if self.job_config.manual_trigger:
if self.job_config.labels and self.comment_arguments.labels:
return bool(
set(self.job_config.labels).intersection(
set(self.comment_arguments.labels)
)
)
if self.job_config.test_command.default_labels:
return bool(
set(self.job_config.labels).intersection(
set(self.job_config.test_command.default_labels)
)
)
return False

def is_manual_job_being_triggered_by_identifier(self) -> bool:
"""
Is this job meant to be triggered manually
and is it being triggered by a matching
/packit test --identifier job-identifier
command or is it the default_identifier?
"""
if self.job_config.manual_trigger:
if self.job_config.identifier and self.comment_arguments.identifier:
return self.job_config.identifier == self.comment_arguments.identifier
if self.job_config.test_command.default_identifier:
return (
self.job_config.identifier
== self.job_config.test_command.default_identifier
)
return False

@property
def build_targets_for_tests(self) -> Set[str]:
"""
Expand All @@ -1355,12 +1316,6 @@ def report_status_to_tests_for_chroot(
links_to_external_services: Optional[Dict[str, str]] = None,
update_feedback_time: Callable = None,
) -> None:
if self.job_config.manual_trigger and not (
self.is_manual_job_being_triggered_by_label()
or self.is_manual_job_being_triggered_by_identifier()
):
return

if chroot in self.build_targets_for_tests:
test_targets = self.build_target2test_targets_for_test_job(
chroot, self.job_config
Expand All @@ -1386,12 +1341,6 @@ def report_status_to_tests_for_test_target(
links_to_external_services: Optional[Dict[str, str]] = None,
update_feedback_time: Callable = None,
) -> None:
if self.job_config.manual_trigger and not (
self.is_manual_job_being_triggered_by_label()
or self.is_manual_job_being_triggered_by_identifier()
):
return

if target in self.tests_targets:
self._report(
description=description,
Expand All @@ -1412,12 +1361,6 @@ def report_status_to_tests(
links_to_external_services: Optional[Dict[str, str]] = None,
update_feedback_time: Callable = None,
) -> None:
if self.job_config.manual_trigger and not (
self.is_manual_job_being_triggered_by_label()
or self.is_manual_job_being_triggered_by_identifier()
):
return

self._report(
description=description,
state=state,
Expand All @@ -1437,6 +1380,10 @@ def report_status_to_configured_job(
links_to_external_services: Optional[Dict[str, str]] = None,
update_feedback_time: Callable = None,
):
if self.job_config.manual_trigger and self.build_required():
logger.debug("Skipping the reporting.")
return

self.report_status_to_tests(
description=description,
state=state,
Expand Down
77 changes: 77 additions & 0 deletions tests/integration/test_pr_comment.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,83 @@ def test_pr_comment_build_build_and_test_handler(
assert "already handled" in first_dict_value(results["job"])["details"]["msg"]


@pytest.mark.parametrize(
"mock_pr_comment_functionality",
(
[
[
{
"trigger": "pull_request",
"job": "copr_build",
"metadata": {"targets": "fedora-rawhide-x86_64"},
},
{
"trigger": "pull_request",
"job": "tests",
"manual_trigger": True,
"metadata": {"targets": "fedora-rawhide-x86_64"},
},
]
]
),
indirect=True,
)
def test_pr_comment_build_build_and_test_handler_manual_test_reporting(
mock_pr_comment_functionality, pr_build_comment_event
):
flexmock(GithubProject).should_receive("is_private").and_return(False)
flexmock(CoprHelper).should_receive("get_valid_build_targets").and_return(set())
flexmock(CoprBuildJobHelper).should_receive("report_status_to_build").with_args(
description=TASK_ACCEPTED,
state=BaseCommitStatus.pending,
url="",
markdown_content=None,
links_to_external_services=None,
update_feedback_time=object,
).once()
# check that we are not reporting task accepted if manual_trigger is enabled for tests
flexmock(TestingFarmJobHelper).should_receive("report_status_to_tests").with_args(
description=TASK_ACCEPTED,
state=BaseCommitStatus.pending,
url="",
markdown_content=None,
links_to_external_services=None,
update_feedback_time=object,
).never()
flexmock(CoprBuildJobHelper).should_receive(
"is_custom_copr_project_defined"
).and_return(False).once()
flexmock(Signature).should_receive("apply_async").twice()
flexmock(Pushgateway).should_receive("push").times(4).and_return()
pr = flexmock(head_commit="12345")
flexmock(GithubProject).should_receive("get_pr").and_return(pr)
comment = flexmock()
flexmock(pr).should_receive("get_comment").and_return(comment)
flexmock(comment).should_receive("add_reaction").with_args(COMMENT_REACTION).once()

processing_results = SteveJobs().process_message(pr_build_comment_event)
assert len(processing_results) == 2

copr_build_job = [
item for item in processing_results if item["details"]["job"] == "copr_build"
]
assert copr_build_job

test_job = [
item for item in processing_results if item["details"]["job"] == "tests"
]
assert test_job

event_dict, job, job_config, package_config = get_parameters_from_results(test_job)
assert json.dumps(event_dict)
results = run_testing_farm_handler(
package_config=package_config,
event=event_dict,
job_config=job_config,
)
assert first_dict_value(results["job"])["success"]


def test_pr_comment_production_build_handler(pr_production_build_comment_event):
packit_yaml = str(
{
Expand Down
150 changes: 5 additions & 145 deletions tests/unit/test_testing_farm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
from celery.canvas import Signature
from flexmock import flexmock

from packit.config.commands import TestCommandConfig

import packit_service.models
import packit_service.service.urls as urls
from packit.config import (
Expand Down Expand Up @@ -96,6 +94,7 @@ def test_testing_farm_response(
),
JobConfig(
type=JobType.tests,
manual_trigger=True,
trigger=JobConfigTriggerType.pull_request,
packages={
"package": CommonPackageConfig(
Expand All @@ -108,7 +107,9 @@ def test_testing_farm_response(
flexmock(PackageConfigGetter).should_receive(
"get_package_config_from_repo"
).and_return(package_config)
config = flexmock(command_handler_work_dir=flexmock())
config = flexmock(
command_handler_work_dir=flexmock(), comment_command_prefix="/packit"
)
flexmock(TFResultsHandler).should_receive("service_config").and_return(config)
flexmock(TFResultsEvent).should_receive("db_project_object").and_return(None)
config.should_receive("get_project").with_args(
Expand Down Expand Up @@ -139,6 +140,7 @@ def test_testing_farm_response(
job_config=JobConfig(
type=JobType.tests,
trigger=JobConfigTriggerType.pull_request,
manual_trigger=True,
packages={
"package": CommonPackageConfig(
identifier=None,
Expand Down Expand Up @@ -1896,145 +1898,3 @@ def test_parse_comment_arguments(
assert helper.comment_arguments.pr_argument == expected_pr_arg
assert helper.comment_arguments.identifier == expected_identifier
assert helper.comment_arguments.labels == expected_labels


@pytest.mark.parametrize(
"comment,job_identifier,job_labels,manual_trigger,test_command,show_status_check_times",
[
(
"/packit-dev test --identifier my-id-1 --labels label1,label2",
"my-id-1",
[],
True,
None,
1,
),
(
"/packit-dev test --identifier my-id-1 --labels label1,label2",
"my-id-2",
[
"label1",
],
True,
None,
1,
),
(
"/packit-dev test --identifier my-id-2 --labels label1,label2",
"my-id-1",
[
"label3",
],
True,
None,
0,
),
(
"/packit-dev test --identifier my-id-2 --labels label1,label2",
"my-id-1",
[
"label3",
],
False,
None,
1,
),
(
"/packit-dev test",
"my-id-2",
[
"label1",
],
True,
None,
0,
),
(
"/packit-dev test",
"my-id-2",
[
"label1",
],
False,
None,
1,
),
(
"/packit-dev test --identifier my-id-1 --labels label1,label2",
"my-id-1",
[],
False,
None,
1,
),
(
"/packit-dev test --identifier my-id-1 --labels label1,label2",
"my-id-2",
[
"label1",
],
False,
None,
1,
),
(
"/packit-dev test",
"my-id-2",
[
"label1",
],
True,
TestCommandConfig(default_labels=["label5", "label1"]),
1,
),
(
"/packit-dev test",
"my-id-2",
[
"label1",
],
True,
TestCommandConfig(default_identifier="my-id-2"),
1,
),
],
)
def test_manually_triggered_tests_statuses_report(
comment: str,
job_identifier: str,
job_labels: List[str],
manual_trigger,
test_command,
show_status_check_times,
):
job_config = JobConfig(
trigger=JobConfigTriggerType.pull_request,
type=JobType.tests,
manual_trigger=manual_trigger,
labels=job_labels,
packages={
"package": CommonPackageConfig(
test_command=test_command,
identifier=job_identifier,
_targets=["test-target", "another-test-target"],
)
},
)
metadata = flexmock(event_dict={"comment": comment})

git_project = flexmock()

helper = TFJobHelper(
service_config=flexmock(comment_command_prefix="/packit-dev"),
package_config=flexmock(jobs=[]),
project=git_project,
metadata=metadata,
db_project_event=flexmock()
.should_receive("get_project_event_object")
.and_return(flexmock(job_config_trigger_type=JobConfigTriggerType.pull_request))
.mock(),
job_config=job_config,
)

flexmock(helper).should_receive("_report").times(show_status_check_times)
helper.report_status_to_tests("a description", "a new status")

0 comments on commit cef6943

Please sign in to comment.