[PATCH 09/13] tests: Rework REST API tests

Stephen Finucane stephen at that.guru
Thu Nov 17 12:39:12 AEDT 2016


- Combine anonymous user tests with authenticated user tests
- Rename and simplify some tests
- Improve documentation

Signed-off-by: Stephen Finucane <stephen at that.guru>
Cc: Andy Doan <andy.doan at linaro.org>
---
 patchwork/api/project.py         |   1 -
 patchwork/tests/test_rest_api.py | 218 ++++++++++++++++-----------------------
 2 files changed, 89 insertions(+), 130 deletions(-)

diff --git a/patchwork/api/project.py b/patchwork/api/project.py
index 3319339..881de2d 100644
--- a/patchwork/api/project.py
+++ b/patchwork/api/project.py
@@ -36,7 +36,6 @@ class ProjectSerializer(HyperlinkedModelSerializer):
         model = Project
         fields = ('url', 'name', 'link_name', 'list_id', 'list_email',
                   'web_url', 'scm_url', 'webscm_url')
-        read_only_fields = ('name', 'maintainers')
         extra_kwargs = {
             'url': {'view_name': 'api-project-detail'},
         }
diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py
index 667f9f3..4675191 100644
--- a/patchwork/tests/test_rest_api.py
+++ b/patchwork/tests/test_rest_api.py
@@ -24,7 +24,6 @@ from django.conf import settings
 from django.core.urlresolvers import reverse
 
 from patchwork.models import Check
-from patchwork.models import Patch
 from patchwork.models import Project
 from patchwork.tests.utils import create_check
 from patchwork.tests.utils import create_maintainer
@@ -52,7 +51,7 @@ class TestProjectAPI(APITestCase):
             return reverse('api-project-list')
         return reverse('api-project-detail', args=[item])
 
-    def test_list_simple(self):
+    def test_list(self):
         """Validate we can list the default test project."""
         project = create_project()
 
@@ -85,64 +84,53 @@ class TestProjectAPI(APITestCase):
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(project.name, resp.data['name'])
 
-    def test_anonymous_create(self):
-        """Ensure anonymous POST operations are rejected."""
-        resp = self.client.post(
-            self.api_url(),
-            {'linkname': 'l', 'name': 'n', 'listid': 'l', 'listemail': 'e'})
-        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
-
-    def test_anonymous_update(self):
-        """Ensure anonymous "PATCH" operations are rejected."""
-        project = create_project()
-
-        resp = self.client.patch(self.api_url(project.id),
-                                 {'linkname': 'foo'})
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
-
-    def test_anonymous_delete(self):
-        """Ensure anonymous "DELETE" operations are rejected."""
-        project = create_project()
-
-        resp = self.client.delete(self.api_url(project.id))
-        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
-
     def test_create(self):
         """Ensure creations are rejected."""
         project = create_project()
+        data = {'linkname': 'l', 'name': 'n', 'listid': 'l', 'listemail': 'e'}
+
+        # an anonymous user
+        resp = self.client.post(self.api_url(), data)
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
+        # a superuser
         user = create_maintainer(project)
         user.is_superuser = True
         user.save()
         self.client.force_authenticate(user=user)
-        resp = self.client.post(
-            self.api_url(),
-            {'linkname': 'l', 'name': 'n', 'listid': 'l', 'listemail': 'e'})
+        resp = self.client.post(self.api_url(), data)
         self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
     def test_update(self):
-        """Ensure updates can be performed maintainers."""
+        """Ensure updates can be performed by maintainers."""
         project = create_project()
+        data = {'linkname': 'TEST'}
 
-        # A maintainer can update
-        user = create_maintainer(project)
-        self.client.force_authenticate(user=user)
-        resp = self.client.patch(self.api_url(project.id),
-                                 {'linkname': 'TEST'})
-        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        # an anonymous user
+        resp = self.client.patch(self.api_url(project.id), data)
+        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
 
-        # A normal user can't
+        # a normal user
         user = create_user()
         self.client.force_authenticate(user=user)
-        resp = self.client.patch(self.api_url(project.id),
-                                 {'linkname': 'TEST'})
+        resp = self.client.patch(self.api_url(project.id), data)
         self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
 
+        # a maintainer
+        user = create_maintainer(project)
+        self.client.force_authenticate(user=user)
+        resp = self.client.patch(self.api_url(project.id), data)
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+
     def test_delete(self):
         """Ensure deletions are rejected."""
