tests: Replace incorrect tests

Message ID 20180417084110.7760-1-stephen@that.guru
State New
Headers show
Series
  • tests: Replace incorrect tests
Related show

Commit Message

Stephen Finucane April 17, 2018, 8:41 a.m.
In commit 683792d1, the 'test_api.py' was split into multiple
'api/test_xyz.py' files. As part of this change, the tests for cover
letter were mistakenly included in place of tests for checks. Correct
this oversight.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Reported-by: Veronika Kabatova <vkabatov@redhat.com>
Fixes: 683792d1 ("tests: Split 'test_rest_api'")
---
 patchwork/tests/api/test_check.py | 140 ++++++++++++++++++++++----------------
 1 file changed, 81 insertions(+), 59 deletions(-)

Comments

Veronika Kabatova April 17, 2018, 9:24 a.m. | #1
----- Original Message -----
> From: "Stephen Finucane" <stephen@that.guru>
> To: patchwork@lists.ozlabs.org
> Cc: vkabatov@redhat.com, "Stephen Finucane" <stephen@that.guru>
> Sent: Tuesday, April 17, 2018 10:41:10 AM
> Subject: [PATCH] tests: Replace incorrect tests
> 
> In commit 683792d1, the 'test_api.py' was split into multiple
> 'api/test_xyz.py' files. As part of this change, the tests for cover
> letter were mistakenly included in place of tests for checks. Correct
> this oversight.
> 
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> Reported-by: Veronika Kabatova <vkabatov@redhat.com>
> Fixes: 683792d1 ("tests: Split 'test_rest_api'")
> ---

Thanks for the fast reaction and fix!

Veronika

