)]}'
{"oslo_concurrency/tests/unit/test_lockutils.py":[{"author":{"_account_id":28522,"name":"Hervé Beraud","email":"herveberaud.pro@gmail.com","username":"hberaud"},"change_message_id":"90c0657c381ebae5b41512bd012bade8c644655b","unresolved":false,"context_lines":[{"line_number":329,"context_line":"            self._test_remove_lock_external_file(tempfile.mkdtemp(),"},{"line_number":330,"context_line":"                                                 use_external\u003dTrue)"},{"line_number":331,"context_line":"            # No log call means we actually deleted the file"},{"line_number":332,"context_line":"            log_mock.assert_not_called()"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"    def test_remove_lock_external_file_lock_path_doesnt_exist(self):"},{"line_number":335,"context_line":"        with mock.patch.object(lockutils.LOG, \u0027info\u0027) as log_mock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_0d0a2478","line":332,"updated":"2019-10-28 15:21:55.000000000","message":"I suppose that you waiting for this log call:\nhttps://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/lockutils.py#L204\n\nPlease, confirm if I\u0027m right.\n\nThen if I\u0027m right can we instead test for an `os.remove` call by mocking this one?\n\nI think it would be less misleading to explicitly assert on os.remove rather than expecting log call.","commit_id":"9ee959422937b6bb8b0f878f77e5ca06496e2ad4"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1977123da27f5dc95e4565832521d3409ab256e9","unresolved":false,"context_lines":[{"line_number":329,"context_line":"            self._test_remove_lock_external_file(tempfile.mkdtemp(),"},{"line_number":330,"context_line":"                                                 use_external\u003dTrue)"},{"line_number":331,"context_line":"            # No log call means we actually deleted the file"},{"line_number":332,"context_line":"            log_mock.assert_not_called()"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"    def test_remove_lock_external_file_lock_path_doesnt_exist(self):"},{"line_number":335,"context_line":"        with mock.patch.object(lockutils.LOG, \u0027info\u0027) as log_mock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_b2d079ca","line":332,"in_reply_to":"3fa7e38b_0d0a2478","updated":"2019-12-17 16:54:29.000000000","message":"I was thinking the same, but if we do that we\u0027ll have to still remove the file manually to ensure we don\u0027t leave noise on the system. These tests are bad andare more functional than unit (we shouldn\u0027t have to create real files and should instead mock everything), but this is an improvement on what we have so I\u0027m okay with it","commit_id":"9ee959422937b6bb8b0f878f77e5ca06496e2ad4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0281a11b7f2a65cbc4125c29f27628e4c129cac2","unresolved":false,"context_lines":[{"line_number":329,"context_line":"            self._test_remove_lock_external_file(tempfile.mkdtemp(),"},{"line_number":330,"context_line":"                                                 use_external\u003dTrue)"},{"line_number":331,"context_line":"            # No log call means we actually deleted the file"},{"line_number":332,"context_line":"            log_mock.assert_not_called()"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"    def test_remove_lock_external_file_lock_path_doesnt_exist(self):"},{"line_number":335,"context_line":"        with mock.patch.object(lockutils.LOG, \u0027info\u0027) as log_mock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_0ad1e2ca","line":332,"in_reply_to":"3fa7e38b_9bb3ffec","updated":"2019-12-20 12:34:49.000000000","message":"Thanks for your reviews and for your willingness to accept the patch.  Truth is I got caught up with the fact that we were using tempfile.mkdtemp(), assumed you preferred to test it with a real file, and didn\u0027t write proper unit tests.\n\nI see that you prefer proper unit tests, so I\u0027ll go ahead and write them, which is what I should have done from the start.","commit_id":"9ee959422937b6bb8b0f878f77e5ca06496e2ad4"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"2807f12c52d19a7d1b370dae045c0127cd89d2db","unresolved":false,"context_lines":[{"line_number":329,"context_line":"            self._test_remove_lock_external_file(tempfile.mkdtemp(),"},{"line_number":330,"context_line":"                                                 use_external\u003dTrue)"},{"line_number":331,"context_line":"            # No log call means we actually deleted the file"},{"line_number":332,"context_line":"            log_mock.assert_not_called()"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"    def test_remove_lock_external_file_lock_path_doesnt_exist(self):"},{"line_number":335,"context_line":"        with mock.patch.object(lockutils.LOG, \u0027info\u0027) as log_mock:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa7e38b_9bb3ffec","line":332,"in_reply_to":"3fa7e38b_b2d079ca","updated":"2019-12-18 22:30:51.000000000","message":"Possibly not since we\u0027re using tempfile here and this test class appears to inherit from oslotest so it gets the NestedTempfile fixture. That should clean up the leftover lock file either way.\n\nOn the other hand, this fixes a pretty big oops in the tests and if we want to try another approach that can be done in a followup.","commit_id":"9ee959422937b6bb8b0f878f77e5ca06496e2ad4"}]}