Skip to content

Commit

Permalink
Use same handling for comment events as for other events
Browse files Browse the repository at this point in the history
Signed-off-by: Frantisek Lachman <flachman@redhat.com>
  • Loading branch information
lachmanfrantisek committed Mar 10, 2020
1 parent 879ad71 commit 40021fd
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 66 deletions.
10 changes: 9 additions & 1 deletion packit_service/service/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,16 @@ def __init__(
self.base_repo_namespace = build.get("repo_namespace")
https_url = build["https_url"]

super().__init__(trigger=TheJobTriggerType.pull_request, project_url=https_url)
self.topic = FedmsgTopic(topic)
if self.topic == FedmsgTopic.copr_build_started:
trigger = TheJobTriggerType.copr_start
elif self.topic == FedmsgTopic.copr_build_finished:
trigger = TheJobTriggerType.copr_end
else:
raise ValueError(f"Unknown topic for CoprEvent: '{self.topic}'")

super().__init__(trigger=trigger, project_url=https_url)

self.build_id = build_id
self.build = build
self.chroot = chroot
Expand Down
2 changes: 1 addition & 1 deletion packit_service/worker/handlers/comment_action_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class CommentAction(enum.Enum):


def add_to_comment_action_mapping(kls: Type["CommentActionHandler"]):
COMMENT_ACTION_HANDLER_MAPPING[kls.type] = kls
# COMMENT_ACTION_HANDLER_MAPPING[kls.type] = kls
return kls


Expand Down
30 changes: 16 additions & 14 deletions packit_service/worker/handlers/github_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,6 @@
add_alias,
use_for,
)
from packit_service.worker.handlers.comment_action_handler import (
add_to_comment_action_mapping,
add_to_comment_action_mapping_with_name,
CommentAction,
)
from packit_service.worker.result import HandlerResults
from packit_service.worker.testing_farm import TestingFarmJobHelper
from packit_service.worker.whitelist import Whitelist
Expand Down Expand Up @@ -458,21 +453,27 @@ def run(self) -> HandlerResults:
return self.testing_farm_helper.run_testing_farm(chroot=self.chroot)


@add_to_comment_action_mapping
@add_to_comment_action_mapping_with_name(name=CommentAction.build)
@add_to_mapping
@use_for(job_type=JobType.copr_build)
@required_by(job_type=JobType.tests)
class GitHubPullRequestCommentCoprBuildHandler(
CommentActionHandler, GithubPackageConfigGetter
):
""" Handler for PR comment `/packit copr-build` """

type = CommentAction.copr_build
triggers = [TheJobTriggerType.pr_comment]
event: PullRequestCommentEvent

def __init__(self, config: ServiceConfig, event: PullRequestCommentEvent):
def __init__(
self,
config: ServiceConfig,
job_config: JobConfig,
event: PullRequestCommentEvent,
):
super().__init__(config=config, event=event)
self.config = config
self.event = event
self.job_config = job_config
self.project: GitProject = event.get_project()
# Get the latest pull request commit
self.event.commit_sha = self.project.get_all_pr_commits(self.event.pr_id)[-1]
Expand All @@ -498,13 +499,14 @@ def run(self) -> HandlerResults:
return handler_results


@add_to_comment_action_mapping
@add_to_mapping
@use_for(job_type=JobType.propose_downstream)
class GitHubIssueCommentProposeUpdateHandler(
CommentActionHandler, GithubPackageConfigGetter
):
""" Handler for issue comment `/packit propose-update` """

type = CommentAction.propose_update
triggers = [TheJobTriggerType.issue_comment]
event: IssueCommentEvent

def __init__(self, config: ServiceConfig, event: IssueCommentEvent):
Expand Down Expand Up @@ -606,15 +608,15 @@ def run(self) -> HandlerResults:
return HandlerResults(success=True, details={})


@add_to_comment_action_mapping
@add_to_mapping
@use_for(job_type=JobType.tests)
class GitHubPullRequestCommentTestingFarmHandler(
CommentActionHandler, GithubPackageConfigGetter
):
""" Issue handler for comment `/packit test` """

type = CommentAction.test
event: PullRequestCommentEvent
trgggers = [TheJobTriggerType.pr_comment]
triggers = [TheJobTriggerType.pr_comment]

def __init__(self, config: ServiceConfig, event: PullRequestCommentEvent):
super().__init__(config=config, event=event)
Expand Down
42 changes: 7 additions & 35 deletions packit_service/worker/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@
PullRequestCommentEvent,
IssueCommentEvent,
Event,
TestingFarmResultsEvent,
CoprBuildEvent,
FedmsgTopic,
TheJobTriggerType,
)
from packit_service.trigger_mapping import is_trigger_matching_job_config
Expand Down Expand Up @@ -104,6 +101,12 @@ def get_config_for_handler_kls(
handler_kls
] and is_trigger_matching_job_config(trigger=event.trigger, job_config=job):
return job