>  patchwork/tests/api/test_check.py | 140
>  ++++++++++++++++++++++----------------
>  1 file changed, 81 insertions(+), 59 deletions(-)
> 
> diff --git a/patchwork/tests/api/test_check.py
> b/patchwork/tests/api/test_check.py
> index 6e3d68b8..7b8139ad 100644
> --- a/patchwork/tests/api/test_check.py
> +++ b/patchwork/tests/api/test_check.py
> @@ -38,84 +38,106 @@ else:
>  
>  
>  @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
> -class TestCoverLetterAPI(APITestCase):
> +class TestCheckAPI(APITestCase):
>      fixtures = ['default_tags']
>  
> -    @staticmethod
> -    def api_url(item=None):
> +    def api_url(self, item=None):
>          if item is None:
> -            return reverse('api-cover-list')
> -        return reverse('api-cover-detail', args=[item])
> -
> -    def assertSerialized(self, cover_obj, cover_json):
> -        self.assertEqual(cover_obj.id, cover_json['id'])
> -        self.assertEqual(cover_obj.name, cover_json['name'])
> -        self.assertIn(cover_obj.get_mbox_url(), cover_json['mbox'])
> -
> -        # nested fields
> -
> -        self.assertEqual(cover_obj.submitter.id,
> -                         cover_json['submitter']['id'])
> +            return reverse('api-check-list', args=[self.patch.id])
> +        return reverse('api-check-detail', kwargs={
> +            'patch_id': self.patch.id, 'check_id': item.id})
> +
> +    def setUp(self):
> +        super(TestCheckAPI, self).setUp()
> +        project = create_project()
> +        self.user = create_maintainer(project)
> +        self.patch = create_patch(project=project)
> +
> +    def _create_check(self):
> +        values = {
> +            'patch': self.patch,
> +            'user': self.user,
> +        }
> +        return create_check(**values)
> +
> +    def assertSerialized(self, check_obj, check_json):
> +        self.assertEqual(check_obj.id, check_json['id'])
> +        self.assertEqual(check_obj.get_state_display(), check_json['state'])
> +        self.assertEqual(check_obj.target_url, check_json['target_url'])
> +        self.assertEqual(check_obj.context, check_json['context'])
> +        self.assertEqual(check_obj.description, check_json['description'])
>  
>      def test_list(self):
> -        """Validate we can list cover letters."""
> +        """Validate we can list checks on a patch."""
>          resp = self.client.get(self.api_url())
>          self.assertEqual(status.HTTP_200_OK, resp.status_code)
>          self.assertEqual(0, len(resp.data))
>  
> -        person_obj = create_person(email='test@example.com')
> -        project_obj = create_project(linkname='myproject')
> -        cover_obj = create_cover(project=project_obj, submitter=person_obj)
> +        check_obj = self._create_check()
>  
> -        # anonymous user
> -        resp = self.client.get(self.api_url())
> -        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> -        self.assertEqual(1, len(resp.data))
> -        self.assertSerialized(cover_obj, resp.data[0])
> -
> -        # authenticated user
> -        user = create_user()
> -        self.client.force_authenticate(user=user)
>          resp = self.client.get(self.api_url())
>          self.assertEqual(status.HTTP_200_OK, resp.status_code)
>          self.assertEqual(1, len(resp.data))
> -        self.assertSerialized(cover_obj, resp.data[0])
> -
> -        # test filtering by project
> -        resp = self.client.get(self.api_url(), {'project': 'myproject'})
> -        self.assertEqual([cover_obj.id], [x['id'] for x in resp.data])
> -        resp = self.client.get(self.api_url(), {'project':
> 'invalidproject'})
> -        self.assertEqual(0, len(resp.data))
> -
> -        # test filtering by submitter, both ID and email
> -        resp = self.client.get(self.api_url(), {'submitter': person_obj.id})
> -        self.assertEqual([cover_obj.id], [x['id'] for x in resp.data])
> -        resp = self.client.get(self.api_url(), {
> -            'submitter': 'test@example.com'})
> -        self.assertEqual([cover_obj.id], [x['id'] for x in resp.data])
> -        resp = self.client.get(self.api_url(), {
> -            'submitter': 'test@example.org'})
> +        self.assertSerialized(check_obj, resp.data[0])
> +
> +        # test filtering by owner, both ID and username
> +        resp = self.client.get(self.api_url(), {'user': self.user.id})
> +        self.assertEqual([check_obj.id], [x['id'] for x in resp.data])
> +        resp = self.client.get(self.api_url(), {'user': self.user.username})
> +        self.assertEqual([check_obj.id], [x['id'] for x in resp.data])
> +        resp = self.client.get(self.api_url(), {'user': 'otheruser'})
>          self.assertEqual(0, len(resp.data))
>  
>      def test_detail(self):
> -        """Validate we can get a specific cover letter."""
> -        cover_obj = create_cover()
> -
> -        resp = self.client.get(self.api_url(cover_obj.id))
> +        """Validate we can get a specific check."""
> +        check = self._create_check()
> +        resp = self.client.get(self.api_url(check))
>          self.assertEqual(status.HTTP_200_OK, resp.status_code)
> -        self.assertSerialized(cover_obj, resp.data)
> +        self.assertSerialized(check, resp.data)
> +
> +    def test_create(self):
> +        """Ensure creations can be performed by user of patch."""
> +        check = {
> +            'state': 'success',
> +            'target_url': 'http://t.co',
> +            'description': 'description',
> +            'context': 'context',
> +        }
> +
> +        self.client.force_authenticate(user=self.user)
> +        resp = self.client.post(self.api_url(), check)
> +        self.assertEqual(status.HTTP_201_CREATED, resp.status_code)
> +        self.assertEqual(1, Check.objects.all().count())
> +        self.assertSerialized(Check.objects.first(), resp.data)
>  
> -    def test_create_update_delete(self):
> -        user = create_maintainer()
> -        user.is_superuser = True
> -        user.save()
> +        user = create_user()
>          self.client.force_authenticate(user=user)
> -
> -        resp = self.client.post(self.api_url(), {'name': 'test cover'})
> -        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED,
> resp.status_code)
> -
> -        resp = self.client.patch(self.api_url(), {'name': 'test cover'})
> +        resp = self.client.post(self.api_url(), check)
> +        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
> +
> +    def test_create_invalid(self):
> +        """Ensure we handle invalid check states."""
> +        check = {
> +            'state': 'this-is-not-a-valid-state',
> +            'target_url': 'http://t.co',
> +            'description': 'description',
> +            'context': 'context',
> +        }
> +
> +        self.client.force_authenticate(user=self.user)
> +        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())
> +        resp = self.client.delete(self.api_url(check))
>          self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED,
>          resp.status_code)
> --
> 2.14.3
> 
>
Stephen Finucane April 17, 2018, 9:51 a.m. | #2
On Tue, 2018-04-17 at 05:24 -0400, Veronika Kabatova wrote:
> Thanks for the fast reaction and fix!
> 
> Veronika