-        # Even an admin can't remove a project
         project = create_project()
 
+        # an anonymous user
+        resp = self.client.delete(self.api_url(project.id))
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
+
+        # a super user
         user = create_maintainer(project)
         user.is_superuser = True
         user.save()
@@ -161,13 +149,13 @@ class TestPersonAPI(APITestCase):
             return reverse('api-person-list')
         return reverse('api-person-detail', args=[item])
 
-    def test_anonymous_list(self):
-        """The API should reject anonymous users."""
+    def test_list(self):
+        """This API requires authenticated users."""
+        # anonymous user
         resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
 
-    def test_authenticated_list(self):
-        """This API requires authenticated users."""
+        # authenticated user
         user = create_user()
         self.client.force_authenticate(user=user)
 
@@ -186,24 +174,22 @@ class TestPersonAPI(APITestCase):
         resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(2, len(resp.data))
-        self.assertEqual(person.name,
-                         resp.data[0]['name'])
+        self.assertEqual(person.name, resp.data[0]['name'])
         self.assertIsNone(resp.data[0]['user'])
 
-    def test_readonly(self):
+    def test_create_update_delete(self):
         user = create_maintainer()
         user.is_superuser = True
         user.save()
         self.client.force_authenticate(user=user)
 
-        resp = self.client.delete(self.api_url(user.id))
+        resp = self.client.post(self.api_url(), {'email': 'foo at f.com'})
         self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
-        resp = self.client.patch(self.api_url(user.id),
-                                 {'email': 'foo at f.com'})
+        resp = self.client.patch(self.api_url(user.id), {'email': 'foo at f.com'})
         self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
-        resp = self.client.post(self.api_url(), {'email': 'foo at f.com'})
+        resp = self.client.delete(self.api_url(user.id))
         self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
 
@@ -216,13 +202,13 @@ class TestUserAPI(APITestCase):
             return reverse('api-user-list')
         return reverse('api-user-detail', args=[item])
 
-    def test_anonymous_list(self):
-        """The API should reject anonymous users."""
+    def test_list(self):
+        """This API requires authenticated users."""
+        # anonymous users
         resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
 
-    def test_authenticated_list(self):
-        """This API requires authenticated users."""
+        # authenticated user
         user = create_user()
         self.client.force_authenticate(user=user)
 
@@ -233,15 +219,6 @@ class TestUserAPI(APITestCase):
         self.assertNotIn('password', resp.data[0])
         self.assertNotIn('is_superuser', resp.data[0])
 
-    def test_update(self):
-        user = create_maintainer()
-        user.is_superuser = True
-        user.save()
-        self.client.force_authenticate(user=user)
-
-        resp = self.client.patch(self.api_url(user.id), {'first_name': 'Tan'})
-        self.assertEqual(status.HTTP_200_OK, resp.status_code)
-
     def test_create_delete(self):
         user = create_maintainer()
         user.is_superuser = True
@@ -254,6 +231,18 @@ class TestUserAPI(APITestCase):
         resp = self.client.post(self.api_url(user.id), {'email': 'foo at f.com'})
         self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
+        resp = self.client.delete(self.api_url(user.id))
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
+
+    def test_update(self):
+        user = create_maintainer()
+        user.is_superuser = True
+        user.save()
+        self.client.force_authenticate(user=user)
+
+        resp = self.client.patch(self.api_url(user.id), {'first_name': 'Tan'})
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+
 
 @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
 class TestPatchAPI(APITestCase):
@@ -265,7 +254,7 @@ class TestPatchAPI(APITestCase):
             return reverse('api-patch-list')
         return reverse('api-patch-detail', args=[item])
 
-    def test_list_simple(self):
+    def test_list(self):
         """Validate we can list a patch."""
         resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
@@ -316,93 +305,67 @@ class TestPatchAPI(APITestCase):
         self.assertEqual(3, len(tags))
         self.assertEqual(1, tags['Reviewed-by'])
 
-    def test_anonymous_create(self):
-        """Ensure anonymous "POST" operations are rejected."""
+    def test_create(self):
+        """Ensure creations are rejected."""
+        project = create_project()
         patch = {
-            'project': create_project().id,
+            'project': project,
             'submitter': create_person().id,
             'msgid': make_msgid(),
             'name': 'test-create-patch',
             'diff': 'patch diff',
         }
 
