)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c5a044c614d1b4a6092a773b7c4a98a4612fc408","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"XenAPI: add unit test for the plugin - glance"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Unit tet for the xapi plugin - glance."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Change-Id: Ib1a8bd455b972798f2988834de61cfce048c15f9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba0121b8_aba82f78","line":9,"range":{"start_line":9,"start_character":5,"end_line":9,"end_character":8},"updated":"2016-03-31 14:19:31.000000000","message":"nit: s/tet/test/\n\nAlso I think this second sentence does not add anything extra too the first sentence of the commit message.","commit_id":"dd784940c413539a96e889df6452e06980941a58"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"d26bb958b3ea483b3a151ab91361cd663ad6ae5b","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"XenAPI: add unit test for the plugin - glance"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Unit tet for the xapi plugin - glance."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Change-Id: Ib1a8bd455b972798f2988834de61cfce048c15f9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ba0121b8_a943f86a","line":9,"range":{"start_line":9,"start_character":5,"end_line":9,"end_character":8},"in_reply_to":"ba0121b8_aba82f78","updated":"2016-04-01 02:08:42.000000000","message":"Done","commit_id":"dd784940c413539a96e889df6452e06980941a58"}],"nova/tests/unit/virt/xenapi/plugins/test_glance.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c5a044c614d1b4a6092a773b7c4a98a4612fc408","unresolved":false,"context_lines":[{"line_number":92,"context_line":"                self.glance._download_tarball_and_verify,"},{"line_number":93,"context_line":"                fake_request, \u0027fake_staging_path\u0027)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        _test_download_and_verify_ok()"},{"line_number":96,"context_line":"        _test_download_ok_verify_failed()"},{"line_number":97,"context_line":"        _test_download_failed_HTTPError()"},{"line_number":98,"context_line":"        _test_download_failed_URLError()"},{"line_number":99,"context_line":"        _test_download_failed_HTTPException()"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    @mock.patch.object(urllib2, \u0027Request\u0027)"},{"line_number":102,"context_line":"    def test_download_tarball_by_url(self, mock_request):"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba0121b8_cba80323","line":99,"range":{"start_line":95,"start_character":0,"end_line":99,"end_character":45},"updated":"2016-03-31 14:19:31.000000000","message":"This seems to me as 5 separate test cases with a common setup function.","commit_id":"dd784940c413539a96e889df6452e06980941a58"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"54839a70422ff9dffd58b2c66df269513e4ddc1f","unresolved":false,"context_lines":[{"line_number":92,"context_line":"                self.glance._download_tarball_and_verify,"},{"line_number":93,"context_line":"                fake_request, \u0027fake_staging_path\u0027)"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        _test_download_and_verify_ok()"},{"line_number":96,"context_line":"        _test_download_ok_verify_failed()"},{"line_number":97,"context_line":"        _test_download_failed_HTTPError()"},{"line_number":98,"context_line":"        _test_download_failed_URLError()"},{"line_number":99,"context_line":"        _test_download_failed_HTTPException()"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    @mock.patch.object(urllib2, \u0027Request\u0027)"},{"line_number":102,"context_line":"    def test_download_tarball_by_url(self, mock_request):"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba0121b8_091b04cb","line":99,"range":{"start_line":95,"start_character":0,"end_line":99,"end_character":45},"in_reply_to":"ba0121b8_cba80323","updated":"2016-04-01 02:06:56.000000000","message":"sure. To make each test case to be simple as possible, I will change them to be 5 separate test case.","commit_id":"dd784940c413539a96e889df6452e06980941a58"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c5a044c614d1b4a6092a773b7c4a98a4612fc408","unresolved":false,"context_lines":[{"line_number":174,"context_line":"            self.assertTrue("},{"line_number":175,"context_line":"                mock_HTTPSConn.return_value.getresponse.called)"},{"line_number":176,"context_line":"            self.assertTrue(mock_check_resp_status.called)"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        _test_upload_tarball_by_url_http()"},{"line_number":179,"context_line":"        _test_upload_tarball_by_url_https()"},{"line_number":180,"context_line":"        _test_upload_tarball_by_url_https_failed_retry()"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def test_check_resp_status_and_retry(self):"},{"line_number":183,"context_line":"        mock_resp_badrequest \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba0121b8_3db2a206","line":180,"range":{"start_line":177,"start_character":0,"end_line":180,"end_character":56},"updated":"2016-03-31 14:19:31.000000000","message":"ditto","commit_id":"dd784940c413539a96e889df6452e06980941a58"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"54839a70422ff9dffd58b2c66df269513e4ddc1f","unresolved":false,"context_lines":[{"line_number":174,"context_line":"            self.assertTrue("},{"line_number":175,"context_line":"                mock_HTTPSConn.return_value.getresponse.called)"},{"line_number":176,"context_line":"            self.assertTrue(mock_check_resp_status.called)"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        _test_upload_tarball_by_url_http()"},{"line_number":179,"context_line":"        _test_upload_tarball_by_url_https()"},{"line_number":180,"context_line":"        _test_upload_tarball_by_url_https_failed_retry()"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"    def test_check_resp_status_and_retry(self):"},{"line_number":183,"context_line":"        mock_resp_badrequest \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba0121b8_c952ac66","line":180,"range":{"start_line":177,"start_character":0,"end_line":180,"end_character":56},"in_reply_to":"ba0121b8_3db2a206","updated":"2016-04-01 02:06:56.000000000","message":"I will change them to be separate test cases.","commit_id":"dd784940c413539a96e889df6452e06980941a58"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c5a044c614d1b4a6092a773b7c4a98a4612fc408","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                mock_conn.getresponse.return_value.read.call_count, 2)"},{"line_number":220,"context_line":"            self.assertFalse(mock_check_resp_status_and_retry.called)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        def _test_image_status_ko():"},{"line_number":223,"context_line":"            mock_head_resp.getheader.return_value \u003d \u0027not-queued\u0027"},{"line_number":224,"context_line":"            mock_conn.getresponse.return_value \u003d mock_head_resp"},{"line_number":225,"context_line":"            self.assertRaises(self.glance.PluginError,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba0121b8_9dbf6ea9","line":222,"range":{"start_line":222,"start_character":12,"end_line":222,"end_character":33},"updated":"2016-03-31 14:19:31.000000000","message":"does \u0027ko\u0027 means \u0027not ok\u0027 here?","commit_id":"dd784940c413539a96e889df6452e06980941a58"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"54839a70422ff9dffd58b2c66df269513e4ddc1f","unresolved":false,"context_lines":[{"line_number":219,"context_line":"                mock_conn.getresponse.return_value.read.call_count, 2)"},{"line_number":220,"context_line":"            self.assertFalse(mock_check_resp_status_and_retry.called)"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        def _test_image_status_ko():"},{"line_number":223,"context_line":"            mock_head_resp.getheader.return_value \u003d \u0027not-queued\u0027"},{"line_number":224,"context_line":"            mock_conn.getresponse.return_value \u003d mock_head_resp"},{"line_number":225,"context_line":"            self.assertRaises(self.glance.PluginError,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba0121b8_c9d90cf5","line":222,"range":{"start_line":222,"start_character":12,"end_line":222,"end_character":33},"in_reply_to":"ba0121b8_9dbf6ea9","updated":"2016-04-01 02:06:56.000000000","message":"yes. I will change it to be failure for better understanding.","commit_id":"dd784940c413539a96e889df6452e06980941a58"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    def setUp(self):"},{"line_number":38,"context_line":"        super(GlanceTestCase, self).setUp()"},{"line_number":39,"context_line":"        # md5 is deprecated in py2.7 and forward;"},{"line_number":40,"context_line":"        sys.modules[\u0027md5\u0027] \u003d mock.Mock()"},{"line_number":41,"context_line":"        self.glance \u003d self.load_plugin(\"glance\")"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    @mock.patch.object(urllib2, \u0027urlopen\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_b7dfdb38","line":40,"updated":"2016-04-20 14:38:18.000000000","message":"Do we have to do this here?  Would a self.mock_patch_object work as well, or not?","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"fd548d4ab904d6a4c34bc7baf35fc0738002d062","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    def setUp(self):"},{"line_number":38,"context_line":"        super(GlanceTestCase, self).setUp()"},{"line_number":39,"context_line":"        # md5 is deprecated in py2.7 and forward;"},{"line_number":40,"context_line":"        sys.modules[\u0027md5\u0027] \u003d mock.Mock()"},{"line_number":41,"context_line":"        self.glance \u003d self.load_plugin(\"glance\")"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    @mock.patch.object(urllib2, \u0027urlopen\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_9d5c794c","line":40,"in_reply_to":"1a122d0e_b7dfdb38","updated":"2016-04-20 16:09:23.000000000","message":"I tried that but can\u0027t work well. As it will have problem when load md5 which goes before the mock_patch-object.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    @mock.patch.object(urllib2, \u0027urlopen\u0027)"},{"line_number":44,"context_line":"    def test_download_and_verify_ok(self, mock_urlopen):"},{"line_number":45,"context_line":"        mock_extract_tarball \u003d self.mock_patch_object("},{"line_number":46,"context_line":"            self.glance.utils, \u0027extract_tarball\u0027)"},{"line_number":47,"context_line":"        mock_info \u003d mock.MagicMock()"},{"line_number":48,"context_line":"        mock_info.getheader.return_value \u003d \u0027expect_cksum\u0027"},{"line_number":49,"context_line":"        mock_urlopen.return_value.info.return_value \u003d mock_info"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_778ee32e","line":46,"updated":"2016-04-20 14:38:18.000000000","message":"Perhaps we should actually have a mock extract tarball function which just invokes the callback with appropriate data?  Then we could have a more comprehensive test here.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"fd548d4ab904d6a4c34bc7baf35fc0738002d062","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    @mock.patch.object(urllib2, \u0027urlopen\u0027)"},{"line_number":44,"context_line":"    def test_download_and_verify_ok(self, mock_urlopen):"},{"line_number":45,"context_line":"        mock_extract_tarball \u003d self.mock_patch_object("},{"line_number":46,"context_line":"            self.glance.utils, \u0027extract_tarball\u0027)"},{"line_number":47,"context_line":"        mock_info \u003d mock.MagicMock()"},{"line_number":48,"context_line":"        mock_info.getheader.return_value \u003d \u0027expect_cksum\u0027"},{"line_number":49,"context_line":"        mock_urlopen.return_value.info.return_value \u003d mock_info"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_600bb624","line":46,"in_reply_to":"1a122d0e_778ee32e","updated":"2016-04-20 16:09:23.000000000","message":"Ok. Will investigate how to achieve that.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        mock_info.getheader.return_value \u003d \u0027expect_cksum\u0027"},{"line_number":49,"context_line":"        mock_urlopen.return_value.info.return_value \u003d mock_info"},{"line_number":50,"context_line":"        fake_request \u003d urllib2.Request(\u0027http://fakeurl.com\u0027)"},{"line_number":51,"context_line":"        self.glance.md5.new.return_value.hexdigest.return_value \u003d \\"},{"line_number":52,"context_line":"            \u0027expect_cksum\u0027"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        result \u003d self.glance._download_tarball_and_verify("}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_179a0fb9","line":51,"updated":"2016-04-20 14:38:18.000000000","message":"this is a bit of a mouthful...\nI\u0027d suggest having a mock_md5\u003dmock.MagicMock()","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"fd548d4ab904d6a4c34bc7baf35fc0738002d062","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        mock_info.getheader.return_value \u003d \u0027expect_cksum\u0027"},{"line_number":49,"context_line":"        mock_urlopen.return_value.info.return_value \u003d mock_info"},{"line_number":50,"context_line":"        fake_request \u003d urllib2.Request(\u0027http://fakeurl.com\u0027)"},{"line_number":51,"context_line":"        self.glance.md5.new.return_value.hexdigest.return_value \u003d \\"},{"line_number":52,"context_line":"            \u0027expect_cksum\u0027"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        result \u003d self.glance._download_tarball_and_verify("}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_8025325d","line":51,"in_reply_to":"1a122d0e_179a0fb9","updated":"2016-04-20 16:09:23.000000000","message":"it has some invoking statements as:\na \u003d md5()\ncksum \u003d a.hexdigest()\nif cksum \u003d\u003d expect_cksum:\n...\nI\u0027m not sure if there is a simple way to mock that well.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        self.glance.md5.new.return_value.hexdigest.return_value \u003d \\"},{"line_number":52,"context_line":"            \u0027expect_cksum\u0027"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        result \u003d self.glance._download_tarball_and_verify("},{"line_number":55,"context_line":"            fake_request, \u0027fake_staging_path\u0027)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        self.assertTrue(mock_urlopen.called)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_d4d095cb","line":54,"updated":"2016-04-20 14:38:18.000000000","message":"I think the point is that we don\u0027t need a return value?\nverify will always raise an exception if we\u0027re not happy with the tarball.  No new return value is needed?","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"fd548d4ab904d6a4c34bc7baf35fc0738002d062","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        self.glance.md5.new.return_value.hexdigest.return_value \u003d \\"},{"line_number":52,"context_line":"            \u0027expect_cksum\u0027"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"        result \u003d self.glance._download_tarball_and_verify("},{"line_number":55,"context_line":"            fake_request, \u0027fake_staging_path\u0027)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        self.assertTrue(mock_urlopen.called)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_608576cd","line":54,"in_reply_to":"1a122d0e_d4d095cb","updated":"2016-04-20 16:09:23.000000000","message":"It\u0027s not true that it raise exception always. e.g.\n    if etag is None:\n        msg \u003d \"No ETag found for comparison to checksum %(checksum)s\"\n        logging.info(msg % {\u0027checksum\u0027: checksum})\n...\n    else:\n        msg \u003d \"Verified image checksum %(checksum)s\"\n        logging.info(msg % {\u0027checksum\u0027: checksum})","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        result \u003d self.glance._download_tarball_and_verify("},{"line_number":55,"context_line":"            fake_request, \u0027fake_staging_path\u0027)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        self.assertTrue(mock_urlopen.called)"},{"line_number":58,"context_line":"        self.assertTrue(mock_extract_tarball.called)"},{"line_number":59,"context_line":"        self.assertTrue(result)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_b70b5b23","line":57,"updated":"2016-04-20 14:38:18.000000000","message":"No point verifying that this function was called if we don\u0027t check the arguments or similar.\n\nmock_extract_tarball should be verified that it gets called with the right arguments - particularly \"response\"","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"fd548d4ab904d6a4c34bc7baf35fc0738002d062","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        result \u003d self.glance._download_tarball_and_verify("},{"line_number":55,"context_line":"            fake_request, \u0027fake_staging_path\u0027)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        self.assertTrue(mock_urlopen.called)"},{"line_number":58,"context_line":"        self.assertTrue(mock_extract_tarball.called)"},{"line_number":59,"context_line":"        self.assertTrue(result)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_60b8d675","line":57,"in_reply_to":"1a122d0e_b70b5b23","updated":"2016-04-20 16:09:23.000000000","message":"ok.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"        self.assertTrue(mock_urlopen.called)"},{"line_number":58,"context_line":"        self.assertTrue(mock_extract_tarball.called)"},{"line_number":59,"context_line":"        self.assertTrue(result)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    @mock.patch.object(urllib2, \u0027urlopen\u0027)"},{"line_number":62,"context_line":"    def test_download_ok_verify_failed(self, mock_urlopen):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_5776473e","line":59,"updated":"2016-04-20 14:38:18.000000000","message":"Not needed - if the result was False, an exception would have been raised.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        self.glance.md5.new.return_value.hexdigest.return_value \u003d \\"},{"line_number":70,"context_line":"            \u0027unexpect_cksum\u0027"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        is_RetryableError_raised \u003d False"},{"line_number":73,"context_line":"        try:"},{"line_number":74,"context_line":"            self.glance._download_tarball_and_verify("},{"line_number":75,"context_line":"                fake_request, \u0027fake_staging_path\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_57c4873e","line":72,"updated":"2016-04-20 14:38:18.000000000","message":"Can\u0027t we use the self.assertRaises construct?","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        except self.glance.RetryableError:"},{"line_number":77,"context_line":"            is_RetryableError_raised \u003d True"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        self.assertTrue(mock_urlopen.called)"},{"line_number":80,"context_line":"        self.assertTrue(mock_extract_tarball.called)"},{"line_number":81,"context_line":"        self.assertTrue(is_RetryableError_raised)"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_17432fb3","line":79,"updated":"2016-04-20 14:38:18.000000000","message":"These probably aren\u0027t needed at all if you swtich to assertRaises since the main path is tested above.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"fd548d4ab904d6a4c34bc7baf35fc0738002d062","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        except self.glance.RetryableError:"},{"line_number":77,"context_line":"            is_RetryableError_raised \u003d True"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        self.assertTrue(mock_urlopen.called)"},{"line_number":80,"context_line":"        self.assertTrue(mock_extract_tarball.called)"},{"line_number":81,"context_line":"        self.assertTrue(is_RetryableError_raised)"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_c399f4c5","line":79,"in_reply_to":"1a122d0e_17432fb3","updated":"2016-04-20 16:09:23.000000000","message":"it raises the same exception for many different failures. With these two additional assetTrue, it can verify the exception is raised exactly by the expected call flow.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            fake_request, \u0027fake_staging_path\u0027)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    @mock.patch.object(urllib2, \u0027Request\u0027)"},{"line_number":115,"context_line":"    def test_download_tarball_by_url(self, mock_request):"},{"line_number":116,"context_line":"        mock_download_tarball_and_verify \u003d self.mock_patch_object("},{"line_number":117,"context_line":"            self.glance, \u0027_download_tarball_and_verify\u0027)"},{"line_number":118,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_aa5ad2ef","line":115,"updated":"2016-04-20 14:38:18.000000000","message":"Update comment in plugin to remove the suggestion that it retries?  The retry actually happens in Nova, not in the plugin.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"fd548d4ab904d6a4c34bc7baf35fc0738002d062","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            fake_request, \u0027fake_staging_path\u0027)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    @mock.patch.object(urllib2, \u0027Request\u0027)"},{"line_number":115,"context_line":"    def test_download_tarball_by_url(self, mock_request):"},{"line_number":116,"context_line":"        mock_download_tarball_and_verify \u003d self.mock_patch_object("},{"line_number":117,"context_line":"            self.glance, \u0027_download_tarball_and_verify\u0027)"},{"line_number":118,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_6328e033","line":115,"in_reply_to":"1a122d0e_aa5ad2ef","updated":"2016-04-20 16:09:23.000000000","message":"ok. will do that.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        self.assertTrue(mock_download_tarball_and_verify.called)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    @mock.patch.object(httplib, \u0027HTTPConnection\u0027)"},{"line_number":127,"context_line":"    def test_upload_tarball_by_url_http(self, mock_HTTPConn):"},{"line_number":128,"context_line":"        mock_validate_image \u003d self.mock_patch_object("},{"line_number":129,"context_line":"            self.glance, \u0027validate_image_status_before_upload\u0027)"},{"line_number":130,"context_line":"        mock_create_tarball \u003d self.mock_patch_object("}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_6a0a7a8b","line":127,"updated":"2016-04-20 14:38:18.000000000","message":"Similar comments to download above; this is a very big method where lots of paths aren\u0027t covered.  For example, because create_tarball is mocked out, the callback is never invoked.\n\nAlso better to check the arguments for some of the methods called below, not just the fact that they are called.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"fd548d4ab904d6a4c34bc7baf35fc0738002d062","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        self.assertTrue(mock_download_tarball_and_verify.called)"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    @mock.patch.object(httplib, \u0027HTTPConnection\u0027)"},{"line_number":127,"context_line":"    def test_upload_tarball_by_url_http(self, mock_HTTPConn):"},{"line_number":128,"context_line":"        mock_validate_image \u003d self.mock_patch_object("},{"line_number":129,"context_line":"            self.glance, \u0027validate_image_status_before_upload\u0027)"},{"line_number":130,"context_line":"        mock_create_tarball \u003d self.mock_patch_object("}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_c3ea94da","line":127,"in_reply_to":"1a122d0e_6a0a7a8b","updated":"2016-04-20 16:09:23.000000000","message":"ok. Will refine the tests.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":148,"context_line":"        self.assertTrue(mock_create_tarball.called)"},{"line_number":149,"context_line":"        self.assertTrue("},{"line_number":150,"context_line":"            mock_HTTPConn.return_value.getresponse.called)"},{"line_number":151,"context_line":"        self.assertFalse(mock_check_resp_status.called)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @mock.patch.object(httplib, \u0027HTTPSConnection\u0027)"},{"line_number":154,"context_line":"    def test_upload_tarball_by_url_https(self, mock_HTTPSConn):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_2de09c95","line":151,"updated":"2016-04-20 14:38:18.000000000","message":"Should definitely check mock_HTTPConn.close.assert_called_with()","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        self.assertFalse(mock_check_resp_status.called)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @mock.patch.object(httplib, \u0027HTTPSConnection\u0027)"},{"line_number":154,"context_line":"    def test_upload_tarball_by_url_https(self, mock_HTTPSConn):"},{"line_number":155,"context_line":"        mock_validate_image \u003d self.mock_patch_object("},{"line_number":156,"context_line":"            self.glance, \u0027validate_image_status_before_upload\u0027)"},{"line_number":157,"context_line":"        mock_create_tarball \u003d self.mock_patch_object("}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_cab58e7e","line":154,"updated":"2016-04-20 14:38:18.000000000","message":"This can be a simple extension of the previous test; i.e. it\u0027s fine for it to not test everything again.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"fd548d4ab904d6a4c34bc7baf35fc0738002d062","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        self.assertFalse(mock_check_resp_status.called)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @mock.patch.object(httplib, \u0027HTTPSConnection\u0027)"},{"line_number":154,"context_line":"    def test_upload_tarball_by_url_https(self, mock_HTTPSConn):"},{"line_number":155,"context_line":"        mock_validate_image \u003d self.mock_patch_object("},{"line_number":156,"context_line":"            self.glance, \u0027validate_image_status_before_upload\u0027)"},{"line_number":157,"context_line":"        mock_create_tarball \u003d self.mock_patch_object("}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_03c48c3d","line":154,"in_reply_to":"1a122d0e_cab58e7e","updated":"2016-04-20 16:09:23.000000000","message":"it only tries to cover both http and https. Do you mean this https test is not needed and I can remove this test?","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        self.assertFalse(mock_check_resp_status.called)"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    @mock.patch.object(httplib, \u0027HTTPSConnection\u0027)"},{"line_number":181,"context_line":"    def test_upload_tarball_by_url_https_failed_retry(self, mock_HTTPSConn):"},{"line_number":182,"context_line":"        mock_validate_image \u003d self.mock_patch_object("},{"line_number":183,"context_line":"            self.glance, \u0027validate_image_status_before_upload\u0027)"},{"line_number":184,"context_line":"        mock_create_tarball \u003d self.mock_patch_object("}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_2d5d3c15","line":181,"updated":"2016-04-20 14:38:18.000000000","message":"no \u0027https\u0027 needed.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        self.assertTrue(mock_create_tarball.called)"},{"line_number":203,"context_line":"        self.assertTrue("},{"line_number":204,"context_line":"            mock_HTTPSConn.return_value.getresponse.called)"},{"line_number":205,"context_line":"        self.assertTrue(mock_check_resp_status.called)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def test_check_resp_status_and_retry(self):"},{"line_number":208,"context_line":"        mock_resp_badrequest \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_6d31f459","line":205,"updated":"2016-04-20 14:38:18.000000000","message":"Because you\u0027re mocking this, this test is actually not testing anything.\nI suggest that the check_resp_status_and_retry test below is sufficient.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"fd548d4ab904d6a4c34bc7baf35fc0738002d062","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        self.assertTrue(mock_create_tarball.called)"},{"line_number":203,"context_line":"        self.assertTrue("},{"line_number":204,"context_line":"            mock_HTTPSConn.return_value.getresponse.called)"},{"line_number":205,"context_line":"        self.assertTrue(mock_check_resp_status.called)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def test_check_resp_status_and_retry(self):"},{"line_number":208,"context_line":"        mock_resp_badrequest \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_c329f4e7","line":205,"in_reply_to":"1a122d0e_6d31f459","updated":"2016-04-20 16:09:23.000000000","message":"ok.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        self.assertTrue(mock_conn.getresponse.called)"},{"line_number":243,"context_line":"        self.assertEqual("},{"line_number":244,"context_line":"            mock_conn.getresponse.return_value.read.call_count, 2)"},{"line_number":245,"context_line":"        self.assertFalse(mock_check_resp_status_and_retry.called)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"    def test_validate_image_status_before_upload_failed(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_683442dd","line":244,"updated":"2016-04-20 14:38:18.000000000","message":"Surely just mock_head_resp.read.call_count?","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":273,"context_line":"            \u0027fake_uuid_stack\u0027, \u0027fake_sr_path\u0027, \u0027fake_extra_headers\u0027)"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        self.assertTrue(mock_make_staging_area.called)"},{"line_number":276,"context_line":"        self.assertTrue(mock_download_tarball_by_url.called)"},{"line_number":277,"context_line":"        self.assertTrue(mock_import_vhds.called)"},{"line_number":278,"context_line":"        self.assertTrue(mock_cleanup_staging_area.called)"},{"line_number":279,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_28e1aa2d","line":276,"updated":"2016-04-20 14:38:18.000000000","message":"mock_download_tarball_by_url.assert_called_with(\u0027fake_sr_path\u0027, \u0027mock_staging_area\u0027, ...)","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"fd548d4ab904d6a4c34bc7baf35fc0738002d062","unresolved":false,"context_lines":[{"line_number":273,"context_line":"            \u0027fake_uuid_stack\u0027, \u0027fake_sr_path\u0027, \u0027fake_extra_headers\u0027)"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        self.assertTrue(mock_make_staging_area.called)"},{"line_number":276,"context_line":"        self.assertTrue(mock_download_tarball_by_url.called)"},{"line_number":277,"context_line":"        self.assertTrue(mock_import_vhds.called)"},{"line_number":278,"context_line":"        self.assertTrue(mock_cleanup_staging_area.called)"},{"line_number":279,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_03e5aca6","line":276,"in_reply_to":"1a122d0e_28e1aa2d","updated":"2016-04-20 16:09:23.000000000","message":"ok. Will add the verification on arguments.","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        self.assertTrue(mock_make_staging_area.called)"},{"line_number":276,"context_line":"        self.assertTrue(mock_download_tarball_by_url.called)"},{"line_number":277,"context_line":"        self.assertTrue(mock_import_vhds.called)"},{"line_number":278,"context_line":"        self.assertTrue(mock_cleanup_staging_area.called)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def test_download_vhd(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_685562c4","line":277,"updated":"2016-04-20 14:38:18.000000000","message":"mock_import_vhds.assert_called_with(\u0027fake_sr_path\u0027, \u0027mock_staging_area\u0027, mock.ANY)","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"939b935c44584478da57e5539b98a9d72449ccb6","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        self.assertTrue(mock_make_staging_area.called)"},{"line_number":276,"context_line":"        self.assertTrue(mock_download_tarball_by_url.called)"},{"line_number":277,"context_line":"        self.assertTrue(mock_import_vhds.called)"},{"line_number":278,"context_line":"        self.assertTrue(mock_cleanup_staging_area.called)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    def test_download_vhd(self):"},{"line_number":281,"context_line":"        mock_make_staging_area \u003d self.mock_patch_object("}],"source_content_type":"text/x-python","patch_set":5,"id":"1a122d0e_e8c972ad","line":278,"updated":"2016-04-20 14:38:18.000000000","message":"mock_cleanup_staging_area.assert_called_with(\u0027fake_sr_path\u0027)","commit_id":"96d2b4abd71be1b96a0fe1eb473cae7ba3f3d94c"}]}