No problem. Can I get an Acked-by: if you're happy with it? :)

Stephen
Veronika Kabatova April 17, 2018, 9:57 a.m. | #3
----- Original Message -----
> From: "Stephen Finucane" <stephen@that.guru>
> To: "Veronika Kabatova" <vkabatov@redhat.com>
> Cc: patchwork@lists.ozlabs.org
> Sent: Tuesday, April 17, 2018 11:51:24 AM
> Subject: Re: [PATCH] tests: Replace incorrect tests
> 
> On Tue, 2018-04-17 at 05:24 -0400, Veronika Kabatova wrote:
> > Thanks for the fast reaction and fix!
> > 
> > Veronika
> 
> No problem. Can I get an Acked-by: if you're happy with it? :)
> 

Sure!

Acked-by: Veronika Kabatova <vkabatov@redhat.com>

> Stephen
>

Patch

diff --git a/patchwork/tests/api/test_check.py b/patchwork/tests/api/test_check.py
index 6e3d68b8..7b8139ad 100644
--- a/patchwork/tests/api/test_check.py
+++ b/patchwork/tests/api/test_check.py
@@ -38,84 +38,106 @@  else:
 
 
 @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
-class TestCoverLetterAPI(APITestCase):
+class TestCheckAPI(APITestCase):
     fixtures = ['default_tags']
 
-    @staticmethod
-    def api_url(item=None):
+    def api_url(self, item=None):
         if item is None:
-            return reverse('api-cover-list')
-        return reverse('api-cover-detail', args=[item])
-
-    def assertSerialized(self, cover_obj, cover_json):
-        self.assertEqual(cover_obj.id, cover_json['id'])
-        self.assertEqual(cover_obj.name, cover_json['name'])
-        self.assertIn(cover_obj.get_mbox_url(), cover_json['mbox'])
-
-        # nested fields
-
-        self.assertEqual(cover_obj.submitter.id,
-                         cover_json['submitter']['id'])
+            return reverse('api-check-list', args=[self.patch.id])
+        return reverse('api-check-detail', kwargs={
+            'patch_id': self.patch.id, 'check_id': item.id})
+
+    def setUp(self):
+        super(TestCheckAPI, self).setUp()
+        project = create_project()
+        self.user = create_maintainer(project)
+        self.patch = create_patch(project=project)
+
+    def _create_check(self):
+        values = {
+            'patch': self.patch,
+            'user': self.user,
+        }
+        return create_check(**values)
+
+    def assertSerialized(self, check_obj, check_json):
+        self.assertEqual(check_obj.id, check_json['id'])
+        self.assertEqual(check_obj.get_state_display(), check_json['state'])
+        self.assertEqual(check_obj.target_url, check_json['target_url'])
+        self.assertEqual(check_obj.context, check_json['context'])
+        self.assertEqual(check_obj.description, check_json['description'])
 
     def test_list(self):
-        """Validate we can list cover letters."""
+        """Validate we can list checks on a patch."""
         resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(0, len(resp.data))
 
