Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion labelbox/schema/webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,15 @@ def create(client, topics, url, secret, project) -> "Webhook":
ValueError: If the topic is not one of Topic or status is not one of Status

Information on configuring your server can be found here (this is where the url points to and the secret is set).
https://docs.labelbox.com/en/configure-editor/webhooks-setup#setup-steps
https://docs.labelbox.com/reference/webhook
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little out of scope, but this docs link is broken.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!


"""
if not secret:
raise ValueError("Secret must be a non-empty string.")
if not topics:
raise ValueError("Topics must be a non-empty list.")
if not url:
raise ValueError("URL must be a non-empty string.")
Webhook.validate_topics(topics)

project_str = "" if project is None \
Expand Down
36 changes: 36 additions & 0 deletions tests/integration/test_webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,39 @@ def test_webhook_create_update(project, rand_gen):
"Topics must be List[Webhook.Topic]. Found `invalid..`"

webhook.delete()


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These, in my opinion, should be considered unit-tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of tests in integration that could be better considered a unit test, but this needs a configured project and client which only exist in integration fixtures, and pulling them out would be too much of a pain for this I think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

client and project could be mocks, the exceptions are thrown before these are used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I moved them to unit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second Klaus comments, after that can approve

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be fixed now and checks are passing, can I get an approval?

def test_webhook_create_with_no_secret(project, rand_gen):
client = project.client
secret = ""
url = "https:/" + rand_gen(str)
topics = []

with pytest.raises(ValueError) as exc_info:
Webhook.create(client, topics, url, secret, project)
assert str(exc_info.value) == \
"Secret must be a non-empty string."


def test_webhook_create_with_no_topics(project, rand_gen):
client = project.client
secret = rand_gen(str)
url = "https:/" + rand_gen(str)
topics = []

with pytest.raises(ValueError) as exc_info:
Webhook.create(client, topics, url, secret, project)
assert str(exc_info.value) == \
"Topics must be a non-empty list."


def test_webhook_create_with_no_url(project, rand_gen):
client = project.client
secret = rand_gen(str)
url = ""
topics = [Webhook.LABEL_CREATED, Webhook.LABEL_DELETED]

with pytest.raises(ValueError) as exc_info:
Webhook.create(client, topics, url, secret, project)
assert str(exc_info.value) == \
"URL must be a non-empty string."
43 changes: 43 additions & 0 deletions tests/unit/test_unit_webhook.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
from unittest.mock import MagicMock
import pytest

from labelbox import Webhook


def test_webhook_create_with_no_secret(rand_gen):
client = MagicMock()
project = MagicMock()
secret = ""
url = "https:/" + rand_gen(str)
topics = []

with pytest.raises(ValueError) as exc_info:
Webhook.create(client, topics, url, secret, project)
assert str(exc_info.value) == \
"Secret must be a non-empty string."


def test_webhook_create_with_no_topics(rand_gen):
client = MagicMock()
project = MagicMock()
secret = rand_gen(str)
url = "https:/" + rand_gen(str)
topics = []

with pytest.raises(ValueError) as exc_info:
Webhook.create(client, topics, url, secret, project)
assert str(exc_info.value) == \
"Topics must be a non-empty list."


def test_webhook_create_with_no_url(rand_gen):
client = MagicMock()
project = MagicMock()
secret = rand_gen(str)
url = ""
topics = [Webhook.Topic.LABEL_CREATED, Webhook.Topic.LABEL_DELETED]

with pytest.raises(ValueError) as exc_info:
Webhook.create(client, topics, url, secret, project)
assert str(exc_info.value) == \
"URL must be a non-empty string."