required_handlers = JOB_REQUIRED_MAPPING[job.type]
for pos_handler in required_handlers:
for trigger in pos_handler.triggers:
if trigger == event.trigger:
return job
return None


Expand Down Expand Up @@ -170,7 +173,7 @@ def process_jobs(self, event: Event) -> Dict[str, HandlerResults]:
return handlers_results

logger.debug(f"Running handler: {str(handler_kls)}")
handler = handler_kls(self.config, job, event)
handler = handler_kls(config=self.config, job_config=job, event=event)
if handler.pre_check():
handlers_results[job.type.value] = handler.run_n_clean()
# don't break here, other handlers may react to the same event
Expand Down Expand Up @@ -301,37 +304,6 @@ def process_message(self, event: dict, topic: str = None) -> Optional[dict]:
)
job_type = JobType.add_to_whitelist.value
jobs_results[job_type] = handler.run_n_clean()
# Results from testing farm is another job which is not defined in packit.yaml so
# it needs to be handled outside process_jobs method
elif (
event_object.trigger == TheJobTriggerType.testing_farm_results
and isinstance(event_object, TestingFarmResultsEvent)
):
handler = TestingFarmResultsHandler(
self.config, job_config=None, test_results_event=event_object
)
job_type = JobType.report_test_results.value
jobs_results[job_type] = handler.run_n_clean()
elif isinstance(event_object, CoprBuildEvent):
if event_object.topic == FedmsgTopic.copr_build_started:
handler = CoprBuildStartHandler(
self.config, job_config=None, event=event_object
)
job_type = JobType.copr_build_started.value
elif event_object.topic == FedmsgTopic.copr_build_finished:
handler = CoprBuildEndHandler(
self.config, job_config=None, event=event_object
)
job_type = JobType.copr_build_finished.value
else:
raise ValueError(f"Unknown topic {event_object.topic}")
jobs_results[job_type] = handler.run_n_clean()
elif event_object.trigger in {
TheJobTriggerType.issue_comment,
TheJobTriggerType.pr_comment,
} and (isinstance(event_object, (PullRequestCommentEvent, IssueCommentEvent))):
job_type = JobType.pull_request_action.value
jobs_results[job_type] = self.process_comment_jobs(event_object)
else:
# Processing the jobs from the config.
jobs_results = self.process_jobs(event_object)
Expand Down
21 changes: 16 additions & 5 deletions tests/integration/test_issue_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@

import pytest
from flexmock import flexmock
from tests.spellbook import DATA_DIR

from ogr.abstract import PullRequest, PRStatus
from ogr.services.github.project import GithubProject
from packit.api import PackitAPI
from packit.config import JobConfig, JobType, JobConfigTriggerType

from packit_service.service.events import IssueCommentEvent
from packit_service.worker.jobs import SteveJobs
from tests.spellbook import DATA_DIR


@pytest.fixture()
Expand All @@ -56,9 +58,18 @@ def test_issue_comment_propose_update_handler(
created=datetime.now(),
)
)
flexmock(
GithubProject, get_files=lambda filter_regex: [],
)
flexmock(GithubProject, get_files=lambda filter_regex: [])
flexmock(SteveJobs, _is_private=False)
flexmock(IssueCommentEvent).should_receive("get_package_config").and_return(
flexmock(
jobs=[
JobConfig(
type=JobType.propose_downstream,
trigger=JobConfigTriggerType.release,
metadata={},
)
]
)
)
results = SteveJobs().process_message(issue_comment_propose_update_event)
assert results["jobs"]["pull_request_action"]["success"]
64 changes: 54 additions & 10 deletions tests/integration/test_pr_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@
import pytest
from flexmock import flexmock
from ogr.services.github import GithubProject
from packit.config import JobConfig, JobType, JobConfigTriggerType

from packit_service.service.events import PullRequestCommentEvent
from packit_service.worker.build.copr_build import CoprBuildJobHelper
from packit_service.worker.result import HandlerResults
from packit_service.worker.jobs import SteveJobs
from packit_service.worker.result import HandlerResults
from tests.spellbook import DATA_DIR


