Refactor cinder store tests[2/2]

This patch aims at a refactoring effort that would remove
duplicate tests (current and future) by moving them into a
common base class which is called via both single and multi store
test modules with their specific configurations.

This has a lot of benefits:

1) Removes duplicate code
2) Makes addition of new tests easier and cleaner
3) Ensuring a new method/code path added is tested in both
   single and multi store configurations
4) Fixing issues detected while refactoring methods
   (Eg: tests for add method in test_multistore_cinder were not
    passing the hashing_algo parameter which is currently handled
    by the backward compat code (back_compat_add decorator) but
    those tests will break when we remove backward compatibility)

Change-Id: I12569af5623f1cd7803c00a6c3b9eb211f15b6fd

3 files changed, 126 insertions, 191 deletions

diff --git a/glance_store/tests/unit/test_cinder_base.py b/glance_store/tests/unit/test_cinder_base.py
index 45d786a..cb8175c 100644
--- a/glance_store/tests/unit/test_cinder_base.py
+++ b/glance_store/tests/unit/test_cinder_base.py
@@ -14,6 +14,9 @@
 #    under the License.
 
 import contextlib
+import hashlib
+import io
+import math
 import os
 from unittest import mock
 
@@ -26,6 +29,7 @@ import uuid
 
 from os_brick.initiator import connector
 from oslo_concurrency import processutils
+from oslo_utils.secretutils import md5
 from oslo_utils import units
 
 from glance_store.common import attachment_state_manager
@@ -369,3 +373,90 @@ class TestCinderStoreBase(object):
 
             image_size = self.store.get_size(loc, context=self.context)
             self.assertEqual(expected_image_size, image_size)
+
+    def _test_cinder_add(self, fake_volume, volume_file, size_kb=5,
+                         verifier=None, backend='glance_store',
+                         fail_resize=False, is_multi_store=False):
+        expected_image_id = str(uuid.uuid4())
+        expected_size = size_kb * units.Ki
+        expected_file_contents = b"*" * expected_size
+        image_file = six.BytesIO(expected_file_contents)
+        expected_checksum = md5(expected_file_contents,
+                                usedforsecurity=False).hexdigest()
+        expected_multihash = hashlib.sha256(expected_file_contents).hexdigest()
+
+        expected_location = 'cinder://%s' % fake_volume.id
+        if is_multi_store:
+            # Default backend is 'glance_store' for single store but in case
+            # of multi store, if the backend option is not passed, we should
+            # assign it to the default i.e. 'cinder1'
+            if backend == 'glance_store':
+                backend = 'cinder1'
+            expected_location = 'cinder://%s/%s' % (backend, fake_volume.id)
+        self.config(cinder_volume_type='some_type', group=backend)
+
+        fake_client = mock.MagicMock(auth_token=None, management_url=None)
+        fake_volume.manager.get.return_value = fake_volume
+        fake_volumes = mock.MagicMock(create=mock.Mock(
+            return_value=fake_volume))
+
+        @contextlib.contextmanager
+        def fake_open(client, volume, mode):
+            self.assertEqual('wb', mode)
+            yield volume_file
+
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \
+                mock.patch.object(self.store, '_open_cinder_volume',
+                                  side_effect=fake_open), \
+                mock.patch.object(
+                    cinder.Store, '_wait_resize_device') as mock_wait_resize:
+            if fail_resize:
+                mock_wait_resize.side_effect = exceptions.BackendException()
+            mock_cc.return_value = mock.MagicMock(client=fake_client,
+                                                  volumes=fake_volumes)
+            loc, size, checksum, multihash, metadata = self.store.add(
+                expected_image_id, image_file, expected_size, self.hash_algo,
+                self.context, verifier)
+            self.assertEqual(expected_location, loc)
+            self.assertEqual(expected_size, size)
+            self.assertEqual(expected_checksum, checksum)
+            self.assertEqual(expected_multihash, multihash)
+            fake_volumes.create.assert_called_once_with(
+                1,
+                name='image-%s' % expected_image_id,
+                metadata={'image_owner': self.context.project_id,
+                          'glance_image_id': expected_image_id,
+                          'image_size': str(expected_size)},
+                volume_type='some_type')
+            if is_multi_store:
+                self.assertEqual(backend, metadata["store"])
+
+    def test__get_device_size(self):
+        fake_data = b"fake binary data"
+        fake_len = int(math.ceil(float(len(fake_data)) / units.Gi))
+        fake_file = io.BytesIO(fake_data)
+        dev_size = cinder.Store._get_device_size(fake_file)
+        self.assertEqual(fake_len, dev_size)
+
+    @mock.patch.object(time, 'sleep')
+    def test__wait_resize_device_resized(self, mock_sleep):
+        fake_vol = mock.MagicMock()
+        fake_vol.size = 2
+        fake_file = io.BytesIO(b"fake binary data")
+        with mock.patch.object(
+                cinder.Store, '_get_device_size') as mock_get_dev_size:
+            mock_get_dev_size.side_effect = [1, 2]
+            cinder.Store._wait_resize_device(fake_vol, fake_file)
+
+    @mock.patch.object(time, 'sleep')
+    def test__wait_resize_device_fails(self, mock_sleep):
+        fake_vol = mock.MagicMock()
+        fake_vol.size = 2
+        fake_file = io.BytesIO(b"fake binary data")
+        with mock.patch.object(
+                cinder.Store, '_get_device_size',
+                return_value=1):
+            self.assertRaises(
+                exceptions.BackendException,
+                cinder.Store._wait_resize_device,
+                fake_vol, fake_file)
diff --git a/glance_store/tests/unit/test_cinder_store.py b/glance_store/tests/unit/test_cinder_store.py
index ab4f502..77221e2 100644
--- a/glance_store/tests/unit/test_cinder_store.py
+++ b/glance_store/tests/unit/test_cinder_store.py
@@ -13,19 +13,14 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import contextlib
 import errno
