)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a7633396f9530cfe2ef1f0e73f2d6f3cddbcfef7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"88cee5ad_c8f395e7","updated":"2022-03-17 10:38:49.000000000","message":"Thanks Cyril for review, please find my reply inline.","commit_id":"f27ebb4d49dc378b47c3ac682ad0c160a7247222"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"0e5eb2167cb62e6e8f3fb941a53dbebd852234aa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"43a8bb8d_63070f0c","updated":"2022-05-04 17:08:21.000000000","message":"I still disagree with the approach, but I\u0027ll trust Rajat, Dan \u0026 Abhishek and will remove my veto.","commit_id":"e5a3c9eefa83f173f12ee38804207e94bbdfa544"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e9213915857f4b3002b06671dd6c7e669dc934d0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"20a4be30_dd38c05b","updated":"2022-04-28 14:02:05.000000000","message":"I\u0027ll leave my +2 here and not +W until Cyril has a chance to respond.","commit_id":"e5a3c9eefa83f173f12ee38804207e94bbdfa544"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"7afda48e68a7c024d8b5702a3859328eb0b28efc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2176d39a_e5ea0b36","updated":"2022-03-24 03:43:47.000000000","message":"recheck\n\nUnrelated failure in tempest.api.compute.servers.test_device_tagging.TaggedAttachmentsTest.test_tagged_attachment\n\ntempest.lib.exceptions.SSHTimeout: Connection to the 172.24.5.79 via SSH timed out.\nUser: cirros, Password: None\n","commit_id":"e5a3c9eefa83f173f12ee38804207e94bbdfa544"}],"glance_store/tests/unit/test_cinder_base.py":[{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"2ebdab1e101e012f4408b3d1b9f373fdfb31dc36","unresolved":true,"context_lines":[{"line_number":580,"context_line":"        with mock.patch.object(cinder.hashlib, \u0027sha256\u0027) as fake_hashlib:"},{"line_number":581,"context_line":"            self.store.get_hash_str(test_str)"},{"line_number":582,"context_line":"            test_str \u003d test_str.encode(\u0027utf-8\u0027)"},{"line_number":583,"context_line":"            fake_hashlib.assert_called_once_with(test_str)"},{"line_number":584,"context_line":""},{"line_number":585,"context_line":"    def test__get_mount_path(self):"},{"line_number":586,"context_line":"        fake_hex \u003d \u0027fake_hex_digest\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"7937285b_107da5b7","line":583,"updated":"2022-03-16 20:40:53.000000000","message":"I don\u0027t really like testing that methods were called, and called with the right arguments, even though it is very often the easiest way. Here, I think we could do:\n\ntest_str \u003d \u0027test_str\u0027\nexpected \u003d \u00279c02dd5524a5585746ff970192b4862fdd957dd024449f8e79135be3f3e91f39\u0027\nself.assertEqual(cinder.hashlib.sha256(test_str.encode(\u0027utf-8\u0027)), expected)\n\nThis way we would do a \"black box\" testing of _get_hash_str. WDYT?","commit_id":"f27ebb4d49dc378b47c3ac682ad0c160a7247222"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e9213915857f4b3002b06671dd6c7e669dc934d0","unresolved":true,"context_lines":[{"line_number":580,"context_line":"        with mock.patch.object(cinder.hashlib, \u0027sha256\u0027) as fake_hashlib:"},{"line_number":581,"context_line":"            self.store.get_hash_str(test_str)"},{"line_number":582,"context_line":"            test_str \u003d test_str.encode(\u0027utf-8\u0027)"},{"line_number":583,"context_line":"            fake_hashlib.assert_called_once_with(test_str)"},{"line_number":584,"context_line":""},{"line_number":585,"context_line":"    def test__get_mount_path(self):"},{"line_number":586,"context_line":"        fake_hex \u003d \u0027fake_hex_digest\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"2cc020d6_a888bd21","line":583,"in_reply_to":"5e780323_7df2cd21","updated":"2022-04-28 14:02:05.000000000","message":"Yeah, I\u0027m really not sure what the problem is here. We mock libraries all the time to make sure we call them properly. This seems totally okay to me.","commit_id":"f27ebb4d49dc378b47c3ac682ad0c160a7247222"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"a7633396f9530cfe2ef1f0e73f2d6f3cddbcfef7","unresolved":true,"context_lines":[{"line_number":580,"context_line":"        with mock.patch.object(cinder.hashlib, \u0027sha256\u0027) as fake_hashlib:"},{"line_number":581,"context_line":"            self.store.get_hash_str(test_str)"},{"line_number":582,"context_line":"            test_str \u003d test_str.encode(\u0027utf-8\u0027)"},{"line_number":583,"context_line":"            fake_hashlib.assert_called_once_with(test_str)"},{"line_number":584,"context_line":""},{"line_number":585,"context_line":"    def test__get_mount_path(self):"},{"line_number":586,"context_line":"        fake_hex \u003d \u0027fake_hex_digest\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"abc498cd_95d2e6d8","line":583,"in_reply_to":"7937285b_107da5b7","updated":"2022-03-17 10:38:49.000000000","message":"The purpose of this test is to verify if hashlib.sha256 is called with the string we pass to \"get_hash_str\" (in utf-8 encoded form).\nThe test you proposed is more of checking if hashlib is calculating the hash correctly which is beyond the purpose of this test since it\u0027s a python lib and we don\u0027t account for if it\u0027s working correctly or not. Also we try to mock out external dependencies in unit tests which in this case is hashlib.\nI don\u0027t mind adding your proposed test but i still think we\u0027re testing the right thing here irrespective of if it looks easy or complex.\nLet me know if you would still like the new test to be added and I will update in next PS.","commit_id":"f27ebb4d49dc378b47c3ac682ad0c160a7247222"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"24b567aeb192aa01b0e0dd88bd0e629c83d59fa0","unresolved":true,"context_lines":[{"line_number":580,"context_line":"        with mock.patch.object(cinder.hashlib, \u0027sha256\u0027) as fake_hashlib:"},{"line_number":581,"context_line":"            self.store.get_hash_str(test_str)"},{"line_number":582,"context_line":"            test_str \u003d test_str.encode(\u0027utf-8\u0027)"},{"line_number":583,"context_line":"            fake_hashlib.assert_called_once_with(test_str)"},{"line_number":584,"context_line":""},{"line_number":585,"context_line":"    def test__get_mount_path(self):"},{"line_number":586,"context_line":"        fake_hex \u003d \u0027fake_hex_digest\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"5e780323_7df2cd21","line":583,"in_reply_to":"abc498cd_95d2e6d8","updated":"2022-03-22 07:50:14.000000000","message":"I think both the checks serves the purpose here but somehow I think rajat is right about mocking third party library here. Cyril please let me know if you thinks otherwise.\n\nThank you!","commit_id":"f27ebb4d49dc378b47c3ac682ad0c160a7247222"}]}