-        person_obj = create_person(email='test@example.com')
-        project_obj = create_project(linkname='myproject')
-        cover_obj = create_cover(project=project_obj, submitter=person_obj)
+        check_obj = self._create_check()
 
-        # anonymous user
-        resp = self.client.get(self.api_url())
-        self.assertEqual(status.HTTP_200_OK, resp.status_code)
-        self.assertEqual(1, len(resp.data))
-        self.assertSerialized(cover_obj, resp.data[0])
-
-        # authenticated user
-        user = create_user()
-        self.client.force_authenticate(user=user)
         resp = self.client.get(self.api_url())
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(1, len(resp.data))
-        self.assertSerialized(cover_obj, resp.data[0])
-
-        # test filtering by project
-        resp = self.client.get(self.api_url(), {'project': 'myproject'})
-        self.assertEqual([cover_obj.id], [x['id'] for x in resp.data])
-        resp = self.client.get(self.api_url(), {'project': 'invalidproject'})
-        self.assertEqual(0, len(resp.data))
-
-        # test filtering by submitter, both ID and email
-        resp = self.client.get(self.api_url(), {'submitter': person_obj.id})
-        self.assertEqual([cover_obj.id], [x['id'] for x in resp.data])
-        resp = self.client.get(self.api_url(), {
-            'submitter': 'test@example.com'})
-        self.assertEqual([cover_obj.id], [x['id'] for x in resp.data])
-        resp = self.client.get(self.api_url(), {
-            'submitter': 'test@example.org'})
+        self.assertSerialized(check_obj, resp.data[0])
+
+        # test filtering by owner, both ID and username
+        resp = self.client.get(self.api_url(), {'user': self.user.id})
+        self.assertEqual([check_obj.id], [x['id'] for x in resp.data])
+        resp = self.client.get(self.api_url(), {'user': self.user.username})
+        self.assertEqual([check_obj.id], [x['id'] for x in resp.data])
+        resp = self.client.get(self.api_url(), {'user': 'otheruser'})
         self.assertEqual(0, len(resp.data))
 
     def test_detail(self):
-        """Validate we can get a specific cover letter."""
-        cover_obj = create_cover()
-
-        resp = self.client.get(self.api_url(cover_obj.id))
+        """Validate we can get a specific check."""
+        check = self._create_check()
+        resp = self.client.get(self.api_url(check))
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
-        self.assertSerialized(cover_obj, resp.data)
+        self.assertSerialized(check, resp.data)
+
+    def test_create(self):
+        """Ensure creations can be performed by user of patch."""
+        check = {
+            'state': 'success',
+            'target_url': 'http://t.co',
+            'description': 'description',
+            'context': 'context',
+        }
+
+        self.client.force_authenticate(user=self.user)
+        resp = self.client.post(self.api_url(), check)
+        self.assertEqual(status.HTTP_201_CREATED, resp.status_code)
+        self.assertEqual(1, Check.objects.all().count())
+        self.assertSerialized(Check.objects.first(), resp.data)
 
-    def test_create_update_delete(self):
-        user = create_maintainer()
-        user.is_superuser = True
-        user.save()
+        user = create_user()
         self.client.force_authenticate(user=user)
-
-        resp = self.client.post(self.api_url(), {'name': 'test cover'})
-        self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)
-
-        resp = self.client.patch(self.api_url(), {'name': 'test cover'})
+        resp = self.client.post(self.api_url(), check)
+        self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
+
+    def test_create_invalid(self):
+        """Ensure we handle invalid check states."""
+        check = {
+            'state': 'this-is-not-a-valid-state',
+            'target_url': 'http://t.co',
+            'description': 'description',
+            'context': 'context',
+        }
+
+        self.client.force_authenticate(user=self.user)
+        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())
+        resp = self.client.delete(self.api_url(check))
         self.assertEqual(status.HTTP_405_METHOD_NOT_ALLOWED, resp.status_code)