Delete methods should raise exception on failure, rather than returning False

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
 

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):
         """

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):
         """

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):
         """