[PATCH v2 09/13] tests: Rework REST API tests
Daniel Axtens
dja at axtens.net
Mon Nov 21 15:00:15 AEDT 2016
Hi Stephen,
So long as this doesn't decrease the coverage, I am all for simpler and
clearer tests!
Regards,
Daniel
> - 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
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list