Skip to content

Commit

Permalink
forward repository and job id instead of job_url
Browse files Browse the repository at this point in the history
  • Loading branch information
cbartz committed Sep 17, 2024
1 parent aa97810 commit 414d210
Show file tree
Hide file tree
Showing 7 changed files with 252 additions and 69 deletions.
43 changes: 41 additions & 2 deletions src-docs/parse.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Module for parsing the webhook payload.

---

<a href="../webhook_router/parse.py#L50"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../webhook_router/parse.py#L64"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

## <kbd>function</kbd> `webhook_to_job`

Expand Down Expand Up @@ -39,6 +39,44 @@ Parse a raw json payload and extract the required information.
- <b>`ParseError`</b>: An error occurred during parsing.


---

## <kbd>class</kbd> `GitHubRepo`
A class to represent the GitHub repository.



**Attributes:**

- <b>`owner`</b>: The owner of the repository.
- <b>`name`</b>: The name of the repository.


---

#### <kbd>property</kbd> model_extra

Get extra fields set during validation.



**Returns:**
A dictionary of extra fields, or `None` if `config.extra` is not set to `"allow"`.

---

#### <kbd>property</kbd> model_fields_set

Returns the set of fields that have been explicitly set on this model instance.



**Returns:**
A set of strings representing the fields that have been set, i.e. that were not filled from defaults.




---

## <kbd>class</kbd> `Job`
Expand All @@ -50,7 +88,8 @@ A class to translate the payload.

- <b>`labels`</b>: The labels of the job.
- <b>`status`</b>: The status of the job.
- <b>`url`</b>: The URL of the job to be able to check its status.
- <b>`repository`</b>: The repository of the job.
- <b>`id`</b>: The id of the job.


---
Expand Down
15 changes: 13 additions & 2 deletions tests/integration/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
SUPPORTED_GITHUB_EVENT,
WEBHOOK_SIGNATURE_HEADER,
)
from webhook_router.parse import Job, JobStatus
from webhook_router.parse import GitHubRepo, Job, JobStatus

PORT = 8000

Expand Down Expand Up @@ -91,8 +91,12 @@ async def test_forward_webhook( # pylint: disable=too-many-locals
flavour: [
Job(
status=payload["action"],
url=payload["workflow_job"]["url"],
id=payload["workflow_job"]["id"],
labels=payload["workflow_job"]["labels"],
repository=GitHubRepo(
name=payload["repository"]["name"],
owner=payload["repository"]["owner"]["login"],
),
)
for payload in payloads_by_flavour[flavour]
]
Expand Down Expand Up @@ -237,6 +241,13 @@ def _create_valid_data(action: str, labels: list[str]) -> dict:
"labels": labels,
"url": f"https://api.github.com/repos/f/actions/jobs/{_id}",
},
"repository": {
"id": 123456789,
"name": "gh-runner-test",
"full_name": "f/gh-runner-test",
"private": False,
"owner": {"login": "f", "id": 123456},
},
}


Expand Down
11 changes: 9 additions & 2 deletions tests/unit/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import webhook_router.app as app_module
import webhook_router.router
from tests.unit.helpers import create_correct_signature, create_incorrect_signature
from webhook_router.parse import Job, JobStatus, ParseError
from webhook_router.parse import GitHubRepo, Job, JobStatus, ParseError
from webhook_router.router import RouterError, RoutingTable

TEST_PATH = "/webhook"
Expand Down Expand Up @@ -111,7 +111,10 @@ def test_webhook_logs(
expected_job = Job(
labels=data["workflow_job"]["labels"],
status=JobStatus.QUEUED,
url=data["workflow_job"]["url"],
id=data["workflow_job"]["id"],
repository=GitHubRepo(
owner=data["repository"]["owner"]["login"], name=data["repository"]["name"]
),
)
response = client.post(
TEST_PATH,
Expand Down Expand Up @@ -395,4 +398,8 @@ def _create_valid_data(action: str) -> dict:
"labels": ["self-hosted", "linux", "arm64"],
"url": "https://api.github.com/repos/f/actions/jobs/8200803099",
},
"repository": {
"name": "actions",
"owner": {"login": "f"},
},
}
10 changes: 7 additions & 3 deletions tests/unit/test_mq.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from kombu.exceptions import OperationalError

from webhook_router import mq
from webhook_router.parse import Job, JobStatus
from webhook_router.parse import GitHubRepo, Job, JobStatus

IN_MEMORY_URI = "memory://"

