From 8b30d874b539aa9665387bc298e1fbb528c242d0 Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Tue, 24 Jan 2017 12:07:46 -0500
Subject: [PATCH] Update webhooks API test to set expectations independent of
 implementation.

It may seem like this is testing less since fewer API calls are being made - but when you use the backend to generate the data you trust in the test case - the correlation lands up meaning relatively less is tested. For instance, if there is some problem arises in the to_dict method of a webhook - it is going to arise in both the implementation and in the test case and so the test case will just be asserting that the response is broken ... and it will be. Testing more concrete things is also good for ensuring backward compatibility.
---
 test/api/test_webhooks.py | 48 ++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/test/api/test_webhooks.py b/test/api/test_webhooks.py
index 424f01e8e94..9d10589af47 100644
--- a/test/api/test_webhooks.py
+++ b/test/api/test_webhooks.py
@@ -1,44 +1,56 @@
-import os
 
 from base import api
-from galaxy.app import app
-from galaxy.util import galaxy_root_path
+from base.driver_util import TEST_WEBHOOKS_DIR
 from galaxy.webhooks import WebhooksRegistry
 
-WEBHOOKS_DEMO_DIRECTORY = os.path.join(
-    galaxy_root_path, 'config', 'plugins', 'webhooks', 'demo',
-)
-
 
 class WebhooksApiTestCase(api.ApiTestCase):
+
     def setUp(self):
         super(WebhooksApiTestCase, self).setUp()
-        app.webhooks_registry = WebhooksRegistry(WEBHOOKS_DEMO_DIRECTORY)
+        self.webhooks_registry = WebhooksRegistry(TEST_WEBHOOKS_DIR)
 
     def test_get_all(self):
         response = self._get('webhooks')
-        webhooks = [wh.to_dict() for wh in app.webhooks_registry.webhooks]
 
         self._assert_status_code_is(response, 200)
-        self.assertEqual(response.json(), webhooks)
+        webhook_objs = self._assert_are_webhooks(response)
+        names = self._get_webhook_names(webhook_objs)
+        for expected_name in ["history_test1", "history_test2", "masthead_test", "phdcomics", "trans_object", "xkcd"]:
+            assert expected_name in names
 
     def test_get_random(self):
         response = self._get('webhooks/tool')
         self._assert_status_code_is(response, 200)
+        self._assert_is_webhook(response.json())
 
     def test_get_all_by_type(self):
-        webhook_type = 'tool'
-        response = self._get('webhooks/%s/all' % webhook_type)
-        webhooks = [
-            wh.to_dict()
-            for wh in app.webhooks_registry.webhooks
-            if webhook_type in wh.type
-        ]
+        # Ensure tool type filtering include a valid webhook of type tool and excludes a webhook
+        # that isn't of type tool.
+        response = self._get('webhooks/tool/all')
 
         self._assert_status_code_is(response, 200)
-        self.assertEqual(response.json(), webhooks)
+        webhook_objs = self._assert_are_webhooks(response)
+        names = self._get_webhook_names(webhook_objs)
+        assert "phdcomics" in names
+        assert "trans_object" not in names  # properly filtered out by type
 
     def test_get_data(self):
         response = self._get('webhooks/trans_object/get_data')
         self._assert_status_code_is(response, 200)
         self._assert_has_keys(response.json(), 'username')
+
+    def _assert_are_webhooks(self, response):
+        response_list = response.json()
+        assert isinstance(response_list, list)
+        for obj in response_list:
+            self._assert_is_webhook(obj)
+        return response_list
+
+    def _assert_is_webhook(self, obj):
+        assert isinstance(obj, dict)
+        self._assert_has_keys(obj, 'styles', 'activate', 'name', 'script', 'type', 'config')
+
+    def _get_webhook_names(self, webhook_objs):
+        names = [w.get("name") for w in webhook_objs]
+        return names