-import hashlib
 import io
-import math
 from unittest import mock
 
 import six
 import sys
-import time
 import uuid
 
-from oslo_utils.secretutils import md5
 from oslo_utils import units
 
 from glance_store import exceptions
@@ -116,51 +111,6 @@ class TestCinderStore(base.StoreBaseTest,
     def test_cinder_get_size_with_metadata(self):
         self._test_cinder_get_size_with_metadata()
 
-    def _test_cinder_add(self, fake_volume, volume_file, size_kb=5,
-                         verifier=None, fail_resize=False):
-        expected_image_id = str(uuid.uuid4())
-        expected_size = size_kb * units.Ki
-        expected_file_contents = b"*" * expected_size
-        image_file = six.BytesIO(expected_file_contents)
-        expected_checksum = md5(expected_file_contents,
-                                usedforsecurity=False).hexdigest()
-        expected_multihash = hashlib.sha256(expected_file_contents).hexdigest()
-        expected_location = 'cinder://%s' % fake_volume.id
-        fake_client = mock.MagicMock(auth_token=None, management_url=None)
-        fake_volume.manager.get.return_value = fake_volume
-        fake_volumes = mock.MagicMock(create=mock.Mock(
-            return_value=fake_volume))
-        self.config(cinder_volume_type='some_type')
-
-        @contextlib.contextmanager
-        def fake_open(client, volume, mode):
-            self.assertEqual('wb', mode)
-            yield volume_file
-
-        with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \
-                mock.patch.object(self.store, '_open_cinder_volume',
-                                  side_effect=fake_open), \
-                mock.patch.object(
-                    cinder.Store, '_wait_resize_device') as mock_wait_resize:
-            if fail_resize:
-                mock_wait_resize.side_effect = exceptions.BackendException()
-            mock_cc.return_value = mock.MagicMock(client=fake_client,
-                                                  volumes=fake_volumes)
-            loc, size, checksum, multihash, _ = self.store.add(
-                expected_image_id, image_file, expected_size, self.hash_algo,
-                self.context, verifier)
-            self.assertEqual(expected_location, loc)
-            self.assertEqual(expected_size, size)
-            self.assertEqual(expected_checksum, checksum)
-            self.assertEqual(expected_multihash, multihash)
-            fake_volumes.create.assert_called_once_with(
-                1,
-                name='image-%s' % expected_image_id,
-                metadata={'image_owner': self.context.project_id,
-                          'glance_image_id': expected_image_id,
-                          'image_size': str(expected_size)},
-                volume_type='some_type')
-
     def test_cinder_add(self):
         fake_volume = mock.MagicMock(id=str(uuid.uuid4()),
                                      status='available',
@@ -189,6 +139,16 @@ class TestCinderStore(base.StoreBaseTest,
                               self._test_cinder_add, fake_volume, volume_file)
         fake_volume.delete.assert_called_once_with()
 
+    def test_cinder_add_fail_resize(self):
+        volume_file = io.BytesIO()
+        fake_volume = mock.MagicMock(id=str(uuid.uuid4()),
+                                     status='available',
+                                     size=1)
+        self.assertRaises(exceptions.BackendException,
+                          self._test_cinder_add, fake_volume, volume_file,
+                          fail_resize=True)
+        fake_volume.delete.assert_called_once()
+
     def test_cinder_delete(self):
         fake_client = mock.MagicMock(auth_token=None, management_url=None)
         fake_volume_uuid = str(uuid.uuid4())
@@ -215,43 +175,3 @@ class TestCinderStore(base.StoreBaseTest,
         # warning
         self.config(cinder_volume_type='some_random_type')
         self._test_configure_add_invalid_type()
-
-    def test__get_device_size(self):
-        fake_data = b"fake binary data"
-        fake_len = int(math.ceil(float(len(fake_data)) / units.Gi))
-        fake_file = io.BytesIO(fake_data)
-        dev_size = cinder.Store._get_device_size(fake_file)
-        self.assertEqual(fake_len, dev_size)
-
-    @mock.patch.object(time, 'sleep')
-    def test__wait_resize_device_resized(self, mock_sleep):
-        fake_vol = mock.MagicMock()
-        fake_vol.size = 2
-        fake_file = io.BytesIO(b"fake binary data")
-        with mock.patch.object(
-                cinder.Store, '_get_device_size') as mock_get_dev_size:
-            mock_get_dev_size.side_effect = [1, 2]
-            cinder.Store._wait_resize_device(fake_vol, fake_file)
-
-    @mock.patch.object(time, 'sleep')
-    def test__wait_resize_device_fails(self, mock_sleep):
-        fake_vol = mock.MagicMock()
-        fake_vol.size = 2
-        fake_file = io.BytesIO(b"fake binary data")
-        with mock.patch.object(
-                cinder.Store, '_get_device_size',
-                return_value=1):
-            self.assertRaises(
-                exceptions.BackendException,
-                cinder.Store._wait_resize_device,
-                fake_vol, fake_file)
-
-    def test_cinder_add_fail_resize(self):
-        volume_file = io.BytesIO()
-        fake_volume = mock.MagicMock(id=str(uuid.uuid4()),
-                                     status='available',
-                                     size=1)
-        self.assertRaises(exceptions.BackendException,
-                          self._test_cinder_add, fake_volume, volume_file,
-                          fail_resize=True)
-        fake_volume.delete.assert_called_once()
diff --git a/glance_store/tests/unit/test_multistore_cinder.py b/glance_store/tests/unit/test_multistore_cinder.py
index 82bf7a2..baba9a2 100644
--- a/glance_store/tests/unit/test_multistore_cinder.py
+++ b/glance_store/tests/unit/test_multistore_cinder.py
@@ -13,20 +13,16 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import contextlib
 import errno
 import io
-import math
 from unittest import mock
 
 import six
 import sys
-import time
 import uuid
 
 import fixtures
 from oslo_config import cfg
-from oslo_utils.secretutils import md5
 from oslo_utils import units
 
 import glance_store as store
@@ -85,6 +81,7 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
                                       user_id='fake_user',
                                       auth_token='fake_token',
                                       project_id='fake_project')
+        self.hash_algo = 'sha256'
         self.fake_admin_context = mock.MagicMock()
         self.fake_admin_context.elevated.return_value = mock.MagicMock(
             service_catalog=fake_sc,
@@ -226,58 +223,12 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
     def test_cinder_get_size_with_metadata(self):
         self._test_cinder_get_size_with_metadata(is_multi_store=True)
 
-    def _test_cinder_add(self, fake_volume, volume_file, size_kb=5,
-                         verifier=None, backend="cinder1", fail_resize=False):
-        expected_image_id = str(uuid.uuid4())
-        expected_size = size_kb * units.Ki
-        expected_file_contents = b"*" * expected_size
-        image_file = six.BytesIO(expected_file_contents)
-        expected_checksum = md5(expected_file_contents,
-                                usedforsecurity=False).hexdigest()
-        expected_location = 'cinder://%s/%s' % (backend, fake_volume.id)
-        fake_client = mock.MagicMock(auth_token=None, management_url=None)
-        fake_volume.manager.get.return_value = fake_volume
-        fake_volumes = mock.MagicMock(
-            create=mock.Mock(return_value=fake_volume))
-        self.config(cinder_volume_type='some_type', group=backend)
-
-        @contextlib.contextmanager
-        def fake_open(client, volume, mode):
-            self.assertEqual('wb', mode)
-            yield volume_file
-
-        with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \
-                mock.patch.object(self.store, '_open_cinder_volume',
-                                  side_effect=fake_open), \
-                mock.patch.object(
-                    cinder.Store, '_wait_resize_device') as mock_wait_resize:
-            if fail_resize:
-                mock_wait_resize.side_effect = exceptions.BackendException()
-            mock_cc.return_value = mock.MagicMock(client=fake_client,
-                                                  volumes=fake_volumes)
-            loc, size, checksum, metadata = self.store.add(expected_image_id,
-                                                           image_file,
-                                                           expected_size,
-                                                           self.context,
-                                                           verifier)
-            self.assertEqual(expected_location, loc)
-            self.assertEqual(expected_size, size)
-            self.assertEqual(expected_checksum, checksum)
-            fake_volumes.create.assert_called_once_with(
-                1,
-                name='image-%s' % expected_image_id,
-                metadata={'image_owner': self.context.project_id,
-                          'glance_image_id': expected_image_id,
-                          'image_size': str(expected_size)},
-                volume_type='some_type')
-            self.assertEqual(backend, metadata["store"])
-
     def test_cinder_add(self):
         fake_volume = mock.MagicMock(id=str(uuid.uuid4()),
                                      status='available',
                                      size=1)
         volume_file = six.BytesIO()
-        self._test_cinder_add(fake_volume, volume_file)
+        self._test_cinder_add(fake_volume, volume_file, is_multi_store=True)
 
     def test_cinder_add_with_verifier(self):
         fake_volume = mock.MagicMock(id=str(uuid.uuid4()),
@@ -285,7 +236,8 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
                                      size=1)
         volume_file = six.BytesIO()
         verifier = mock.MagicMock()
-        self._test_cinder_add(fake_volume, volume_file, 1, verifier)
+        self._test_cinder_add(fake_volume, volume_file, 1, verifier,
+                              is_multi_store=True)
         verifier.update.assert_called_with(b"*" * units.Ki)
 
     def test_cinder_add_volume_full(self):
@@ -297,25 +249,10 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
                                      size=1)
         with mock.patch.object(volume_file, 'write', side_effect=e):
             self.assertRaises(exceptions.StorageFull,
-                              self._test_cinder_add, fake_volume, volume_file)
+                              self._test_cinder_add, fake_volume, volume_file,
+                              is_multi_store=True)
         fake_volume.delete.assert_called_once_with()
 
-    def test_cinder_delete(self):
-        fake_client = mock.MagicMock(auth_token=None, management_url=None)
-        fake_volume_uuid = str(uuid.uuid4())
-        fake_volumes = mock.MagicMock(delete=mock.Mock())
-
-        with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc:
-            mocked_cc.return_value = mock.MagicMock(client=fake_client,
-                                                    volumes=fake_volumes)
-
-            uri = 'cinder://cinder1/%s' % fake_volume_uuid
-            loc = location.get_location_from_uri_and_backend(uri,
-                                                             "cinder1",
-                                                             conf=self.conf)
-            self.store.delete(loc, context=self.context)
-            fake_volumes.delete.assert_called_once_with(fake_volume_uuid)
-
     def test_cinder_add_different_backend(self):
         self.store = cinder.Store(self.conf, backend="cinder2")
         self.store.configure()
@@ -325,37 +262,8 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
                                      status='available',
                                      size=1)
         volume_file = six.BytesIO()
