openstack/rally
Code Issues Proposed changes

Improve unit tests for UserGenerator

Browse Source 
Our experiments with unit tests for UserGenerator, especially those for
UserGenerator.create_tenants(), have revealed that it is the excessive use
of mocks:

    ...
    context = {"admin": {"endpoint": mock.MagicMock()},
               "task": mock.MagicMock()}
    ...

that leads to race conditions:
    http://paste.openstack.org/show/137037/

Here, we fix the mocking of the task object that caused these race conditions
and unify the UTs code.

We also modify the UserGenerator so that it uses the more efficient collections.deque
instead of ordinary lists.

Change-Id: I8696287ecf9bc8bb44f67f70c81c6c93fdb8acbe
Closes-Bug: #1394158
This commit is contained in:
Mikhail Dubov 2014-11-19 14:42:56 +04:00
parent d79a96a507
commit e7c5ed3805
2 changed files with 14 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
rally/benchmark/context/users.py
View File

@ -13,6 +13,7 @@
# License for the specific language governing permissions and limitations # License for the specific language governing permissions and limitations
# under the License. # under the License.
import collections
import uuid import uuid
from oslo.config import cfg from oslo.config import cfg
@ -134,7 +135,7 @@ class UserGenerator(base.Context):
def _create_tenants(self): def _create_tenants(self):
threads = self.config["resource_management_workers"] threads = self.config["resource_management_workers"]
tenants = [] tenants = collections.deque()
def publish(queue): def publish(queue):
for i in range(self.config["tenants"]): for i in range(self.config["tenants"]):
@ -153,14 +154,14 @@ class UserGenerator(base.Context):
# NOTE(msdubov): cosume() will fill the tenants list in the closure. # NOTE(msdubov): cosume() will fill the tenants list in the closure.
broker.run(publish, consume, threads) broker.run(publish, consume, threads)
return tenants return list(tenants)
def _create_users(self): def _create_users(self):
# NOTE(msdubov): This should be called after _create_tenants(). # NOTE(msdubov): This should be called after _create_tenants().
threads = self.config["resource_management_workers"] threads = self.config["resource_management_workers"]
users_per_tenant = self.config["users_per_tenant"] users_per_tenant = self.config["users_per_tenant"]
users = [] users = collections.deque()
def publish(queue): def publish(queue):
for tenant in self.context["tenants"]: for tenant in self.context["tenants"]:
@ -191,7 +192,7 @@ class UserGenerator(base.Context):
# NOTE(msdubov): cosume() will fill the users list in the closure. # NOTE(msdubov): cosume() will fill the users list in the closure.
broker.run(publish, consume, threads) broker.run(publish, consume, threads)
return users return list(users)
def _delete_tenants(self): def _delete_tenants(self):
threads = self.config["resource_management_workers"] threads = self.config["resource_management_workers"]

34
tests/unit/benchmark/context/test_users.py
View File