Expand All @@ -30,8 +30,12 @@ def test_add_job_to_queue():
"""
flavor = secrets.token_hex(16)
labels = [secrets.token_hex(16), secrets.token_hex(16)]
# mypy: does not recognize that url can be passed as a string
job = Job(labels=labels, status=JobStatus.QUEUED, url="http://example.com") # type: ignore
job = Job(
labels=labels,
status=JobStatus.QUEUED,
repository=GitHubRepo(owner="fake", name="gh-runner"),
id=1234,
)
mq.add_job_to_queue(job, flavor)

with Connection(IN_MEMORY_URI) as conn:
Expand Down
131 changes: 109 additions & 22 deletions tests/unit/test_parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@

import pytest

from webhook_router.parse import Job, JobStatus, ParseError, webhook_to_job
from webhook_router.parse import GitHubRepo, Job, JobStatus, ParseError, webhook_to_job

FAKE_JOB_URL = "https://api.github.com/repos/fakeusergh-runner-test/actions/jobs/8200803099"
FAKE_LABELS = ["self-hosted", "linux", "arm64"]
FAKE_JOB_ID = 22428484402
FAKE_OWNER = "fake"
FAKE_REPO_NAME = "gh-runner-test"


@pytest.mark.parametrize(
Expand All @@ -30,7 +33,7 @@ def test_webhook_to_job(labels: list[str], status: JobStatus):
payload = {
"action": status,
"workflow_job": {
"id": 22428484402,
"id": FAKE_JOB_ID,
"run_id": 8200803099,
"workflow_name": "Push Event Tests",
"head_branch": "github-hosted",
Expand All @@ -54,38 +57,46 @@ def test_webhook_to_job(labels: list[str], status: JobStatus):
"runner_group_id": None,
"runner_group_name": None,
},
"repository": {
"id": 688069339,
"name": "gh-runner-test",
"full_name": f"{FAKE_OWNER}/{FAKE_REPO_NAME}",
"private": False,
"owner": {
"login": FAKE_OWNER,
"id": 4183921,
},
},
}

result = webhook_to_job(payload)

# mypy does not understand that we can pass strings instead of HttpUrl objects
# because of the underlying pydantic magic
assert result == Job(labels=labels, status=status, url=FAKE_JOB_URL) # type: ignore
assert result == Job(
labels=labels,
status=status,
id=FAKE_JOB_ID,
repository=GitHubRepo(owner=FAKE_OWNER, name=FAKE_REPO_NAME),
)


@pytest.mark.parametrize(
"labels, status, url",
"labels, status",
[
pytest.param(
["self-hosted", "linux", "arm64"], "invalid", FAKE_JOB_URL, id="invalid status"
),
pytest.param(["ubuntu-latest"], JobStatus.IN_PROGRESS, "invalid", id="invalid run url"),
pytest.param(None, JobStatus.IN_PROGRESS, FAKE_JOB_URL, id="missing labels"),
pytest.param(["self-hosted", "linux", "amd"], None, FAKE_JOB_URL, id="missing status"),
pytest.param(
["self-hosted", "linux", "amd"], JobStatus.COMPLETED, None, id="missing run url"
),
pytest.param(["self-hosted", "linux", "arm64"], "invalid", id="invalid status"),
pytest.param(None, JobStatus.IN_PROGRESS, id="missing labels"),
pytest.param(["self-hosted", "linux", "amd"], None, id="missing status"),
],
)
def test_webhook_invalid_values(labels: list[str], status: JobStatus, url: str):
def test_webhook_invalid_values(labels: list[str], status: JobStatus):
"""
arrange: A payload dict with invalid values.
act: Call webhook_to_job with the payload.
assert: A ParseError is raised.
"""
payload = {
"action": status,
"workflow_job": {"id": 22428484402, "url": url, "labels": labels},
"workflow_job": {"id": 22428484402, "labels": labels},
"repository": {"name": "gh-runner-test", "owner": {"login": "fakeuser"}},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
Expand All @@ -100,13 +111,46 @@ def test_webhook_workflow_job_not_dict():
"""
payload = {
"action": "queued",
"workflow_job": "not a dict",
"workflow_job": "labels id",
"repository": {"name": FAKE_REPO_NAME, "owner": {"login": FAKE_REPO_NAME}},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
assert f"workflow_job is not a dict in {payload}" in str(exc_info.value)


def test_webhook_repository_not_dict():
"""
arrange: A payload dict with repository not a dict.
act: Call webhook_to_job with the payload.
assert: A ParseError is raised.
"""
payload = {
"action": "queued",
"workflow_job": {"id": 22428484402, "labels": FAKE_LABELS},
"repository": "name owner login",
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
assert f"repository is not a dict in {payload}" in str(exc_info.value)


def test_webhook_repository_owner_not_dict():
"""
arrange: A payload dict with repository owner not a dict.
act: Call webhook_to_job with the payload.
assert: A ParseError is raised.
"""
payload = {
"action": "queued",
"workflow_job": {"id": 22428484402, "labels": FAKE_LABELS},
"repository": {"name": "gh-runner-test", "owner": "not a dict"},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
assert f"owner is not a dict in {payload['repository']}" in str(exc_info.value)


def test_webhook_missing_keys():
"""
arrange: A payload dict with missing keys.
Expand All @@ -119,6 +163,7 @@ def test_webhook_missing_keys():
payload = {
"payload": {
"workflow_job": {"id": 22428484402, "url": FAKE_JOB_URL, "labels": FAKE_LABELS},
"repository": {"name": FAKE_REPO_NAME, "owner": {"login": FAKE_OWNER}},
},
}
with pytest.raises(ParseError) as exc_info:
Expand All @@ -138,17 +183,59 @@ def test_webhook_missing_keys():
# labels key missing
payload = {
"action": "queued",
"workflow_job": {"id": 22428484402, "url": FAKE_JOB_URL},
"workflow_job": {"id": 22428484402},
"repository": {"name": FAKE_REPO_NAME, "owner": {"login": FAKE_OWNER}},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
assert f"labels key not found in {payload['workflow_job']}" in str(exc_info.value)

# id key missing
payload = {
"action": "queued",
"workflow_job": {"labels": FAKE_LABELS},
"repository": {"name": FAKE_REPO_NAME, "owner": {"login": FAKE_OWNER}},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
assert f"id key not found in {payload['workflow_job']}" in str(exc_info.value)

# repository key missing
payload = {
"action": "queued",
"workflow_job": {"id": 22428484402, "labels": FAKE_LABELS},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
assert f"repository key not found in {payload}" in str(exc_info.value)

# owner key missing
payload = {
"action": "queued",
"workflow_job": {"id": 22428484402, "labels": FAKE_LABELS},
"repository": {"name": FAKE_REPO_NAME},
}

with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
assert f"owner key not found in {payload['repository']}" in str(exc_info.value)

# name key missing
payload = {
"action": "queued",
"workflow_job": {"id": 22428484402, "labels": FAKE_LABELS},
"repository": {"owner": {"login": FAKE_OWNER}},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
assert f"labels key not found in {payload}" in str(exc_info.value)
assert f"name key not found in {payload['repository']}" in str(exc_info.value)

# url key missing
# login key missing
payload = {
"action": "queued",
"workflow_job": {"id": 22428484402, "labels": FAKE_LABELS},
"repository": {"name": FAKE_REPO_NAME, "owner": {}},
}
with pytest.raises(ParseError) as exc_info:
webhook_to_job(payload)
assert f"url key not found in {payload}" in str(exc_info.value)
assert f"login key not found in {payload['repository']['owner']}" in str(exc_info.value)
12 changes: 5 additions & 7 deletions tests/unit/test_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import pytest

from webhook_router.mq import add_job_to_queue
from webhook_router.parse import Job, JobStatus
from webhook_router.parse import GitHubRepo, Job, JobStatus
from webhook_router.router import (
RouterError,
RoutingTable,
Expand Down Expand Up @@ -47,12 +47,11 @@ def test_job_is_forwarded(
act: Forward the job to the message queue.
assert: The job is added to the queue if the status is "QUEUED".
"""
# mypy does not understand that we can pass strings instead of HttpUrl objects
# because of the underlying pydantic magic
job = Job(
labels=["arm64"],
status=job_status,
url="https://api.github.com/repos/f/actions/jobs/8200803099", # type: ignore
id=22428484402,
repository=GitHubRepo(**{"owner": "fake", "name": "gh-runner-test"}),
)
forward(
job,
Expand All @@ -70,12 +69,11 @@ def test_invalid_label_combination():
act: Forward the job to the message queue.
assert: A RouterError is raised.
"""
# mypy does not understand that we can pass strings instead of HttpUrl objects
# because of the underlying pydantic magic
job = Job(
labels=["self-hosted", "linux", "arm64", "x64"],
status=JobStatus.QUEUED,
url="https://api.github.com/repos/f/actions/jobs/8200803099", # type: ignore
id=22428484402,
repository=GitHubRepo(**{"owner": "fake", "name": "gh-runner-test"}),
)
with pytest.raises(RouterError) as e:
forward(
Expand Down
Loading

0 comments on commit 414d210

Please sign in to comment.