+        # anonymous user
         resp = self.client.post(self.api_url(), patch)
         self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
-    def test_anonymous_update(self):
-        """Ensure anonymous "PATCH" operations are rejected."""
-        patch = create_patch()
-        patch_url = self.api_url(patch.id)
-
-        resp = self.client.get(patch_url)
-        patch = resp.data
-        patch['msgid'] = 'foo'
-        patch['name'] = 'this should fail'
-
-        resp = self.client.patch(patch_url, {'name': 'foo'})
-        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
-
-    def test_anonymous_delete(self):
-        """Ensure anonymous "DELETE" operations are rejected."""
-        patch = create_patch()
-        patch_url = self.api_url(patch.id)
-
-        resp = self.client.delete(patch_url)
-        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
-
-    def test_create(self):
-        """Ensure creations are rejected."""
-        submitter = create_person()
-        project = create_project()
+        # superuser
         user = create_maintainer(project)
         user.is_superuser = True
         user.save()
         self.client.force_authenticate(user=user)
-
-        patch = {
-            'project': project.id,
-            'submitter': submitter.id,
-            'msgid': make_msgid(),
-            'name': 'test-create-patch',
-            'diff': 'patch diff',
-        }
-
         resp = self.client.post(self.api_url(), patch)
         self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
 
     def test_update(self):
         """Ensure updates can be performed by maintainers."""
-        # A maintainer can update
         project = create_project()
         patch = create_patch(project=project)
         state = create_state()
-        user = create_maintainer(project)
-        self.client.force_authenticate(user=user)
 
-        resp = self.client.patch(self.api_url(patch.id),
-                                 {'state': state.name})
-        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        # anonymous user
+        resp = self.client.patch(self.api_url(patch.id), {'state': state.name})
+        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
 
-        # A normal user can't
+        # authenticated user
         user = create_user()
         self.client.force_authenticate(user=user)
-
-        resp = self.client.patch(self.api_url(patch.id),
-                                 {'state': state.name})
+        resp = self.client.patch(self.api_url(patch.id), {'state': state.name})
         self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
 
+        # maintainer
+        user = create_maintainer(project)
+        self.client.force_authenticate(user=user)
+        resp = self.client.patch(self.api_url(patch.id), {'state': state.name})
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+
     def test_delete(self):
         """Ensure deletions are always rejected."""
         project = create_project()
         patch = create_patch(project=project)
+
+        # anonymous user
+        resp = self.client.delete(self.api_url(patch.id))
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
+
+        # superuser
         user = create_maintainer(project)
         user.is_superuser = True
         user.save()
         self.client.force_authenticate(user=user)
-
         resp = self.client.delete(self.api_url(patch.id))
         self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
-        self.assertEqual(1, Patch.objects.all().count())
 
 
 @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
@@ -428,7 +391,7 @@ class TestCheckAPI(APITestCase):
         }
         return create_check(**values)
 
-    def test_list_simple(self):
+    def test_list(self):
         """Validate we can list checks on a patch."""
         resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
@@ -452,22 +415,6 @@ class TestCheckAPI(APITestCase):
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(check.target_url, resp.data['target_url'])
 
-    def test_update_delete(self):
-        """Ensure updates and deletes aren't allowed"""
-        check = self._create_check()
-        self.user.is_superuser = True
-        self.user.save()
-        self.client.force_authenticate(user=self.user)
-
-        # update
-        resp = self.client.patch(self.api_url(check),
-                                 {'target_url': 'fail'})
-        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
-
-        # delete
-        resp = self.client.delete(self.api_url(check))
-        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
-
     def test_create(self):
         """Ensure creations can be performed by user of patch."""
         check = {
@@ -500,3 +447,16 @@ class TestCheckAPI(APITestCase):
         resp = self.client.post(self.api_url(), check)
         self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
         self.assertEqual(0, Check.objects.all().count())
+
+    def test_update_delete(self):
+        """Ensure updates and deletes aren't allowed"""
+        check = self._create_check()
+        self.user.is_superuser = True
+        self.user.save()
+        self.client.force_authenticate(user=self.user)
+
+        resp = self.client.patch(self.api_url(check), {'target_url': 'fail'})
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
+
+        resp = self.client.delete(self.api_url(check))
+        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
-- 
2.7.4



More information about the Patchwork mailing list