)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"261e0889e8bd5a24b9c3b133daae270afd5f36a2","unresolved":false,"context_lines":[{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Added tests for get_device_info and get_swift_conf_md5."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I149db8ef87573f4a859668d061dc5bef631c5f11"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Fixed PEP8 compliance errors by adding blank lines."},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1a26ad4f_d6ce472c","line":14,"updated":"2015-11-03 07:25:55.000000000","message":"Remove this change id (git commit --amend). The latter one in the msg reflects your latest changes as I see. You can click on those hyperlinks to verify.\n\nAlso, wrap up your commit msg to indicate the actual change i.e. what is it solving/testing - can mention pep8 changes in a patch comment once you upload.","commit_id":"332725b4824c519e51fbf437eadc5bd91a3b67cb"}],"test/unit/common/middleware/test_recon.py":[{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"261e0889e8bd5a24b9c3b133daae270afd5f36a2","unresolved":false,"context_lines":[{"line_number":1340,"context_line":"        old_os_listdir \u003d os.listdir"},{"line_number":1341,"context_line":"        os.listdir \u003d fail_os_listdir"},{"line_number":1342,"context_line":"        self.app.get_device_info \u003d self.real_app_get_device_info"},{"line_number":1343,"context_line":"        resp \u003d self.app.get_device_info()"},{"line_number":1344,"context_line":"        os.listdir \u003d old_os_listdir"},{"line_number":1345,"context_line":"        device_path \u003d resp.keys()[0]"},{"line_number":1346,"context_line":"        self.assertIsNone(resp[device_path])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_56f0f790","line":1343,"updated":"2015-11-03 07:25:55.000000000","message":"Do direct assignment: resp \u003d  self.real_app_get_device_info()","commit_id":"332725b4824c519e51fbf437eadc5bd91a3b67cb"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"261e0889e8bd5a24b9c3b133daae270afd5f36a2","unresolved":false,"context_lines":[{"line_number":1341,"context_line":"        os.listdir \u003d fail_os_listdir"},{"line_number":1342,"context_line":"        self.app.get_device_info \u003d self.real_app_get_device_info"},{"line_number":1343,"context_line":"        resp \u003d self.app.get_device_info()"},{"line_number":1344,"context_line":"        os.listdir \u003d old_os_listdir"},{"line_number":1345,"context_line":"        device_path \u003d resp.keys()[0]"},{"line_number":1346,"context_line":"        self.assertIsNone(resp[device_path])"},{"line_number":1347,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_360bfb8f","line":1344,"updated":"2015-11-03 07:25:55.000000000","message":"You can just use os.listdir \u003d self.real_listdir which is already created in SetUp. And with that you can remove the first line of old_os_listdit\n\nAlso, I understand why you\u0027re replacing it with original; since this is a one line thing, it is okay. In future you could mock it. For eg: line 998","commit_id":"332725b4824c519e51fbf437eadc5bd91a3b67cb"},{"author":{"_account_id":12261,"name":"Mahati","email":"mahati.chamarthy@gmail.com","username":"Mahati"},"change_message_id":"261e0889e8bd5a24b9c3b133daae270afd5f36a2","unresolved":false,"context_lines":[{"line_number":1353,"context_line":"    def test_get_swift_conf_md5_fail(self):"},{"line_number":1354,"context_line":"        \"\"\"Test get_swift_conf_md5 failure by failing file open\"\"\""},{"line_number":1355,"context_line":"        self.app.get_swift_conf_md5 \u003d self.real_app_get_swift_conf_md5"},{"line_number":1356,"context_line":"        resp \u003d self.app.get_swift_conf_md5(fail_io_open)"},{"line_number":1357,"context_line":"        self.assertIsNone(resp[\u0027/etc/swift/swift.conf\u0027])"},{"line_number":1358,"context_line":""},{"line_number":1359,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a26ad4f_162d9ffd","line":1356,"updated":"2015-11-03 07:25:55.000000000","message":"Do direct assignment: resp \u003d self.real_app_get_swift_conf_md5(fail_io_open)","commit_id":"332725b4824c519e51fbf437eadc5bd91a3b67cb"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"882f8a7c0a78450b1ffbb4bed906bdf35dae7019","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"def fail_os_listdir():"},{"line_number":38,"context_line":"    raise OSError(\u0027No such file or directory\u0027)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"def fail_io_open(file_path, open_mode):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa80f949_9b2cfeac","line":38,"updated":"2015-11-03 20:28:08.000000000","message":"I have to imagine somewhere it would look better if when this is needed we have an explict mock with tight scoping:\n\n    with mock.patch(\u0027os.listdir\u0027) as mock_listdir:\n        mock_listdir.side_effect \u003d OSError(errno.ENOENT, os.strerr(errno.ENOENT))","commit_id":"213b1a4900a75f6a0eac4af6ee53ca9c367bbd41"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"882f8a7c0a78450b1ffbb4bed906bdf35dae7019","unresolved":false,"context_lines":[{"line_number":1069,"context_line":"        os.listdir \u003d self.fake_list"},{"line_number":1070,"context_line":"        self.app \u003d recon.ReconMiddleware(FakeApp(), {\u0027object_recon\u0027: \"true\"})"},{"line_number":1071,"context_line":"        self.real_app_get_device_info \u003d self.app.get_device_info"},{"line_number":1072,"context_line":"        self.real_app_get_swift_conf_md5 \u003d self.app.get_swift_conf_md5"},{"line_number":1073,"context_line":"        os.listdir \u003d self.real_listdir"},{"line_number":1074,"context_line":"        # self.app.object_recon \u003d True"},{"line_number":1075,"context_line":"        self.app.get_mem \u003d self.frecon.fake_mem"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa80f949_56798b92","line":1072,"updated":"2015-11-03 20:28:08.000000000","message":"I think this module has some bad examples of old school manual mocking - and would really benifit from some cleanup to do things with mock and context managers.","commit_id":"213b1a4900a75f6a0eac4af6ee53ca9c367bbd41"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"882f8a7c0a78450b1ffbb4bed906bdf35dae7019","unresolved":false,"context_lines":[{"line_number":1333,"context_line":"    def test_get_device_info_function(self):"},{"line_number":1334,"context_line":"        \"\"\"Test get_device_info function call success\"\"\""},{"line_number":1335,"context_line":"        resp \u003d self.app.get_device_info()"},{"line_number":1336,"context_line":"        self.assertEqual([\u0027sdb1\u0027], resp[\u0027/srv/1/node\u0027])"},{"line_number":1337,"context_line":""},{"line_number":1338,"context_line":"    def test_get_device_info_fail(self):"},{"line_number":1339,"context_line":"        \"\"\"Test get_device_info failure by failing os.listdir\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa80f949_f608bf2a","line":1336,"updated":"2015-11-03 20:28:08.000000000","message":"are any other keys present?  might be better to show the full form of resp.","commit_id":"213b1a4900a75f6a0eac4af6ee53ca9c367bbd41"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"882f8a7c0a78450b1ffbb4bed906bdf35dae7019","unresolved":false,"context_lines":[{"line_number":1338,"context_line":"    def test_get_device_info_fail(self):"},{"line_number":1339,"context_line":"        \"\"\"Test get_device_info failure by failing os.listdir\"\"\""},{"line_number":1340,"context_line":"        os.listdir \u003d fail_os_listdir"},{"line_number":1341,"context_line":"        resp \u003d self.real_app_get_device_info()"},{"line_number":1342,"context_line":"        os.listdir \u003d self.real_listdir"},{"line_number":1343,"context_line":"        device_path \u003d resp.keys()[0]"},{"line_number":1344,"context_line":"        self.assertIsNone(resp[device_path])"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa80f949_9b039e0f","line":1341,"updated":"2015-11-03 20:28:08.000000000","message":"is there an assert missing here?","commit_id":"213b1a4900a75f6a0eac4af6ee53ca9c367bbd41"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"882f8a7c0a78450b1ffbb4bed906bdf35dae7019","unresolved":false,"context_lines":[{"line_number":1350,"context_line":""},{"line_number":1351,"context_line":"    def test_get_swift_conf_md5_fail(self):"},{"line_number":1352,"context_line":"        \"\"\"Test get_swift_conf_md5 failure by failing file open\"\"\""},{"line_number":1353,"context_line":"        resp \u003d self.real_app_get_swift_conf_md5(fail_io_open)"},{"line_number":1354,"context_line":"        self.assertIsNone(resp[\u0027/etc/swift/swift.conf\u0027])"},{"line_number":1355,"context_line":""},{"line_number":1356,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa80f949_56318ba1","line":1353,"updated":"2015-11-03 20:28:08.000000000","message":"oh ugh, so now we have self.app which has had a number of it\u0027s methods monkey patched to the FakeRecon responses - and then when you want to test the acctual middleware logic you have to capture the original middlware objects, but even then instead of using a mock.patch context we paramterize open (using the kwarg here to name it would have made it somewhat more obvious)\n\nI think `test_get_ring_md5_ioerror_produces_none_hash` in `TestReconSuccess` has a slightly better model for testing than `TestReconMiddleware`\n\n... honestly the whole mess is pretty confusing.","commit_id":"213b1a4900a75f6a0eac4af6ee53ca9c367bbd41"}]}