-        self._test_cinder_add(fake_volume, volume_file, backend="cinder2")
-
-    def test__get_device_size(self):
-        fake_data = b"fake binary data"
-        fake_len = int(math.ceil(float(len(fake_data)) / units.Gi))
-        fake_file = io.BytesIO(fake_data)
-        dev_size = cinder.Store._get_device_size(fake_file)
-        self.assertEqual(fake_len, dev_size)
-
-    @mock.patch.object(time, 'sleep')
-    def test__wait_resize_device_resized(self, mock_sleep):
-        fake_vol = mock.MagicMock()
-        fake_vol.size = 2
-        fake_file = io.BytesIO(b"fake binary data")
-        with mock.patch.object(
-                cinder.Store, '_get_device_size') as mock_get_dev_size:
-            mock_get_dev_size.side_effect = [1, 2]
-            cinder.Store._wait_resize_device(fake_vol, fake_file)
-
-    @mock.patch.object(time, 'sleep')
-    def test__wait_resize_device_fails(self, mock_sleep):
-        fake_vol = mock.MagicMock()
-        fake_vol.size = 2
-        fake_file = io.BytesIO(b"fake binary data")
-        with mock.patch.object(
-                cinder.Store, '_get_device_size',
-                return_value=1):
-            self.assertRaises(
-                exceptions.BackendException,
-                cinder.Store._wait_resize_device,
-                fake_vol, fake_file)
+        self._test_cinder_add(fake_volume, volume_file, backend="cinder2",
+                              is_multi_store=True)
 
     def test_cinder_add_fail_resize(self):
         volume_file = io.BytesIO()
@@ -364,5 +272,21 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
                                      size=1)
         self.assertRaises(exceptions.BackendException,
                           self._test_cinder_add, fake_volume, volume_file,
-                          fail_resize=True)
+                          fail_resize=True, is_multi_store=True)
         fake_volume.delete.assert_called_once()
+
+    def test_cinder_delete(self):
+        fake_client = mock.MagicMock(auth_token=None, management_url=None)
+        fake_volume_uuid = str(uuid.uuid4())
+        fake_volumes = mock.MagicMock(delete=mock.Mock())
+
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc:
+            mocked_cc.return_value = mock.MagicMock(client=fake_client,
+                                                    volumes=fake_volumes)
+
+            uri = 'cinder://cinder1/%s' % fake_volume_uuid
+            loc = location.get_location_from_uri_and_backend(uri,
+                                                             "cinder1",
+                                                             conf=self.conf)
+            self.store.delete(loc, context=self.context)
+            fake_volumes.delete.assert_called_once_with(fake_volume_uuid)