@ -38,7 +38,7 @@ class UserGeneratorTestCase(test.TestCase):
} }
}, },
"admin": {"endpoint": mock.MagicMock()}, "admin": {"endpoint": mock.MagicMock()},
"task": mock.MagicMock() "task": {"uuid": "task_id"}
} }
def setUp(self): def setUp(self):
@ -70,9 +70,7 @@ class UserGeneratorTestCase(test.TestCase):
clients.nova.return_value = nova_admin clients.nova.return_value = nova_admin
nova_admin.networks.list.return_value = networks nova_admin.networks.list.return_value = networks
nova_admin.networks.get = fake_get_network nova_admin.networks.get = fake_get_network
context = {"admin": {"endpoint": mock.MagicMock()}, user_generator = users.UserGenerator(self.context)
"task": mock.MagicMock()}
user_generator = users.UserGenerator(context)
user_generator.context["tenants"] = [tenant1, tenant2] user_generator.context["tenants"] = [tenant1, tenant2]
user_generator._remove_associated_networks() user_generator._remove_associated_networks()
mock_check_service_status.assert_called_once_with(mock.ANY, mock_check_service_status.assert_called_once_with(mock.ANY,
@ -100,9 +98,7 @@ class UserGeneratorTestCase(test.TestCase):
nova_admin.networks.list.return_value = networks nova_admin.networks.list.return_value = networks
nova_admin.networks.get = fake_get_network nova_admin.networks.get = fake_get_network
nova_admin.networks.disassociate.side_effect = Exception() nova_admin.networks.disassociate.side_effect = Exception()
context = {"admin": {"endpoint": mock.MagicMock()}, user_generator = users.UserGenerator(self.context)
"task": mock.MagicMock()}
user_generator = users.UserGenerator(context)
user_generator.context["tenants"] = [tenant1, tenant2] user_generator.context["tenants"] = [tenant1, tenant2]
user_generator._remove_associated_networks() user_generator._remove_associated_networks()
mock_check_service_status.assert_called_once_with(mock.ANY, mock_check_service_status.assert_called_once_with(mock.ANY,
@ -112,9 +108,7 @@ class UserGeneratorTestCase(test.TestCase):
@mock.patch("rally.benchmark.context.users.broker.time.sleep") @mock.patch("rally.benchmark.context.users.broker.time.sleep")
@mock.patch("rally.benchmark.context.users.keystone") @mock.patch("rally.benchmark.context.users.keystone")
def test__create_tenants(self, mock_keystone, mock_sleep): def test__create_tenants(self, mock_keystone, mock_sleep):
context = {"admin": {"endpoint": mock.MagicMock()}, user_generator = users.UserGenerator(self.context)
"task": mock.MagicMock()}
user_generator = users.UserGenerator(context)
user_generator.config["tenants"] = 2 user_generator.config["tenants"] = 2
tenants = user_generator._create_tenants() tenants = user_generator._create_tenants()
self.assertEqual(2, len(tenants)) self.assertEqual(2, len(tenants))
@ -125,9 +119,7 @@ class UserGeneratorTestCase(test.TestCase):
@mock.patch("rally.benchmark.context.users.broker.time.sleep") @mock.patch("rally.benchmark.context.users.broker.time.sleep")
@mock.patch("rally.benchmark.context.users.keystone") @mock.patch("rally.benchmark.context.users.keystone")
def test__create_users(self, mock_keystone, mock_sleep): def test__create_users(self, mock_keystone, mock_sleep):
context = {"admin": {"endpoint": mock.MagicMock()}, user_generator = users.UserGenerator(self.context)
"task": mock.MagicMock()}
user_generator = users.UserGenerator(context)
tenant1 = mock.MagicMock() tenant1 = mock.MagicMock()
tenant2 = mock.MagicMock() tenant2 = mock.MagicMock()
user_generator.context["tenants"] = [tenant1, tenant2] user_generator.context["tenants"] = [tenant1, tenant2]
@ -140,9 +132,7 @@ class UserGeneratorTestCase(test.TestCase):
@mock.patch("rally.benchmark.context.users.keystone") @mock.patch("rally.benchmark.context.users.keystone")
def test__delete_tenants(self, mock_keystone): def test__delete_tenants(self, mock_keystone):
context = {"admin": {"endpoint": mock.MagicMock()}, user_generator = users.UserGenerator(self.context)
"task": mock.MagicMock()}
user_generator = users.UserGenerator(context)
tenant1 = mock.MagicMock() tenant1 = mock.MagicMock()
tenant2 = mock.MagicMock() tenant2 = mock.MagicMock()
user_generator.context["tenants"] = [tenant1, tenant2] user_generator.context["tenants"] = [tenant1, tenant2]
@ -153,9 +143,7 @@ class UserGeneratorTestCase(test.TestCase):
def test__delete_tenants_failure(self, mock_keystone): def test__delete_tenants_failure(self, mock_keystone):
wrapped_keystone = mock_keystone.wrap.return_value wrapped_keystone = mock_keystone.wrap.return_value
wrapped_keystone.delete_project.side_effect = Exception() wrapped_keystone.delete_project.side_effect = Exception()
context = {"admin": {"endpoint": mock.MagicMock()}, user_generator = users.UserGenerator(self.context)
"task": mock.MagicMock()}
user_generator = users.UserGenerator(context)
tenant1 = mock.MagicMock() tenant1 = mock.MagicMock()
tenant2 = mock.MagicMock() tenant2 = mock.MagicMock()
user_generator.context["tenants"] = [tenant1, tenant2] user_generator.context["tenants"] = [tenant1, tenant2]
@ -164,9 +152,7 @@ class UserGeneratorTestCase(test.TestCase):
@mock.patch("rally.benchmark.context.users.keystone") @mock.patch("rally.benchmark.context.users.keystone")
def test__delete_users(self, mock_keystone): def test__delete_users(self, mock_keystone):
context = {"admin": {"endpoint": mock.MagicMock()}, user_generator = users.UserGenerator(self.context)
"task": mock.MagicMock()}
user_generator = users.UserGenerator(context)
user1 = mock.MagicMock() user1 = mock.MagicMock()
user2 = mock.MagicMock() user2 = mock.MagicMock()
user_generator.context["users"] = [user1, user2] user_generator.context["users"] = [user1, user2]
@ -177,9 +163,7 @@ class UserGeneratorTestCase(test.TestCase):
def test__delete_users_failure(self, mock_keystone): def test__delete_users_failure(self, mock_keystone):
wrapped_keystone = mock_keystone.wrap.return_value wrapped_keystone = mock_keystone.wrap.return_value
wrapped_keystone.delete_user.side_effect = Exception() wrapped_keystone.delete_user.side_effect = Exception()
context = {"admin": {"endpoint": mock.MagicMock()}, user_generator = users.UserGenerator(self.context)
"task": mock.MagicMock()}
user_generator = users.UserGenerator(context)
user1 = mock.MagicMock() user1 = mock.MagicMock()
user2 = mock.MagicMock() user2 = mock.MagicMock()
user_generator.context["users"] = [user1, user2] user_generator.context["users"] = [user1, user2]