From b1293a84f88ca76de450751482ca272b80562f7d Mon Sep 17 00:00:00 2001 From: Adam Sah <140002+asah@users.noreply.github.com> Date: Wed, 18 May 2022 05:15:19 -0400 Subject: [PATCH] testing: 100% coverage for zerver/webhooks/{librato,pivotal}. --- tools/test-backend | 2 - zerver/webhooks/librato/fixtures/bad.json | 2 + .../librato/fixtures/bad_msg_type.json | 56 +++++++++++++++++++ zerver/webhooks/librato/tests.py | 20 +++++++ zerver/webhooks/librato/view.py | 4 +- .../pivotal/fixtures/bad_accepted.xml | 21 +++++++ .../pivotal/fixtures/v5_bad_kind.json | 47 ++++++++++++++++ .../pivotal/fixtures/v5_bad_request.json | 16 ++++++ .../pivotal/fixtures/v5_unsupported.json | 47 ++++++++++++++++ zerver/webhooks/pivotal/tests.py | 36 ++++++++++++ 10 files changed, 247 insertions(+), 4 deletions(-) create mode 100644 zerver/webhooks/librato/fixtures/bad.json create mode 100644 zerver/webhooks/librato/fixtures/bad_msg_type.json create mode 100644 zerver/webhooks/pivotal/fixtures/bad_accepted.xml create mode 100644 zerver/webhooks/pivotal/fixtures/v5_bad_kind.json create mode 100644 zerver/webhooks/pivotal/fixtures/v5_bad_request.json create mode 100644 zerver/webhooks/pivotal/fixtures/v5_unsupported.json diff --git a/tools/test-backend b/tools/test-backend index 6a28eb72e0..a670765c1a 100755 --- a/tools/test-backend +++ b/tools/test-backend @@ -134,8 +134,6 @@ not_yet_fully_covered = [ # Webhook integrations with incomplete coverage "zerver/webhooks/greenhouse/view.py", "zerver/webhooks/jira/view.py", - "zerver/webhooks/librato/view.py", - "zerver/webhooks/pivotal/view.py", "zerver/webhooks/solano/view.py", "zerver/webhooks/teamcity/view.py", "zerver/webhooks/travis/view.py", diff --git a/zerver/webhooks/librato/fixtures/bad.json b/zerver/webhooks/librato/fixtures/bad.json new file mode 100644 index 0000000000..59ddddafd5 --- /dev/null +++ b/zerver/webhooks/librato/fixtures/bad.json @@ -0,0 +1,2 @@ +{ + "alert": { diff --git a/zerver/webhooks/librato/fixtures/bad_msg_type.json b/zerver/webhooks/librato/fixtures/bad_msg_type.json new file mode 100644 index 0000000000..eb84b6ea17 --- /dev/null +++ b/zerver/webhooks/librato/fixtures/bad_msg_type.json @@ -0,0 +1,56 @@ +{ + "alert": { + "id": 6294535, + "name": "alert.name", + "runbook_url": "http://www.google.pl", + "version": 2, + "description": "descrription" + }, + "account": "lizonr@gmail.com", + "trigger_time": 1459415502, + "conditions": [ + { + "id": 3543146, + "type": "below", + "threshold": 44, + "summary_function": "sum", + "duration": 300 + }, + { + "id": 3651148, + "type": "absent", + "summary_function": "average", + "duration": 300 + }, + { + "id": 3651902, + "type": "above", + "threshold": 9, + "summary_function": "derivative", + "duration": 300 + } + ], + "violations2": { + "test-source": [ + { + "metric": "librato.cpu.percent.idle", + "value": 2, + "recorded_at": 1459415502, + "condition_violated": 3543146 + }, + { + "metric": "librato.swap.swap.cached", + "value": 42, + "recorded_at": 1459415502, + "condition_violated": 3651148 + }, + { + "metric": "librato.swap.swap.cached", + "value": 51, + "recorded_at": 1459415502, + "condition_violated": 3651902 + } + ] + }, + "triggered_by_user_test": true +} diff --git a/zerver/webhooks/librato/tests.py b/zerver/webhooks/librato/tests.py index 7a146da107..077eae67e0 100644 --- a/zerver/webhooks/librato/tests.py +++ b/zerver/webhooks/librato/tests.py @@ -68,3 +68,23 @@ class LibratoHookTests(WebhookTestCase): content_type="application/x-www-form-urlencoded", ) self.IS_ATTACHMENT = False + + def test_bad_request(self) -> None: + with self.assertRaises(AssertionError) as e: + self.check_webhook( + "bad", + "", + "", + content_type="application/json", + ) + self.assertIn("Malformed JSON input", e.exception.args[0]) + + def test_bad_msg_type(self) -> None: + with self.assertRaises(AssertionError) as e: + self.check_webhook( + "bad_msg_type", + "", + "", + content_type="application/x-www-form-urlencoded", + ) + self.assertIn("Unexpected message type", e.exception.args[0]) diff --git a/zerver/webhooks/librato/view.py b/zerver/webhooks/librato/view.py index e40158562e..3a71d7c7da 100644 --- a/zerver/webhooks/librato/view.py +++ b/zerver/webhooks/librato/view.py @@ -81,9 +81,9 @@ class LibratoWebhookHandler(LibratoWebhookParser): if self.payload.get(available_type): return self.payload_available_types[available_type] for available_type in self.attachments_available_types: - if self.attachments[0].get(available_type): + if len(self.attachments) > 0 and self.attachments[0].get(available_type): return self.attachments_available_types[available_type] - raise Exception("Unexcepted message type") + raise Exception("Unexpected message type") def handle(self) -> str: return self.find_handle_method()() diff --git a/zerver/webhooks/pivotal/fixtures/bad_accepted.xml b/zerver/webhooks/pivotal/fixtures/bad_accepted.xml new file mode 100644 index 0000000000..ff5d1f939b --- /dev/null +++ b/zerver/webhooks/pivotal/fixtures/bad_accepted.xml @@ -0,0 +1,21 @@ + + + 346167313 + 11 + story_update + 2013/04/17 20:18:54 UTC + Leo Franchi + 807213 + Leo Franchi accepted My new Feature story + + + 48276573 + http://www.pivotaltracker.com/services/v3/projects/807213/stories/48276573 + bug + 2013/04/17 20:18:54 UTC + website + accepted + Leo Franchi + + + diff --git a/zerver/webhooks/pivotal/fixtures/v5_bad_kind.json b/zerver/webhooks/pivotal/fixtures/v5_bad_kind.json new file mode 100644 index 0000000000..deab206cc7 --- /dev/null +++ b/zerver/webhooks/pivotal/fixtures/v5_bad_kind.json @@ -0,0 +1,47 @@ +{ + "occurred_at": 1389218398000, + "kind": "unknown_kind", + "project": { + "name": "Hard Code", + "kind": "project", + "id": 807213 + }, + "changes": [ + { + "name": "Story of the Year", + "story_type": "feature", + "change_type": "update", + "kind": "story", + "new_values": { + "current_state": "accepted", + "updated_at": 1389218398000, + "accepted_at": 1389218397000 + }, + "id": 63486316, + "original_values": { + "current_state": "unstarted", + "updated_at": 1389215951000, + "accepted_at": null + } + } + ], + "highlight": "accepted", + "project_version": 60, + "guid": "807213_60", + "primary_resources": [ + { + "name": "Story of the Year", + "story_type": "feature", + "kind": "story", + "id": 63486316, + "url": "http://www.pivotaltracker.com/story/show/63486316" + } + ], + "performed_by": { + "name": "Leo Franchi", + "initials": "LF", + "kind": "person", + "id": 981905 + }, + "message": "Leo Franchi accepted this feature" +} diff --git a/zerver/webhooks/pivotal/fixtures/v5_bad_request.json b/zerver/webhooks/pivotal/fixtures/v5_bad_request.json new file mode 100644 index 0000000000..ea33ef0114 --- /dev/null +++ b/zerver/webhooks/pivotal/fixtures/v5_bad_request.json @@ -0,0 +1,16 @@ +{ + "occurred_at": 1389218398000, + "kind": "epic_update_activity", + "project": { + "name": "", + "id": "" + }, + "primary_resources": [ + { + "url": "", + "id": 0, + "name": "" + } + ], + "msg": "foo" +} diff --git a/zerver/webhooks/pivotal/fixtures/v5_unsupported.json b/zerver/webhooks/pivotal/fixtures/v5_unsupported.json new file mode 100644 index 0000000000..ca2aa495fb --- /dev/null +++ b/zerver/webhooks/pivotal/fixtures/v5_unsupported.json @@ -0,0 +1,47 @@ +{ + "occurred_at": 1389218398000, + "kind": "epic_update_activity", + "project": { + "name": "Hard Code", + "kind": "project", + "id": 807213 + }, + "changes": [ + { + "name": "Story of the Year", + "story_type": "feature", + "change_type": "update", + "kind": "story", + "new_values": { + "current_state": "accepted", + "updated_at": 1389218398000, + "accepted_at": 1389218397000 + }, + "id": 63486316, + "original_values": { + "current_state": "unstarted", + "updated_at": 1389215951000, + "accepted_at": null + } + } + ], + "highlight": "accepted", + "project_version": 60, + "guid": "807213_60", + "primary_resources": [ + { + "name": "Story of the Year", + "story_type": "feature", + "kind": "story", + "id": 63486316, + "url": "http://www.pivotaltracker.com/story/show/63486316" + } + ], + "performed_by": { + "name": "Leo Franchi", + "initials": "LF", + "kind": "person", + "id": 981905 + }, + "message": "Leo Franchi accepted this feature" +} diff --git a/zerver/webhooks/pivotal/tests.py b/zerver/webhooks/pivotal/tests.py index 31bcca5a4f..5ffa4c5288 100644 --- a/zerver/webhooks/pivotal/tests.py +++ b/zerver/webhooks/pivotal/tests.py @@ -1,4 +1,10 @@ +from unittest import mock + +import orjson + +from zerver.lib.exceptions import UnsupportedWebhookEventType from zerver.lib.test_classes import WebhookTestCase +from zerver.webhooks.pivotal.view import api_pivotal_webhook_v5 class PivotalV3HookTests(WebhookTestCase): @@ -14,6 +20,14 @@ class PivotalV3HookTests(WebhookTestCase): "accepted", expected_topic, expected_message, content_type="application/xml" ) + def test_bad_subject(self) -> None: + expected_topic = "Story changed" + expected_message = "Leo Franchi accepted My new Feature story \ +[(view)](https://www.pivotaltracker.com/s/projects/807213/stories/48276573)." + self.check_webhook( + "bad_accepted", expected_topic, expected_message, content_type="application/xml" + ) + def test_commented(self) -> None: expected_topic = "Comment added" expected_message = 'Leo Franchi added comment: "FIX THIS NOW" \ @@ -186,5 +200,27 @@ Try again next time "type_changed", expected_topic, expected_message, content_type="application/xml" ) + def test_bad_payload(self) -> None: + bad = ("foo", None, "bar") + with self.assertRaisesRegex(AssertionError, "Unable to handle Pivotal payload"): + with mock.patch( + "zerver.webhooks.pivotal.view.api_pivotal_webhook_v3", return_value=bad + ): + self.check_webhook("accepted", expect_topic="foo") + + def test_bad_request(self) -> None: + request = mock.MagicMock() + hamlet = self.example_user("hamlet") + bad = orjson.loads(self.get_body("bad_request")) + + with mock.patch("zerver.webhooks.pivotal.view.orjson.loads", return_value=bad): + result = api_pivotal_webhook_v5(request, hamlet) + self.assertEqual(result[0], "#0: ") + + bad = orjson.loads(self.get_body("bad_kind")) + with self.assertRaisesRegex(UnsupportedWebhookEventType, "'unknown_kind'.* supported"): + with mock.patch("zerver.webhooks.pivotal.view.orjson.loads", return_value=bad): + api_pivotal_webhook_v5(request, hamlet) + def get_body(self, fixture_name: str) -> str: return self.webhook_fixture_data("pivotal", f"v5_{fixture_name}", file_type="json")