From b1e757884aa1a10c939337ba83caf9bf8d615305 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Tue, 9 Feb 2021 12:15:42 -0800 Subject: [PATCH] Fix nonsensical test mocks and assertions The test_inject_image_metadata test was doing some completely nonsensical mock behavior, like mocking something after we have run the method under test, and re-mocking the image repo with the thing that was supposed to change so that the assertions were not even checking the actual behavior. None of the three tests were actually testing the desired behavior. In the case of the admin_user test, it was also asserting that things were not run that *should* be, probably because the mocks were done after the execution and asserting that they *were* run would never work. This also wires up the image and image_repo mocks that were in setUp(), but unused (and incorrect) so that we can assert the calls are actually as we expect. Change-Id: Ifc2f49d49ff62c49c7dd1d2174a78bb1451b7bf3 --- .../plugins/test_inject_image_metadata.py | 36 +++++++------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/glance/tests/unit/async_/flows/plugins/test_inject_image_metadata.py b/glance/tests/unit/async_/flows/plugins/test_inject_image_metadata.py index b5fdc65d8a..9671bc99c5 100644 --- a/glance/tests/unit/async_/flows/plugins/test_inject_image_metadata.py +++ b/glance/tests/unit/async_/flows/plugins/test_inject_image_metadata.py @@ -52,14 +52,16 @@ class TestInjectImageMetadataTask(test_utils.BaseTestCase): self.context = mock.MagicMock() self.img_repo = mock.MagicMock() self.task_repo = mock.MagicMock() - self.image_id = mock.MagicMock() + self.image_id = UUID1 self.gateway = gateway.Gateway() self.task_factory = domain.TaskFactory() self.img_factory = self.gateway.get_image_factory(self.context) self.image = self.img_factory.new_image(image_id=UUID1, disk_format='qcow2', - container_format='bare') + container_format='bare', + extra_properties={}) + self.img_repo.get.return_value = self.image task_input = { "import_from": "http://cloud.foo/image.qcow2", @@ -83,16 +85,10 @@ class TestInjectImageMetadataTask(test_utils.BaseTestCase): self.config(inject={"test": "abc"}, group='inject_metadata_properties') - with mock.patch.object(self.img_repo, 'get') as get_mock: - image = mock.MagicMock(image_id=self.image_id, - extra_properties={"test": "abc"}) - get_mock.return_value = image - - with mock.patch.object(self.img_repo, 'save') as save_mock: - inject_image_metadata.execute() - get_mock.assert_called_once_with(self.image_id) - save_mock.assert_called_once_with(image) - self.assertEqual({"test": "abc"}, image.extra_properties) + inject_image_metadata.execute() + self.img_repo.get.assert_called_once_with(self.image_id) + self.img_repo.save.assert_called_once_with(self.image) + self.assertEqual({"test": "abc"}, self.image.extra_properties) def test_inject_image_metadata_using_admin_user(self): context = test_unit_utils.get_fake_context(roles='admin') @@ -104,12 +100,8 @@ class TestInjectImageMetadataTask(test_utils.BaseTestCase): group='inject_metadata_properties') inject_image_metadata.execute() - - with mock.patch.object(self.img_repo, 'get') as get_mock: - get_mock.assert_not_called() - - with mock.patch.object(self.img_repo, 'save') as save_mock: - save_mock.assert_not_called() + self.img_repo.get.assert_called_once_with(UUID1) + self.img_repo.save.assert_called_once_with(self.image) def test_inject_image_metadata_empty(self): context = test_unit_utils.get_fake_context(roles='member') @@ -120,9 +112,5 @@ class TestInjectImageMetadataTask(test_utils.BaseTestCase): self.config(inject={}, group='inject_metadata_properties') inject_image_metadata.execute() - - with mock.patch.object(self.img_repo, 'get') as get_mock: - get_mock.assert_not_called() - - with mock.patch.object(self.img_repo, 'save') as save_mock: - save_mock.assert_not_called() + self.img_repo.get.assert_not_called() + self.img_repo.save.assert_not_called()