Expand Down Expand Up @@ -79,6 +81,17 @@ def pr_wrong_packit_comment_event():
def test_pr_comment_copr_build_handler(
mock_pr_comment_functionality, pr_copr_build_comment_event
):
flexmock(PullRequestCommentEvent).should_receive("get_package_config").and_return(
flexmock(
jobs=[
JobConfig(
type=JobType.copr_build,
trigger=JobConfigTriggerType.pull_request,
metadata={},
)
]
)
)
flexmock(CoprBuildJobHelper).should_receive("run_copr_build").and_return(
HandlerResults(success=True, details={})
).once()
Expand All @@ -91,12 +104,23 @@ def test_pr_comment_copr_build_handler(
flexmock(GithubProject, get_files="foo.spec")
flexmock(SteveJobs, _is_private=False)
results = SteveJobs().process_message(pr_copr_build_comment_event)
assert results["jobs"]["pull_request_action"]["success"]
assert results["jobs"]["copr_build"]["success"]


def test_pr_comment_build_handler(
mock_pr_comment_functionality, pr_build_comment_event
):
flexmock(PullRequestCommentEvent).should_receive("get_package_config").and_return(
flexmock(
jobs=[
JobConfig(
type=JobType.copr_build,
trigger=JobConfigTriggerType.pull_request,
metadata={},
)
]
)
)
flexmock(CoprBuildJobHelper).should_receive("run_copr_build").and_return(
HandlerResults(success=True, details={})
)
Expand All @@ -109,7 +133,7 @@ def test_pr_comment_build_handler(
flexmock(GithubProject, get_files="foo.spec")
flexmock(SteveJobs, _is_private=False)
results = SteveJobs().process_message(pr_build_comment_event)
assert results["jobs"]["pull_request_action"]["success"]
assert results["jobs"]["copr_build"]["success"]


@pytest.mark.parametrize(
Expand Down Expand Up @@ -149,7 +173,17 @@ def test_pr_comment_invalid(comment):
def test_pr_embedded_command_handler(
mock_pr_comment_functionality, pr_embedded_command_comment_event, comments_list
):

flexmock(PullRequestCommentEvent).should_receive("get_package_config").and_return(
flexmock(
jobs=[
JobConfig(
type=JobType.copr_build,
trigger=JobConfigTriggerType.pull_request,
metadata={},
)
]
)
)
pr_embedded_command_comment_event["comment"]["body"] = comments_list
flexmock(CoprBuildJobHelper).should_receive("run_copr_build").and_return(
HandlerResults(success=True, details={})
Expand All @@ -163,18 +197,28 @@ def test_pr_embedded_command_handler(
flexmock(GithubProject, get_files="foo.spec")
flexmock(SteveJobs, _is_private=False)
results = SteveJobs().process_message(pr_embedded_command_comment_event)
assert results["jobs"]["pull_request_action"]["success"]
assert results["jobs"]["copr_build"]["success"]


def test_pr_comment_empty_handler(
mock_pr_comment_functionality, pr_empty_comment_event
):
flexmock(PullRequestCommentEvent).should_receive("get_package_config").and_return(
flexmock(
jobs=[
JobConfig(
type=JobType.copr_build,
trigger=JobConfigTriggerType.pull_request,
metadata={},
)
]
)
)
flexmock(SteveJobs, _is_private=False)

results = SteveJobs().process_message(pr_empty_comment_event)
assert results["jobs"]["pull_request_action"]["success"]
assert results["jobs"]["copr_build"]["success"]
msg = "comment '' is empty."
assert results["jobs"]["pull_request_action"]["details"]["msg"] == msg
assert results["jobs"]["copr_build"]["details"]["msg"] == msg


def test_pr_comment_packit_only_handler(
Expand All @@ -183,9 +227,9 @@ def test_pr_comment_packit_only_handler(
flexmock(SteveJobs, _is_private=False)

results = SteveJobs().process_message(pr_packit_only_comment_event)
assert results["jobs"]["pull_request_action"]["success"]
assert results["jobs"]["copr_build"]["success"]
msg = "comment '/packit' does not contain a packit-service command."
assert results["jobs"]["pull_request_action"]["details"]["msg"] == msg
assert results["jobs"]["copr_build"]["details"]["msg"] == msg


def test_pr_comment_wrong_packit_command_handler(
Expand Down
26 changes: 26 additions & 0 deletions tests/unit/test_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
ProposeDownstreamHandler,
CoprBuildStartHandler,
CoprBuildEndHandler,
GitHubPullRequestCommentCoprBuildHandler,
GitHubIssueCommentProposeUpdateHandler,
)
from packit_service.worker.jobs import get_handlers_for_event

Expand Down Expand Up @@ -203,6 +205,30 @@
{CoprBuildStartHandler},
id="config=tests_and_copr_build@trigger=copr_start",
),
pytest.param(
TheJobTriggerType.pr_comment,
[
JobConfig(
type=JobType.copr_build,
trigger=JobConfigTriggerType.pull_request,
metadata={},
),
],
{GitHubPullRequestCommentCoprBuildHandler},
id="config=copr_build@trigger=pr_comment",
),
pytest.param(
TheJobTriggerType.issue_comment,
[
JobConfig(
type=JobType.propose_downstream,
trigger=JobConfigTriggerType.pull_request,
metadata={},
),
],
{GitHubIssueCommentProposeUpdateHandler},
id="config=propose_downstream@trigger=issue_comment",
),
],
)
def test_get_handlers_for_event(trigger, jobs, result):
Expand Down

0 comments on commit 40021fd

Please sign in to comment.