From 921410d8f8898ffb42ab313a9f3b8896ff94741e Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 17:17:03 +0100 Subject: [PATCH 01/12] 404 status should raise 404 error, even if JSON is valid. --- openphoto/openphoto_http.py | 6 +++--- tests/unit/test_http_errors.py | 4 ---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/openphoto/openphoto_http.py b/openphoto/openphoto_http.py index 86a8b3d..d1e2475 100644 --- a/openphoto/openphoto_http.py +++ b/openphoto/openphoto_http.py @@ -183,6 +183,9 @@ class OpenPhotoHttp: Decodes the JSON response, returning a dict. Raises an exception if an invalid response code is received. """ + if response.status_code == 404: + raise OpenPhoto404Error("HTTP Error %d: %s" % + (response.status_code, response.reason)) try: json_response = response.json() code = json_response["code"] @@ -192,9 +195,6 @@ class OpenPhotoHttp: if 200 <= response.status_code < 300: # Status code was valid, so just reraise the exception raise - elif response.status_code == 404: - raise OpenPhoto404Error("HTTP Error %d: %s" % - (response.status_code, response.reason)) else: raise OpenPhotoError("HTTP Error %d: %s" % (response.status_code, response.reason)) diff --git a/tests/unit/test_http_errors.py b/tests/unit/test_http_errors.py index d95a838..56b1d19 100644 --- a/tests/unit/test_http_errors.py +++ b/tests/unit/test_http_errors.py @@ -61,8 +61,6 @@ class TestHttpErrors(unittest.TestCase): with self.assertRaises(openphoto.OpenPhotoError): self.client.post(self.test_endpoint) - # TODO: 404 status should raise 404 error, even if JSON is valid - @unittest.expectedFailure @httpretty.activate def test_get_with_404_status(self): """ @@ -73,8 +71,6 @@ class TestHttpErrors(unittest.TestCase): with self.assertRaises(openphoto.OpenPhoto404Error): self.client.get(self.test_endpoint) - # TODO: 404 status should raise 404 error, even if JSON is valid - @unittest.expectedFailure @httpretty.activate def test_post_with_404_status(self): """ From 280521cf4a89a928aa60209e88f551d267c5de9d Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 17:21:20 +0100 Subject: [PATCH 02/12] Mismatch between response status and JSON code raises an exception --- openphoto/openphoto_http.py | 4 ++++ tests/unit/test_http_errors.py | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openphoto/openphoto_http.py b/openphoto/openphoto_http.py index d1e2475..7e0ec4f 100644 --- a/openphoto/openphoto_http.py +++ b/openphoto/openphoto_http.py @@ -199,6 +199,10 @@ class OpenPhotoHttp: raise OpenPhotoError("HTTP Error %d: %s" % (response.status_code, response.reason)) + if code != response.status_code: + raise OpenPhotoError(("Response status code %d does not match " + + "JSON status code %d") % (response.status_code, + code)) if 200 <= code < 300: return json_response elif (code == DUPLICATE_RESPONSE["code"] and diff --git a/tests/unit/test_http_errors.py b/tests/unit/test_http_errors.py index 56b1d19..90373a7 100644 --- a/tests/unit/test_http_errors.py +++ b/tests/unit/test_http_errors.py @@ -163,8 +163,6 @@ class TestHttpErrors(unittest.TestCase): with self.assertRaises(openphoto.OpenPhotoDuplicateError): self.client.post(self.test_endpoint) - # TODO: Status code mismatch should raise an exception - @unittest.expectedFailure @httpretty.activate def test_get_with_status_code_mismatch(self): """ @@ -176,8 +174,6 @@ class TestHttpErrors(unittest.TestCase): with self.assertRaises(openphoto.OpenPhotoError): self.client.get(self.test_endpoint) - # TODO: Status code mismatch should raise an exception - @unittest.expectedFailure @httpretty.activate def test_post_with_status_code_mismatch(self): """ From 3f1757d0aa8b3d1fc61d20596218625560ffd24d Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 17:30:25 +0100 Subject: [PATCH 03/12] photos.update/update now also accept a list of Photo objects --- openphoto/api_photo.py | 19 +++++++++++++++++-- tests/unit/test_photos.py | 4 ---- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/openphoto/api_photo.py b/openphoto/api_photo.py index cfa2c45..c5052ef 100644 --- a/openphoto/api_photo.py +++ b/openphoto/api_photo.py @@ -4,6 +4,19 @@ from openphoto.errors import OpenPhotoError import openphoto.openphoto_http from openphoto.objects import Photo +def extract_ids(photos): + """ + Given a list of objects, extract the photo id for each Photo + object. + """ + ids = [] + for photo in photos: + if isinstance(photo, Photo): + ids.append(photo.id) + else: + ids.append(photo) + return ids + class ApiPhotos: def __init__(self, client): self._client = client @@ -20,7 +33,8 @@ class ApiPhotos: Returns True if successful. Raises OpenPhotoError if not. """ - if not self._client.post("/photos/update.json", ids=photos, + ids = extract_ids(photos) + if not self._client.post("/photos/update.json", ids=ids, **kwds)["result"]: raise OpenPhotoError("Update response returned False") return True @@ -31,7 +45,8 @@ class ApiPhotos: Returns True if successful. Raises OpenPhotoError if not. """ - if not self._client.post("/photos/delete.json", ids=photos, + ids = extract_ids(photos) + if not self._client.post("/photos/delete.json", ids=ids, **kwds)["result"]: raise OpenPhotoError("Delete response returned False") return True diff --git a/tests/unit/test_photos.py b/tests/unit/test_photos.py index bf783fe..6a53677 100644 --- a/tests/unit/test_photos.py +++ b/tests/unit/test_photos.py @@ -40,8 +40,6 @@ class TestPhotosList(TestPhotos): self.assertEqual(result[1].tags, ["tag3", "tag4"]) class TestPhotosUpdate(TestPhotos): - # TODO: photos.update should accept a list of Photo objects - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'post') def test_photos_update(self, mock_post): """Check that multiple photos can be updated""" @@ -71,8 +69,6 @@ class TestPhotosUpdate(TestPhotos): self.client.photos.update(self.test_photos, title="Test") class TestPhotosDelete(TestPhotos): - # TODO: photos.delete should accept a list of Photo objects - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'post') def test_photos_delete(self, mock_post): """Check that multiple photos can be deleted""" From 9968d7f265170fca53bf800de1dbe62a57d0b999 Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 17:34:41 +0100 Subject: [PATCH 04/12] Delete methods should raise exception on failure, rather than returning False --- openphoto/objects.py | 8 ++++++++ tests/unit/test_albums.py | 4 ---- tests/unit/test_photos.py | 4 ---- tests/unit/test_tags.py | 4 ---- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/openphoto/objects.py b/openphoto/objects.py index 6d5daa2..61b4b7c 100644 --- a/openphoto/objects.py +++ b/openphoto/objects.py @@ -3,6 +3,8 @@ try: except ImportError: from urllib import quote # Python2 +from openphoto.errors import OpenPhotoError + class OpenPhotoObject: """ Base object supporting the storage of custom fields as attributes """ def __init__(self, openphoto, json_dict): @@ -51,6 +53,8 @@ class Photo(OpenPhotoObject): """ result = self._openphoto.post("/photo/%s/delete.json" % self.id, **kwds)["result"] + if not result: + raise OpenPhotoError("Delete response returned False") self._replace_fields({}) return result @@ -136,6 +140,8 @@ class Tag(OpenPhotoObject): """ result = self._openphoto.post("/tag/%s/delete.json" % quote(self.id), **kwds)["result"] + if not result: + raise OpenPhotoError("Delete response returned False") self._replace_fields({}) return result @@ -172,6 +178,8 @@ class Album(OpenPhotoObject): """ result = self._openphoto.post("/album/%s/delete.json" % self.id, **kwds)["result"] + if not result: + raise OpenPhotoError("Delete response returned False") self._replace_fields({}) return result diff --git a/tests/unit/test_albums.py b/tests/unit/test_albums.py index a117bd5..a29fd15 100644 --- a/tests/unit/test_albums.py +++ b/tests/unit/test_albums.py @@ -88,8 +88,6 @@ class TestAlbumDelete(TestAlbums): mock_post.assert_called_with("/album/1/delete.json") self.assertEqual(result, True) - # TODO: album.delete should raise exception on failure - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'post') def test_album_delete_failure(self, mock_post): """Check that an exception is raised if an album cannot be deleted""" @@ -110,8 +108,6 @@ class TestAlbumDelete(TestAlbums): # self.assertEqual(album.id, None) # self.assertEqual(album.name, None) - # TODO: album.delete should raise exception on failure - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'post') def test_album_object_delete_failure(self, mock_post): """ diff --git a/tests/unit/test_photos.py b/tests/unit/test_photos.py index 6a53677..92440aa 100644 --- a/tests/unit/test_photos.py +++ b/tests/unit/test_photos.py @@ -112,8 +112,6 @@ class TestPhotoDelete(TestPhotos): mock_post.assert_called_with("/photo/1a/delete.json") self.assertEqual(result, True) - # TODO: photo.delete should raise exception on failure - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'post') def test_photo_delete_failure(self, mock_post): """Check that an exception is raised if a photo cannot be deleted""" @@ -136,8 +134,6 @@ class TestPhotoDelete(TestPhotos): self.assertEqual(photo.get_fields(), {}) # self.assertEqual(photo.id, None) - # TODO: photo.delete should raise exception on failure - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'post') def test_photo_object_delete_failure(self, mock_post): """ diff --git a/tests/unit/test_tags.py b/tests/unit/test_tags.py index 76fac62..b266fbe 100644 --- a/tests/unit/test_tags.py +++ b/tests/unit/test_tags.py @@ -64,8 +64,6 @@ class TestTagDelete(TestTags): mock_post.assert_called_with("/tag/tag1/delete.json") self.assertEqual(result, True) - # TODO: tag.delete should raise exception on failure - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'post') def test_tag_delete_failure(self, mock_post): """Check that an exception is raised if a tag cannot be deleted""" @@ -85,8 +83,6 @@ class TestTagDelete(TestTags): self.assertEqual(tag.get_fields(), {}) # self.assertEqual(tag.id, None) - # TODO: tag.delete should raise exception on failure - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'post') def test_tag_object_delete_failure(self, mock_post): """ From 5656221147db05ae561e7b95c319eb49e08699f6 Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 17:39:02 +0100 Subject: [PATCH 05/12] After deleting an object's fields, name and id should be set to None --- openphoto/objects.py | 16 +++++++++++++--- tests/unit/test_albums.py | 5 ++--- tests/unit/test_photos.py | 3 +-- tests/unit/test_tags.py | 3 +-- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/openphoto/objects.py b/openphoto/objects.py index 61b4b7c..d95b803 100644 --- a/openphoto/objects.py +++ b/openphoto/objects.py @@ -31,6 +31,16 @@ class OpenPhotoObject: self._json_dict = json_dict self._set_fields(json_dict) + def _delete_fields(self): + """ + Delete this object's attributes, including name and id + """ + for key in self._json_dict.keys(): + delattr(self, key) + self._json_dict = {} + self.id = None + self.name = None + def __repr__(self): if self.name is not None: return "<%s name='%s'>" % (self.__class__, self.name) @@ -55,7 +65,7 @@ class Photo(OpenPhotoObject): self.id, **kwds)["result"] if not result: raise OpenPhotoError("Delete response returned False") - self._replace_fields({}) + self._delete_fields() return result def edit(self, **kwds): @@ -142,7 +152,7 @@ class Tag(OpenPhotoObject): quote(self.id), **kwds)["result"] if not result: raise OpenPhotoError("Delete response returned False") - self._replace_fields({}) + self._delete_fields() return result def update(self, **kwds): @@ -180,7 +190,7 @@ class Album(OpenPhotoObject): self.id, **kwds)["result"] if not result: raise OpenPhotoError("Delete response returned False") - self._replace_fields({}) + self._delete_fields() return result def form(self, **kwds): diff --git a/tests/unit/test_albums.py b/tests/unit/test_albums.py index a29fd15..5751688 100644 --- a/tests/unit/test_albums.py +++ b/tests/unit/test_albums.py @@ -95,7 +95,6 @@ class TestAlbumDelete(TestAlbums): with self.assertRaises(openphoto.OpenPhotoError): self.client.album.delete(self.test_albums[0]) - # TODO: after deleting object fields, name and id should be set to None @mock.patch.object(openphoto.OpenPhoto, 'post') def test_album_object_delete(self, mock_post): """Check that an album can be deleted using the album object directly""" @@ -105,8 +104,8 @@ class TestAlbumDelete(TestAlbums): mock_post.assert_called_with("/album/1/delete.json") self.assertEqual(result, True) self.assertEqual(album.get_fields(), {}) - # self.assertEqual(album.id, None) - # self.assertEqual(album.name, None) + self.assertEqual(album.id, None) + self.assertEqual(album.name, None) @mock.patch.object(openphoto.OpenPhoto, 'post') def test_album_object_delete_failure(self, mock_post): diff --git a/tests/unit/test_photos.py b/tests/unit/test_photos.py index 92440aa..6cc43fa 100644 --- a/tests/unit/test_photos.py +++ b/tests/unit/test_photos.py @@ -119,7 +119,6 @@ class TestPhotoDelete(TestPhotos): with self.assertRaises(openphoto.OpenPhotoError): self.client.photo.delete(self.test_photos[0]) - # TODO: after deleting object fields, name and id should be set to None @mock.patch.object(openphoto.OpenPhoto, 'post') def test_photo_object_delete(self, mock_post): """ @@ -132,7 +131,7 @@ class TestPhotoDelete(TestPhotos): mock_post.assert_called_with("/photo/1a/delete.json") self.assertEqual(result, True) self.assertEqual(photo.get_fields(), {}) - # self.assertEqual(photo.id, None) + self.assertEqual(photo.id, None) @mock.patch.object(openphoto.OpenPhoto, 'post') def test_photo_object_delete_failure(self, mock_post): diff --git a/tests/unit/test_tags.py b/tests/unit/test_tags.py index b266fbe..c4ecd4a 100644 --- a/tests/unit/test_tags.py +++ b/tests/unit/test_tags.py @@ -71,7 +71,6 @@ class TestTagDelete(TestTags): with self.assertRaises(openphoto.OpenPhotoError): self.client.tag.delete(self.test_tags[0]) - # TODO: after deleting object fields, id should be set to None @mock.patch.object(openphoto.OpenPhoto, 'post') def test_tag_object_delete(self, mock_post): """Check that a tag can be deleted when using the tag object directly""" @@ -81,7 +80,7 @@ class TestTagDelete(TestTags): mock_post.assert_called_with("/tag/tag1/delete.json") self.assertEqual(result, True) self.assertEqual(tag.get_fields(), {}) - # self.assertEqual(tag.id, None) + self.assertEqual(tag.id, None) @mock.patch.object(openphoto.OpenPhoto, 'post') def test_tag_object_delete_failure(self, mock_post): From 413cf297a3a5a62d787e09926335aecad0733d34 Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 17:40:25 +0100 Subject: [PATCH 06/12] Replace_encoded parameter should be called photo_file, not encoded_photo --- openphoto/objects.py | 2 +- tests/unit/test_photos.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/openphoto/objects.py b/openphoto/objects.py index d95b803..8e40cc7 100644 --- a/openphoto/objects.py +++ b/openphoto/objects.py @@ -77,7 +77,7 @@ class Photo(OpenPhotoObject): def replace(self, photo_file, **kwds): raise NotImplementedError() - def replace_encoded(self, encoded_photo, **kwds): + def replace_encoded(self, photo_file, **kwds): raise NotImplementedError() def update(self, **kwds): diff --git a/tests/unit/test_photos.py b/tests/unit/test_photos.py index 6cc43fa..c76f4cc 100644 --- a/tests/unit/test_photos.py +++ b/tests/unit/test_photos.py @@ -203,9 +203,6 @@ class TestPhotoReplace(TestPhotos): with self.assertRaises(NotImplementedError): self.client.photo.replace_encoded("1a", self.test_file) - # TODO: replace_encoded parameter should be called photo_file, - # not encoded_photo - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'post') def test_photo_object_replace_encoded(self, _): """ If photo.replace_encoded gets implemented, write a test! """ From 971bab4f7b62747a9e20e6de2d482b145704797b Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 18:04:36 +0100 Subject: [PATCH 07/12] Status code mismatches are actually quite common. Rather than raise an exception, just ensure we return the JSON code. --- openphoto/openphoto_http.py | 4 ---- tests/unit/test_http_errors.py | 22 ++++++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/openphoto/openphoto_http.py b/openphoto/openphoto_http.py index 7e0ec4f..d1e2475 100644 --- a/openphoto/openphoto_http.py +++ b/openphoto/openphoto_http.py @@ -199,10 +199,6 @@ class OpenPhotoHttp: raise OpenPhotoError("HTTP Error %d: %s" % (response.status_code, response.reason)) - if code != response.status_code: - raise OpenPhotoError(("Response status code %d does not match " + - "JSON status code %d") % (response.status_code, - code)) if 200 <= code < 300: return json_response elif (code == DUPLICATE_RESPONSE["code"] and diff --git a/tests/unit/test_http_errors.py b/tests/unit/test_http_errors.py index 90373a7..d5c5988 100644 --- a/tests/unit/test_http_errors.py +++ b/tests/unit/test_http_errors.py @@ -166,22 +166,24 @@ class TestHttpErrors(unittest.TestCase): @httpretty.activate def test_get_with_status_code_mismatch(self): """ - Check that an exception is raised if a get returns a - status code that doesn't match the JSON code + Check that a mismatched HTTP status code still returns the + JSON status code for get requests. """ - data = {"message": "Test Message", "code": 200} - self._register_uri(httpretty.GET, data=data, status=202) - with self.assertRaises(openphoto.OpenPhotoError): - self.client.get(self.test_endpoint) + data = {"message": "Test Message", "code": 202} + self._register_uri(httpretty.GET, data=data, status=200) + response = self.client.get(self.test_endpoint) + self.assertEqual(response["code"], 202) + # TODO: Status code mismatch should raise an exception + @unittest.expectedFailure @httpretty.activate def test_post_with_status_code_mismatch(self): """ - Check that an exception is raised if a post returns a - status code that doesn't match the JSON code + Check that a mismatched HTTP status code still returns the + JSON status code for post requests. """ data = {"message": "Test Message", "code": 200} self._register_uri(httpretty.POST, data=data, status=202) - with self.assertRaises(openphoto.OpenPhotoError): - self.client.post(self.test_endpoint) + response = self.client.post(self.test_endpoint) + self.assertEqual(response["code"], 202) From 4c6b41dbc3e722ade21adb289b148874582b0b26 Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 18:10:15 +0100 Subject: [PATCH 08/12] Don't test the tag/create endpoint, as it is redundant and will be removed. See #38 for details. --- tests/unit/test_tags.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/unit/test_tags.py b/tests/unit/test_tags.py index c4ecd4a..f915b99 100644 --- a/tests/unit/test_tags.py +++ b/tests/unit/test_tags.py @@ -35,18 +35,6 @@ class TestTagsList(TestTags): self.assertEqual(result[1].id, "tag2") self.assertEqual(result[1].count, 5) -class TestTagCreate(TestTags): - # TODO: should return a tag object, not a result dict - @unittest.expectedFailure - @mock.patch.object(openphoto.OpenPhoto, 'post') - def test_tag_create(self, mock_post): - """Check that a tag can be created""" - mock_post.return_value = self._return_value(self.test_tags_dict[0]) - result = self.client.tag.create(tag="Test", foo="bar") - mock_post.assert_called_with("/tag/create.json", tag="Test", foo="bar") - self.assertEqual(result.id, "tag1") - self.assertEqual(result.count, 11) - class TestTagDelete(TestTags): @mock.patch.object(openphoto.OpenPhoto, 'post') def test_tag_delete(self, mock_post): From 33c700a3cdb53499b8558b0bb04b710bac441594 Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 18:15:28 +0100 Subject: [PATCH 09/12] Ensure album cover is updated to a Photo object during Album() initialisation --- openphoto/objects.py | 2 +- tests/unit/test_albums.py | 35 ++++++++++++++--------------------- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/openphoto/objects.py b/openphoto/objects.py index 8e40cc7..4d1babb 100644 --- a/openphoto/objects.py +++ b/openphoto/objects.py @@ -164,9 +164,9 @@ class Tag(OpenPhotoObject): class Album(OpenPhotoObject): def __init__(self, openphoto, json_dict): - OpenPhotoObject.__init__(self, openphoto, json_dict) self.photos = None self.cover = None + OpenPhotoObject.__init__(self, openphoto, json_dict) self._update_fields_with_objects() def _update_fields_with_objects(self): diff --git a/tests/unit/test_albums.py b/tests/unit/test_albums.py index 5751688..0b1b5f4 100644 --- a/tests/unit/test_albums.py +++ b/tests/unit/test_albums.py @@ -58,7 +58,6 @@ class TestAlbumsList(TestAlbums): self.assertEqual(result[1].cover.tags, ["tag3", "tag4"]) class TestAlbumCreate(TestAlbums): - # TODO: cover should be updated to Photo object @mock.patch.object(openphoto.OpenPhoto, 'post') def test_album_create(self, mock_post): """Check that an album can be created""" @@ -68,8 +67,8 @@ class TestAlbumCreate(TestAlbums): foo="bar") self.assertEqual(result.id, "1") self.assertEqual(result.name, "Album 1") - # self.assertEqual(result.cover.id, "1a") - # self.assertEqual(result.cover.tags, ["tag1", "tag2"]) + self.assertEqual(result.cover.id, "1a") + self.assertEqual(result.cover.tags, ["tag1", "tag2"]) class TestAlbumDelete(TestAlbums): @mock.patch.object(openphoto.OpenPhoto, 'post') @@ -180,7 +179,6 @@ class TestAlbumRemovePhotos(TestAlbums): self.test_albums[0].remove_photos(["Photo Objects"]) class TestAlbumUpdate(TestAlbums): - # TODO: cover should be updated to Photo object @mock.patch.object(openphoto.OpenPhoto, 'post') def test_album_update(self, mock_post): """Check that an album can be updated""" @@ -189,10 +187,9 @@ class TestAlbumUpdate(TestAlbums): mock_post.assert_called_with("/album/1/update.json", name="Test") self.assertEqual(result.id, "2") self.assertEqual(result.name, "Album 2") - # self.assertEqual(result.cover.id, "2b") - # self.assertEqual(result.cover.tags, ["tag3", "tag4"]) + self.assertEqual(result.cover.id, "2b") + self.assertEqual(result.cover.tags, ["tag3", "tag4"]) - # TODO: cover should be updated to Photo object @mock.patch.object(openphoto.OpenPhoto, 'post') def test_album_update_id(self, mock_post): """Check that an album can be updated using its ID""" @@ -201,10 +198,9 @@ class TestAlbumUpdate(TestAlbums): mock_post.assert_called_with("/album/1/update.json", name="Test") self.assertEqual(result.id, "2") self.assertEqual(result.name, "Album 2") - # self.assertEqual(result.cover.id, "2b") - # self.assertEqual(result.cover.tags, ["tag3", "tag4"]) + self.assertEqual(result.cover.id, "2b") + self.assertEqual(result.cover.tags, ["tag3", "tag4"]) - # TODO: cover should be updated to Photo object @mock.patch.object(openphoto.OpenPhoto, 'post') def test_album_object_update(self, mock_post): """Check that an album can be updated using the album object directly""" @@ -214,11 +210,10 @@ class TestAlbumUpdate(TestAlbums): mock_post.assert_called_with("/album/1/update.json", name="Test") self.assertEqual(album.id, "2") self.assertEqual(album.name, "Album 2") - # self.assertEqual(album.cover.id, "2b") - # self.assertEqual(album.cover.tags, ["tag3", "tag4"]) + self.assertEqual(album.cover.id, "2b") + self.assertEqual(album.cover.tags, ["tag3", "tag4"]) class TestAlbumView(TestAlbums): - # TODO: cover should be updated to Photo object @mock.patch.object(openphoto.OpenPhoto, 'get') def test_album_view(self, mock_get): """Check that an album can be viewed""" @@ -227,10 +222,9 @@ class TestAlbumView(TestAlbums): mock_get.assert_called_with("/album/1/view.json", name="Test") self.assertEqual(result.id, "2") self.assertEqual(result.name, "Album 2") - # self.assertEqual(result.cover.id, "2b") - # self.assertEqual(result.cover.tags, ["tag3", "tag4"]) + self.assertEqual(result.cover.id, "2b") + self.assertEqual(result.cover.tags, ["tag3", "tag4"]) - # TODO: cover should be updated to Photo object @mock.patch.object(openphoto.OpenPhoto, 'get') def test_album_view_id(self, mock_get): """Check that an album can be viewed using its ID""" @@ -239,10 +233,9 @@ class TestAlbumView(TestAlbums): mock_get.assert_called_with("/album/1/view.json", name="Test") self.assertEqual(result.id, "2") self.assertEqual(result.name, "Album 2") - # self.assertEqual(result.cover.id, "2b") - # self.assertEqual(result.cover.tags, ["tag3", "tag4"]) + self.assertEqual(result.cover.id, "2b") + self.assertEqual(result.cover.tags, ["tag3", "tag4"]) - # TODO: cover should be updated to Photo object @mock.patch.object(openphoto.OpenPhoto, 'get') def test_album_object_view(self, mock_get): """Check that an album can be viewed using the album object directly""" @@ -252,6 +245,6 @@ class TestAlbumView(TestAlbums): mock_get.assert_called_with("/album/1/view.json", name="Test") self.assertEqual(album.id, "2") self.assertEqual(album.name, "Album 2") - # self.assertEqual(album.cover.id, "2b") - # self.assertEqual(album.cover.tags, ["tag3", "tag4"]) + self.assertEqual(album.cover.id, "2b") + self.assertEqual(album.cover.tags, ["tag3", "tag4"]) From a23dbb36c5da252f93f6a5de08021fcdbc11f168 Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 18:16:09 +0100 Subject: [PATCH 10/12] Remove invalid comment --- tests/unit/test_albums.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/test_albums.py b/tests/unit/test_albums.py index 0b1b5f4..a2fb844 100644 --- a/tests/unit/test_albums.py +++ b/tests/unit/test_albums.py @@ -39,7 +39,6 @@ class TestAlbumsList(TestAlbums): self.assertEqual(result[1].id, "2") self.assertEqual(result[1].name, "Album 2") - # TODO: cover should be updated to Photo object @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'get') def test_albums_list_returns_cover_photos(self, mock_get): From 639836c2489efc35c719be264f7b008f0af32d00 Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 18:17:48 +0100 Subject: [PATCH 11/12] object.add/remove_photos should accept photos list as first parameter (even though it's not implemented yet) --- openphoto/objects.py | 4 ++-- tests/unit/test_albums.py | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/openphoto/objects.py b/openphoto/objects.py index 4d1babb..8072188 100644 --- a/openphoto/objects.py +++ b/openphoto/objects.py @@ -196,10 +196,10 @@ class Album(OpenPhotoObject): def form(self, **kwds): raise NotImplementedError() - def add_photos(self, **kwds): + def add_photos(self, photos, **kwds): raise NotImplementedError() - def remove_photos(self, **kwds): + def remove_photos(self, photos, **kwds): raise NotImplementedError() def update(self, **kwds): diff --git a/tests/unit/test_albums.py b/tests/unit/test_albums.py index a2fb844..88054f1 100644 --- a/tests/unit/test_albums.py +++ b/tests/unit/test_albums.py @@ -147,8 +147,6 @@ class TestAlbumAddPhotos(TestAlbums): with self.assertRaises(NotImplementedError): self.client.album.add_photos("1", ["Photo Objects"]) - # TODO: object.add_photos should accept photos list as first parameter - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'post') def test_album_object_add_photos(self, _): """ If album.add_photos gets implemented, write a test! """ @@ -169,8 +167,6 @@ class TestAlbumRemovePhotos(TestAlbums): with self.assertRaises(NotImplementedError): self.client.album.remove_photos("1", ["Photo Objects"]) - # TODO: object.remove_photos should accept photos list as first parameter - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'post') def test_album_object_remove_photos(self, _): """ If album.remove_photos gets implemented, write a test! """ From b4861e4332b77c2a2961c5b9e4f6b659e6fecf0c Mon Sep 17 00:00:00 2001 From: sneakypete81 Date: Sun, 30 Jun 2013 18:25:05 +0100 Subject: [PATCH 12/12] Fix a couple of unit tests --- tests/unit/test_albums.py | 3 +-- tests/unit/test_http_errors.py | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_albums.py b/tests/unit/test_albums.py index 88054f1..3a5a5b2 100644 --- a/tests/unit/test_albums.py +++ b/tests/unit/test_albums.py @@ -39,7 +39,6 @@ class TestAlbumsList(TestAlbums): self.assertEqual(result[1].id, "2") self.assertEqual(result[1].name, "Album 2") - @unittest.expectedFailure @mock.patch.object(openphoto.OpenPhoto, 'get') def test_albums_list_returns_cover_photos(self, mock_get): """Check that the album list returns cover photo objects""" @@ -52,7 +51,7 @@ class TestAlbumsList(TestAlbums): self.assertEqual(result[0].cover.id, "1a") self.assertEqual(result[0].cover.tags, ["tag1", "tag2"]) self.assertEqual(result[1].id, "2") - self.assertEqual(result[0].name, "Album 2") + self.assertEqual(result[1].name, "Album 2") self.assertEqual(result[1].cover.id, "2b") self.assertEqual(result[1].cover.tags, ["tag3", "tag4"]) diff --git a/tests/unit/test_http_errors.py b/tests/unit/test_http_errors.py index d5c5988..557f7ca 100644 --- a/tests/unit/test_http_errors.py +++ b/tests/unit/test_http_errors.py @@ -174,16 +174,14 @@ class TestHttpErrors(unittest.TestCase): response = self.client.get(self.test_endpoint) self.assertEqual(response["code"], 202) - # TODO: Status code mismatch should raise an exception - @unittest.expectedFailure @httpretty.activate def test_post_with_status_code_mismatch(self): """ Check that a mismatched HTTP status code still returns the JSON status code for post requests. """ - data = {"message": "Test Message", "code": 200} - self._register_uri(httpretty.POST, data=data, status=202) + data = {"message": "Test Message", "code": 202} + self._register_uri(httpretty.POST, data=data, status=200) response = self.client.post(self.test_endpoint) self.assertEqual(response["code"], 202)