)]}'
{"cinder/tests/unit/test_volume.py":[{"author":{"_account_id":13636,"name":"aarefiev","display_name":"Anton","email":"arefyevanton@gmail.com","username":"aarefiev"},"change_message_id":"6cb3d7004509dcc02774933ca582de5a8c9a8ed4","unresolved":false,"context_lines":[{"line_number":139,"context_line":"            shutil.rmtree(CONF.volumes_dir)"},{"line_number":140,"context_line":"        except OSError:"},{"line_number":141,"context_line":"            pass"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def fake_get_target(obj, iqn):"},{"line_number":144,"context_line":"        return 1"},{"line_number":145,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_88322d0f","line":142,"updated":"2015-07-14 12:47:47.000000000","message":"We have patch which remove this https://review.openstack.org/#/c/200378/4 . Maybe we should do it one patch","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"48a3e4cf57b372a0e39d0684e4e1880df98426f5","unresolved":false,"context_lines":[{"line_number":234,"context_line":"                               last_fetched)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"@mock.patch(\u0027cinder.volume.utils.clear_volume\u0027, new\u003dmock.MagicMock())"},{"line_number":238,"context_line":"class VolumeTestCase(BaseVolumeTestCase):"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_e5d764eb","line":237,"updated":"2015-07-14 09:00:37.000000000","message":"If you don\u0027t define new the default is already a MagicMock.\n\nI think it is more interesting to pass autospec\u003dTrue to the patch decorator, that way arguments passed to clear_volume will be checked.\n\nAnd if you are doing it to avoid the extra argument in the methods I think it would be better to do the mocking in the setUp.","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"5ecb99cceaf89f2fc1a05cbb9b8fd226c6daf38d","unresolved":false,"context_lines":[{"line_number":234,"context_line":"                               last_fetched)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"@mock.patch(\u0027cinder.volume.utils.clear_volume\u0027, new\u003dmock.MagicMock())"},{"line_number":238,"context_line":"class VolumeTestCase(BaseVolumeTestCase):"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_f3cabd6d","line":237,"in_reply_to":"9a41bdd9_e5d764eb","updated":"2015-07-14 13:40:30.000000000","message":"I\u0027ll look at this.","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"30ab5ad02de11b65ce614d142e71002ecc7cfe78","unresolved":false,"context_lines":[{"line_number":234,"context_line":"                               last_fetched)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"@mock.patch(\u0027cinder.volume.utils.clear_volume\u0027, new\u003dmock.MagicMock())"},{"line_number":238,"context_line":"class VolumeTestCase(BaseVolumeTestCase):"},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_325a5e2b","line":237,"in_reply_to":"9a41bdd9_f3cabd6d","updated":"2015-07-14 14:19:36.000000000","message":"Done","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"48a3e4cf57b372a0e39d0684e4e1880df98426f5","unresolved":false,"context_lines":[{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    @mock.patch(\u0027cinder.quota.QUOTAS.rollback\u0027, new\u003dmock.Mock())"},{"line_number":503,"context_line":"    @mock.patch(\u0027cinder.quota.QUOTAS.commit\u0027, new\u003dmock.Mock())"},{"line_number":504,"context_line":"    @mock.patch(\u0027cinder.quota.QUOTAS.reserve\u0027)"},{"line_number":505,"context_line":"    def test_create_delete_volume(self, mock_reserve):"},{"line_number":506,"context_line":"        \"\"\"Test volume can be created and deleted.\"\"\""},{"line_number":507,"context_line":"        mock_reserve.return_value \u003d [\u0027RESERVATION\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_c5e6e0b3","line":504,"updated":"2015-07-14 09:00:37.000000000","message":"autospec?\n\nYou should be consistent with the changes, in line 724 you include the return_value in the decorator.","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"30ab5ad02de11b65ce614d142e71002ecc7cfe78","unresolved":false,"context_lines":[{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    @mock.patch(\u0027cinder.quota.QUOTAS.rollback\u0027, new\u003dmock.Mock())"},{"line_number":503,"context_line":"    @mock.patch(\u0027cinder.quota.QUOTAS.commit\u0027, new\u003dmock.Mock())"},{"line_number":504,"context_line":"    @mock.patch(\u0027cinder.quota.QUOTAS.reserve\u0027)"},{"line_number":505,"context_line":"    def test_create_delete_volume(self, mock_reserve):"},{"line_number":506,"context_line":"        \"\"\"Test volume can be created and deleted.\"\"\""},{"line_number":507,"context_line":"        mock_reserve.return_value \u003d [\u0027RESERVATION\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_d2c69299","line":504,"in_reply_to":"9a41bdd9_c5e6e0b3","updated":"2015-07-14 14:19:36.000000000","message":"Done","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":13636,"name":"aarefiev","display_name":"Anton","email":"arefyevanton@gmail.com","username":"aarefiev"},"change_message_id":"6cb3d7004509dcc02774933ca582de5a8c9a8ed4","unresolved":false,"context_lines":[{"line_number":909,"context_line":"        volume \u003d tests_utils.create_volume(self.context, **self.volume_params)"},{"line_number":910,"context_line":"        volume_id \u003d volume[\u0027id\u0027]"},{"line_number":911,"context_line":"        self.volume.create_volume(self.context, volume_id)"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"        self.mox.StubOutWithMock(self.volume.driver, \u0027delete_volume\u0027)"},{"line_number":914,"context_line":"        self.volume.driver.delete_volume("},{"line_number":915,"context_line":"            mox.IgnoreArg()).AndRaise(exception.VolumeIsBusy("}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_82c0c1e1","line":912,"updated":"2015-07-14 12:47:47.000000000","message":"I think StubOutWithMock should be replaced too.","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"5ecb99cceaf89f2fc1a05cbb9b8fd226c6daf38d","unresolved":false,"context_lines":[{"line_number":909,"context_line":"        volume \u003d tests_utils.create_volume(self.context, **self.volume_params)"},{"line_number":910,"context_line":"        volume_id \u003d volume[\u0027id\u0027]"},{"line_number":911,"context_line":"        self.volume.create_volume(self.context, volume_id)"},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"        self.mox.StubOutWithMock(self.volume.driver, \u0027delete_volume\u0027)"},{"line_number":914,"context_line":"        self.volume.driver.delete_volume("},{"line_number":915,"context_line":"            mox.IgnoreArg()).AndRaise(exception.VolumeIsBusy("}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_b31675ef","line":912,"in_reply_to":"9a41bdd9_82c0c1e1","updated":"2015-07-14 13:40:30.000000000","message":"There are a lot of things that should also be fixed up here.  As I noted in the commit message, I don\u0027t want to try to address them all in one patch.  It will never be finished enough to merge if we use that strategy.  We need to make reasonable improvements in chunks that are small enough to code, review, and merge without too many conflicts.","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"48a3e4cf57b372a0e39d0684e4e1880df98426f5","unresolved":false,"context_lines":[{"line_number":1320,"context_line":"            self, mock_lvm_create):"},{"line_number":1321,"context_line":"        # mock the synchroniser so we can record events"},{"line_number":1322,"context_line":"        orig_flow \u003d engine.ActionEngine.run"},{"line_number":1323,"context_line":"        self.stubs.Set(utils, \u0027synchronized\u0027, self._mock_synchronized)"},{"line_number":1324,"context_line":""},{"line_number":1325,"context_line":"        def mock_flow_run(*args, **kwargs):"},{"line_number":1326,"context_line":"            # ensure the lock has been taken"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_68f7a9f5","line":1323,"updated":"2015-07-14 09:00:37.000000000","message":"Why are you using stubs?\n\nWe should use mock as well.\n\n   @mock.patch.object(utils, \u0027synchronized\u0027)                            \n   @mock.patch.object(cinder.volume.drivers.lvm.LVMISCSIDriver,         \n                      \u0027create_volume_from_snapshot\u0027)                    \n   def test_create_volume_from_snapshot_check_locks(                    \n           self, mock_lvm_create, mock_synchro):                        \n       # mock the synchroniser so we can record events                  \n       mock_synchro.side_effect \u003d self._mock_synchronized","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"5ecb99cceaf89f2fc1a05cbb9b8fd226c6daf38d","unresolved":false,"context_lines":[{"line_number":1320,"context_line":"            self, mock_lvm_create):"},{"line_number":1321,"context_line":"        # mock the synchroniser so we can record events"},{"line_number":1322,"context_line":"        orig_flow \u003d engine.ActionEngine.run"},{"line_number":1323,"context_line":"        self.stubs.Set(utils, \u0027synchronized\u0027, self._mock_synchronized)"},{"line_number":1324,"context_line":""},{"line_number":1325,"context_line":"        def mock_flow_run(*args, **kwargs):"},{"line_number":1326,"context_line":"            # ensure the lock has been taken"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_b319957b","line":1323,"in_reply_to":"9a41bdd9_68f7a9f5","updated":"2015-07-14 13:40:30.000000000","message":"Because it was already there, and I\u0027m not trying to fix everything in this patch in one go.  (I didn\u0027t add this, apparently just reordered it somehow.)","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a291bc130d796e3454083e224322d4ff35509be5","unresolved":false,"context_lines":[{"line_number":1320,"context_line":"            self, mock_lvm_create):"},{"line_number":1321,"context_line":"        # mock the synchroniser so we can record events"},{"line_number":1322,"context_line":"        orig_flow \u003d engine.ActionEngine.run"},{"line_number":1323,"context_line":"        self.stubs.Set(utils, \u0027synchronized\u0027, self._mock_synchronized)"},{"line_number":1324,"context_line":""},{"line_number":1325,"context_line":"        def mock_flow_run(*args, **kwargs):"},{"line_number":1326,"context_line":"            # ensure the lock has been taken"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_9dda4e4b","line":1323,"in_reply_to":"9a41bdd9_b319957b","updated":"2015-07-14 18:01:27.000000000","message":"Oh, so it was reordered, then it\u0027s ok.","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"48a3e4cf57b372a0e39d0684e4e1880df98426f5","unresolved":false,"context_lines":[{"line_number":1381,"context_line":"                          \u0027unlock-%s\u0027 % (\u0027%s-delete_volume\u0027 % (src_vol_id))],"},{"line_number":1382,"context_line":"                         self.called)"},{"line_number":1383,"context_line":""},{"line_number":1384,"context_line":"        self.assertEqual(True, mock_lvm_create.called)"},{"line_number":1385,"context_line":""},{"line_number":1386,"context_line":"    def test_create_volume_from_volume_check_locks(self):"},{"line_number":1387,"context_line":"        # mock the synchroniser so we can record events"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_e819797a","line":1384,"updated":"2015-07-14 09:00:37.000000000","message":"This should check arguments passed to create are the expected ones, so we should use mock_lvm.create.assert_called_once_with.\n\nIf you don\u0027t want to check the arguments, which I think it\u0027s not such a great idea, you should at least consider using:\n\n set.assertTrue(mock_lvm_create.called)","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"5ecb99cceaf89f2fc1a05cbb9b8fd226c6daf38d","unresolved":false,"context_lines":[{"line_number":1381,"context_line":"                          \u0027unlock-%s\u0027 % (\u0027%s-delete_volume\u0027 % (src_vol_id))],"},{"line_number":1382,"context_line":"                         self.called)"},{"line_number":1383,"context_line":""},{"line_number":1384,"context_line":"        self.assertEqual(True, mock_lvm_create.called)"},{"line_number":1385,"context_line":""},{"line_number":1386,"context_line":"    def test_create_volume_from_volume_check_locks(self):"},{"line_number":1387,"context_line":"        # mock the synchroniser so we can record events"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_53836956","line":1384,"in_reply_to":"9a41bdd9_e819797a","updated":"2015-07-14 13:40:30.000000000","message":"I don\u0027t really think it should get into checking all of the arguments, since the test case is for testing lock behavior.  There are other tests that cover the details of create_volume_from_snapshot.","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"48a3e4cf57b372a0e39d0684e4e1880df98426f5","unresolved":false,"context_lines":[{"line_number":5043,"context_line":""},{"line_number":5044,"context_line":"        self.assertTrue(mock_create_cgsnap.called)"},{"line_number":5045,"context_line":"        self.assertTrue(mock_del_cgsnap.called)"},{"line_number":5046,"context_line":"        self.assertTrue(mock_del_cg.called)"},{"line_number":5047,"context_line":""},{"line_number":5048,"context_line":"    @mock.patch(\u0027cinder.volume.driver.VolumeDriver.create_consistencygroup\u0027,"},{"line_number":5049,"context_line":"                return_value\u003d{\u0027status\u0027: \u0027available\u0027})"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a41bdd9_8861cd3b","line":5046,"updated":"2015-07-14 09:00:37.000000000","message":"I\u0027m not a big fan of just checking called without checking the arguments.  We could set the mock to autospec so it checks the right number of arguments are passed.","commit_id":"0c46555750a4f16ae80da17c4e2f3bcc1a0d0c0d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0e2eabba871075b9f11676d4bf4b520339753b1b","unresolved":false,"context_lines":[{"line_number":244,"context_line":""},{"line_number":245,"context_line":"    def tearDown(self):"},{"line_number":246,"context_line":"        super(VolumeTestCase, self).tearDown()"},{"line_number":247,"context_line":"        self._clear_patch.stop()"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    def test_init_host_clears_downloads(self):"},{"line_number":250,"context_line":"        \"\"\"Test that init_host will unwedge a volume stuck in downloading.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"9a41bdd9_1d72be7b","line":247,"updated":"2015-07-14 18:01:47.000000000","message":"I was thinking you would do this in the setUp with:\n\n self.addCleanup(self._clear_patch.stop)\n\nBut this is nice.","commit_id":"f9c179c38c83f9d6f4dae47d42328861cef6855e"}]}
