| Message ID | 20180417084110.7760-1-stephen@that.guru |
|---|---|
| State | Accepted |
| Headers | show |
| Series | tests: Replace incorrect tests | expand |
----- 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 > >
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
----- 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 >
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)
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(-)