)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"e1c0d28e955c3992ba6b5c4f597826e1de771416","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Replace plain mock.Mock() call with mock.Mock(spec)"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In order to simulate the tests accurately, it is better to use objects"},{"line_number":10,"context_line":"of classes intead of plan Mock() calls. The changes made in the code"},{"line_number":11,"context_line":"replaces Mock() call with Mock(spec) or Mock(autospec\u003dTrue)."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Partial-Bug: #2004174"},{"line_number":14,"context_line":"Change-Id: I209ff5d0fd7e3eaa0b50dcbf71ff4b0960403f96"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5b9c6774_9743b82a","line":11,"range":{"start_line":10,"start_character":40,"end_line":11,"end_character":60},"updated":"2023-03-15 12:12:56.000000000","message":"Please rewrite this explaining why using Mock with `spec` will fix the problem of using plain Mock() calls. \n\nhttps://docs.python.org/3/library/unittest.mock.html#the-mock-class","commit_id":"126a44eb7f60cf37e5de22f9933e8429cba15354"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"0eca1a25fe4e56697ccc518b90918040cb995187","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Replace plain mock.Mock() call with mock.Mock(spec)"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In order to simulate the tests accurately, it is better to use objects"},{"line_number":10,"context_line":"of classes intead of plan Mock() calls. The changes made in the code"},{"line_number":11,"context_line":"replaces Mock() call with Mock(spec) or Mock(autospec\u003dTrue)."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Partial-Bug: #2004174"},{"line_number":14,"context_line":"Change-Id: I209ff5d0fd7e3eaa0b50dcbf71ff4b0960403f96"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"dc5d08d0_0ce3c03b","line":11,"range":{"start_line":10,"start_character":40,"end_line":11,"end_character":60},"in_reply_to":"5b9c6774_9743b82a","updated":"2023-03-15 12:30:39.000000000","message":"My last comment is wrong. However, it\u0027s necessary to rewrite the commit message.. Please explain why autospec\u003dTrue is the solution to the plain Mock call.","commit_id":"126a44eb7f60cf37e5de22f9933e8429cba15354"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"921c0d16aee31cc12ae00fdb72b5d919ac3cb262","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Khadija Kamran \u003ckamrankhadijadj@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-03-24 02:28:41 +0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Replace plain mock.Mock() call with mock.sentinel"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In order to simulate the tests accurately, it is better to use objects"},{"line_number":10,"context_line":"of classes intead of plain Mock() calls."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"f3f9864e_a3c69bca","line":7,"updated":"2023-03-24 12:47:20.000000000","message":"Just want to leave a comment here so you don\u0027t forget to update the commit message if you make the changes I suggested.\n\nI think you could make the subject line:\n\n  Improve test_execute_root_and_helper\n\nand then in the body you can explain the 3 changes (sentinel, autospec, assert_not_called) and why you made them.","commit_id":"e446e4e8b24d62afee4cb4b4d77525475981e71a"},{"author":{"_account_id":35839,"name":"Khadija Kamran","display_name":"khadija","email":"kamrankhadijadj@gmail.com","username":"khadija"},"change_message_id":"44c9aa6e92121e09da972534b825d7efac7ac8cc","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Khadija Kamran \u003ckamrankhadijadj@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-03-24 02:28:41 +0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Replace plain mock.Mock() call with mock.sentinel"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In order to simulate the tests accurately, it is better to use objects"},{"line_number":10,"context_line":"of classes intead of plain Mock() calls."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"432066e7_52979186","line":7,"in_reply_to":"f3f9864e_a3c69bca","updated":"2023-03-24 19:10:00.000000000","message":"I am sorry about missing this in the patch. Thank you for explaining everything.","commit_id":"e446e4e8b24d62afee4cb4b4d77525475981e71a"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"40065e323e09b08a50cefcbf395c8ecc451f0ec2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"326fe4f3_ff94b9da","updated":"2023-03-13 13:30:06.000000000","message":"Khadija, thank you for your work! Comments in line!","commit_id":"266316938b76400dbd95f4704c7869c7a9fa5a15"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"e1c0d28e955c3992ba6b5c4f597826e1de771416","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9b710ff9_9f6aef08","updated":"2023-03-15 12:12:56.000000000","message":"Great work! Just a few comments in line","commit_id":"126a44eb7f60cf37e5de22f9933e8429cba15354"},{"author":{"_account_id":35839,"name":"Khadija Kamran","display_name":"khadija","email":"kamrankhadijadj@gmail.com","username":"khadija"},"change_message_id":"8a7eb7cf84a93863e223601857dc11808acd0755","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7d33b35f_b98acc50","updated":"2023-03-15 11:59:50.000000000","message":"Hi Sofia! \nI have looked through the mock documentation, watched youtube tutorials and learned a lot about mock and its amazing features. \nBut I am still confused as to what can I pass here in spec. The get_helper being mocked is a method and not a class.\nKindly help me clear my understanding here.","commit_id":"126a44eb7f60cf37e5de22f9933e8429cba15354"},{"author":{"_account_id":33807,"name":"Jacob Wang","email":"jacob_wang1@dell.com","username":"jacob0522"},"change_message_id":"5caf0ac9e9820919cd01c47fe9cfdaaad91ec0d5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"57f0d4be_5a610e23","updated":"2023-03-15 06:03:20.000000000","message":"run-DellEMC PowerMAX CI","commit_id":"126a44eb7f60cf37e5de22f9933e8429cba15354"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"65d65a8bd09b0f60b633aa42b9d2eb766ead93bb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"51d61aec_5a78255c","updated":"2023-03-17 10:22:42.000000000","message":"Added my thoughts but I agree to the changes Brian mentioned.","commit_id":"9b146f4ebf341c9d8c897253e711fd5ba8c2d549"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b577ab91ee0cb7b2b319f5d5dedcce5a0b53e540","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5f642916_cba91840","updated":"2023-03-16 01:37:58.000000000","message":"Thanks for working on this!\n\nSee inline for specific comments.  Once you have revised the patch, this should be good to go.","commit_id":"9b146f4ebf341c9d8c897253e711fd5ba8c2d549"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"db5920297dcc2ffe681761a3b81d5496c814e233","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"c8748dbc_b40d9210","updated":"2023-03-24 12:40:31.000000000","message":"Looks good so far, but I think you can improve it a bit more.  See comment inline.","commit_id":"e446e4e8b24d62afee4cb4b4d77525475981e71a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"1da1d8b75be3dddc17b3e705260e4ff47a40d3f0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"e467c459_28b9f834","updated":"2023-03-27 12:53:25.000000000","message":"All my comments have been addressed.  LGTM!","commit_id":"41da45ddd3b9677b4ee7f82f1a59e3bca9dee615"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"f0068c53c24f0c734433501bc41e57998e87b2e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"909abbb5_cf83b385","updated":"2023-03-27 12:32:46.000000000","message":"Comments addressed! LGTM","commit_id":"41da45ddd3b9677b4ee7f82f1a59e3bca9dee615"}],"cinder/tests/unit/test_utils.py":[{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"40065e323e09b08a50cefcbf395c8ecc451f0ec2","unresolved":true,"context_lines":[{"line_number":65,"context_line":"    @mock.patch(\u0027cinder.utils.get_root_helper\u0027)"},{"line_number":66,"context_line":"    @mock.patch(\u0027cinder.utils.processutils.execute\u0027)"},{"line_number":67,"context_line":"    def test_execute_root_and_helper(self, mock_putils_exe, mock_get_helper):"},{"line_number":68,"context_line":"        mock_helper \u003d mock.Mock(spec\u003dcontext.RequestContext)"},{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"}],"source_content_type":"text/x-python","patch_set":1,"id":"b91292cb_5a4ec582","line":68,"updated":"2023-03-13 13:30:06.000000000","message":":-1: It is necessary to look at what the objective of this test is. The spec is not correct. What is being mocked is not the context.RequestContext but the helper.","commit_id":"266316938b76400dbd95f4704c7869c7a9fa5a15"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"0eca1a25fe4e56697ccc518b90918040cb995187","unresolved":true,"context_lines":[{"line_number":65,"context_line":"    @mock.patch(\u0027cinder.utils.get_root_helper\u0027)"},{"line_number":66,"context_line":"    @mock.patch(\u0027cinder.utils.processutils.execute\u0027)"},{"line_number":67,"context_line":"    def test_execute_root_and_helper(self, mock_putils_exe, mock_get_helper):"},{"line_number":68,"context_line":"        mock_helper \u003d mock.Mock(spec\u003dcontext.RequestContext)"},{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"}],"source_content_type":"text/x-python","patch_set":1,"id":"a588ac89_808a11ac","line":68,"in_reply_to":"79ad1f4a_4ab60800","updated":"2023-03-15 12:30:39.000000000","message":"Oh, it\u0027s true! Good catch, Sorry about that!\n\nYes, you can use autospec\u003dTrue instead of spec\u003dutils.get_root_helper.\n\nThe issue is that get_root_helper is a method and cannot be directly mocked with `spec\u003dutils.get_root_helper`, I think one alternative is spec_set to specify the method signature of get_root_helper, but that would make the test difficult to read. \n\nWhen `autospec\u003dTrue` is used, the Mock object will be created with a mock spec that is based on the object being replaced (in this case utils.get_root_helper). This means that the mock object will have the same attributes and methods as the original object and any attribute access or method call that is not defined on the mock object will raise an `AttributeError`.\n\nIn this patch, the Mock object for mock_helper is created with autospec\u003dTrue. This will ensure that the mock_helper object has the same attributes and methods as the original utils.get_root_helper function, and any attribute access or method call that is not defined on the mock_helper object will raise an AttributeError.","commit_id":"266316938b76400dbd95f4704c7869c7a9fa5a15"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"e1c0d28e955c3992ba6b5c4f597826e1de771416","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    @mock.patch(\u0027cinder.utils.get_root_helper\u0027)"},{"line_number":66,"context_line":"    @mock.patch(\u0027cinder.utils.processutils.execute\u0027)"},{"line_number":67,"context_line":"    def test_execute_root_and_helper(self, mock_putils_exe, mock_get_helper):"},{"line_number":68,"context_line":"        mock_helper \u003d mock.Mock(spec\u003dcontext.RequestContext)"},{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"}],"source_content_type":"text/x-python","patch_set":1,"id":"79ad1f4a_4ab60800","line":68,"in_reply_to":"b91292cb_5a4ec582","updated":"2023-03-15 12:12:56.000000000","message":"Done","commit_id":"266316938b76400dbd95f4704c7869c7a9fa5a15"},{"author":{"_account_id":35839,"name":"Khadija Kamran","display_name":"khadija","email":"kamrankhadijadj@gmail.com","username":"khadija"},"change_message_id":"8a7eb7cf84a93863e223601857dc11808acd0755","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    @mock.patch(\u0027cinder.utils.get_root_helper\u0027)"},{"line_number":66,"context_line":"    @mock.patch(\u0027cinder.utils.processutils.execute\u0027)"},{"line_number":67,"context_line":"    def test_execute_root_and_helper(self, mock_putils_exe, mock_get_helper):"},{"line_number":68,"context_line":"        mock_helper \u003d mock.Mock(spec\u003dcontext.RequestContext)"},{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ad6ebb1f_56aac92c","line":68,"in_reply_to":"b91292cb_5a4ec582","updated":"2023-03-15 11:59:50.000000000","message":"Hi Sofia! \nI have looked through the mock documentation, watched youtube tutorials and learned a lot about mock and its amazing features. \nBut I am still confused as to what can I pass here in spec. The get_helper being mocked is a method and not a class.\nKindly help me clear my understanding here.","commit_id":"266316938b76400dbd95f4704c7869c7a9fa5a15"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"e1c0d28e955c3992ba6b5c4f597826e1de771416","unresolved":true,"context_lines":[{"line_number":65,"context_line":"    @mock.patch(\u0027cinder.utils.get_root_helper\u0027)"},{"line_number":66,"context_line":"    @mock.patch(\u0027cinder.utils.processutils.execute\u0027)"},{"line_number":67,"context_line":"    def test_execute_root_and_helper(self, mock_putils_exe, mock_get_helper):"},{"line_number":68,"context_line":"        mock_helper \u003d mock.Mock(autospec\u003dTrue)"},{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"}],"source_content_type":"text/x-python","patch_set":2,"id":"e4b43512_7d4a048b","line":68,"range":{"start_line":68,"start_character":32,"end_line":68,"end_character":45},"updated":"2023-03-15 12:12:56.000000000","message":"I don\u0027t think autospec is doing what you are expecting here. I think you should keep the idea of your first patchet, be more explicit and use `Mock(spec\u003dutils.get_root_helper)`\n\nI would like to explain how using Mock with spec can fix the problem of using plain Mock() calls. By using Mock with spec, we can restrict the attributes and methods that are available on the mock object. This means that we can ensure that our mock objects behave more like the real objects they are replacing, which can help to prevent unexpected errors from occurring.\n\nFor example, if we create a mock object using plain Mock(), it wont have all of the attributes and methods of the real object it is replacing and Mock() will never fail if something is wrong. This can be problematic if we make assumptions about the behavior of the real object that turn out to be incorrect. However, if we use Mock with spec, we can restrict the mock object to only have the attributes and methods of the real object that we are interested in. This makes it much easier to write tests that accurately reflect the behavior of the real system.\n\nIn summary, by using Mock with spec, we can create more accurate and reliable mock objects, which can help to prevent unexpected errors in our tests.","commit_id":"126a44eb7f60cf37e5de22f9933e8429cba15354"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"0eca1a25fe4e56697ccc518b90918040cb995187","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    @mock.patch(\u0027cinder.utils.get_root_helper\u0027)"},{"line_number":66,"context_line":"    @mock.patch(\u0027cinder.utils.processutils.execute\u0027)"},{"line_number":67,"context_line":"    def test_execute_root_and_helper(self, mock_putils_exe, mock_get_helper):"},{"line_number":68,"context_line":"        mock_helper \u003d mock.Mock(autospec\u003dTrue)"},{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a51260a_8b488575","line":68,"range":{"start_line":68,"start_character":32,"end_line":68,"end_character":45},"in_reply_to":"e4b43512_7d4a048b","updated":"2023-03-15 12:30:39.000000000","message":"This is no valid here because get_root_helper is not a class or and instance. Done.","commit_id":"126a44eb7f60cf37e5de22f9933e8429cba15354"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"f983dc6a8538e29ca37818619ca290b1075ae4e4","unresolved":true,"context_lines":[{"line_number":65,"context_line":"    @mock.patch(\u0027cinder.utils.get_root_helper\u0027)"},{"line_number":66,"context_line":"    @mock.patch(\u0027cinder.utils.processutils.execute\u0027)"},{"line_number":67,"context_line":"    def test_execute_root_and_helper(self, mock_putils_exe, mock_get_helper):"},{"line_number":68,"context_line":"        mock_helper \u003d mock.Mock(spec\u003dutils.get_root_helper)"},{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"}],"source_content_type":"text/x-python","patch_set":3,"id":"8c5e846c_e8e8165e","line":68,"range":{"start_line":68,"start_character":32,"end_line":68,"end_character":58},"updated":"2023-03-15 13:38:15.000000000","message":"My bad, please go back to `autospec\u003dTrue`. In case someone else see this the reason is `get_root_helper` is a method and cannot be directly mocked with `spec\u003dutils.get_root_helper` .","commit_id":"2e04a729056f79ef77c85b6152425dffba94def4"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b577ab91ee0cb7b2b319f5d5dedcce5a0b53e540","unresolved":true,"context_lines":[{"line_number":65,"context_line":"    @mock.patch(\u0027cinder.utils.get_root_helper\u0027)"},{"line_number":66,"context_line":"    @mock.patch(\u0027cinder.utils.processutils.execute\u0027)"},{"line_number":67,"context_line":"    def test_execute_root_and_helper(self, mock_putils_exe, mock_get_helper):"},{"line_number":68,"context_line":"        mock_helper \u003d mock.Mock(autospec\u003dTrue)"},{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"}],"source_content_type":"text/x-python","patch_set":4,"id":"aa3e0e1b_192eabae","line":68,"updated":"2023-03-16 01:37:58.000000000","message":"I don\u0027t think that passing autospec here actually does anything ... how does the test code know what class is being spec\u0027d here?  I think that the initializer just ignores the argument and you get a regular (non-spec\u0027d) mock object.  You can try this for yourself:\n\n    mock_helper \u003d mock.Mock(garbage\u003d\u0027whatever\u0027)\n    \ndoesn\u0027t raise an error, it just creates a standard mock object.\n\nWhen you look at what mock_helper is being used for, we\u0027re just passing it at line 70 and checking to make sure that utils.execute() doesn\u0027t call get_root_helper because we\u0027re giving it the helper we want.  So the Mock is being created here just to have something to pass, and so that we can verify that this thing was passed to processutils.execute() at line 73.  So what you could do to refactor this is to use a \u0027sentinel\u0027 instead of a full-blown mock object, that is, change line 68 to\n\n    mock_helper \u003d mock.sentinel\n\nThis gives us some extra protection, because a sentinel object isn\u0027t callable ... so if the code tries to use it like a function, it will raise a TypeError.\n\nNow about the autospec\u0027ing ... you can pass it when you patch whatever it is you want to mock, because at that point, the code knows what kind of thing we\u0027re talking about.  So, for example, if you put a print statement after line 68 to see what mock_get_helper is right now, you\u0027ll see something like this:\n\n    \u003cMagicMock name\u003d\u0027get_root_helper\u0027 id\u003d\u0027140318857424336\u0027\u003e\n\nthat is, you\u0027re dealing with a full-blown mock object.  If you leave the print statement in, and change line 65 to:\n\n    @mock.patch(\u0027cinder.utils.get_root_helper\u0027, autospec\u003dTrue)\n\nyou\u0027ll see something like this:\n\n    \u003cfunction get_root_helper at 0x7f10e24e5ea0\u003e\n\nwhich gives us the specificity you\u0027re looking for.","commit_id":"9b146f4ebf341c9d8c897253e711fd5ba8c2d549"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"65d65a8bd09b0f60b633aa42b9d2eb766ead93bb","unresolved":true,"context_lines":[{"line_number":65,"context_line":"    @mock.patch(\u0027cinder.utils.get_root_helper\u0027)"},{"line_number":66,"context_line":"    @mock.patch(\u0027cinder.utils.processutils.execute\u0027)"},{"line_number":67,"context_line":"    def test_execute_root_and_helper(self, mock_putils_exe, mock_get_helper):"},{"line_number":68,"context_line":"        mock_helper \u003d mock.Mock(autospec\u003dTrue)"},{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"}],"source_content_type":"text/x-python","patch_set":4,"id":"f305bf66_142f228f","line":68,"in_reply_to":"aa3e0e1b_192eabae","updated":"2023-03-17 10:22:42.000000000","message":"I can\u0027t describe it in a better/elaborate way of how Brian has already done but just to add a few points:\n\n1) the autospec\u003dTrue should be done while patching a function or class and not when creating a mock object (it might be possible to autospec a mock object but not sure how that would work out)\n2) passing autospec\u003dTrue to the mock object creates an attribute of the mock object that can be accessed like, mock_helper.autospec (value is True) which is of no use to us so there isn\u0027t any autospec\u0027ing going on here.\n\nAs Brian mentioned, you can do the autospec on the mocked methods but doing it on mock_helper, which is a replacement of the rootwrap command (sudo cinder-rootwrap ...), is not useful here since it doesn\u0027t replace any class object/method.","commit_id":"9b146f4ebf341c9d8c897253e711fd5ba8c2d549"},{"author":{"_account_id":35839,"name":"Khadija Kamran","display_name":"khadija","email":"kamrankhadijadj@gmail.com","username":"khadija"},"change_message_id":"44c9aa6e92121e09da972534b825d7efac7ac8cc","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    @mock.patch(\u0027cinder.utils.get_root_helper\u0027)"},{"line_number":66,"context_line":"    @mock.patch(\u0027cinder.utils.processutils.execute\u0027)"},{"line_number":67,"context_line":"    def test_execute_root_and_helper(self, mock_putils_exe, mock_get_helper):"},{"line_number":68,"context_line":"        mock_helper \u003d mock.Mock(autospec\u003dTrue)"},{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"}],"source_content_type":"text/x-python","patch_set":4,"id":"baedf821_65538b6b","line":68,"in_reply_to":"f305bf66_142f228f","updated":"2023-03-24 19:10:00.000000000","message":"Thank you Brian and Rajat for helping with this.\nThis is great help.","commit_id":"9b146f4ebf341c9d8c897253e711fd5ba8c2d549"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"b577ab91ee0cb7b2b319f5d5dedcce5a0b53e540","unresolved":true,"context_lines":[{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"},{"line_number":72,"context_line":"        self.assertFalse(mock_get_helper.called)"},{"line_number":73,"context_line":"        mock_putils_exe.assert_called_once_with(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027,"},{"line_number":74,"context_line":"                                                run_as_root\u003dTrue,"},{"line_number":75,"context_line":"                                                root_helper\u003dmock_helper)"}],"source_content_type":"text/x-python","patch_set":4,"id":"d758c7d5_d7356db1","line":72,"range":{"start_line":72,"start_character":8,"end_line":72,"end_character":48},"updated":"2023-03-16 01:37:58.000000000","message":"while you\u0027re refactoring this, I think it would be better to express this line like:\n\n    mock_get_helper.assert_not_called()","commit_id":"9b146f4ebf341c9d8c897253e711fd5ba8c2d549"},{"author":{"_account_id":35839,"name":"Khadija Kamran","display_name":"khadija","email":"kamrankhadijadj@gmail.com","username":"khadija"},"change_message_id":"44c9aa6e92121e09da972534b825d7efac7ac8cc","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        output \u003d utils.execute(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027, run_as_root\u003dTrue,"},{"line_number":70,"context_line":"                               root_helper\u003dmock_helper)"},{"line_number":71,"context_line":"        self.assertEqual(mock_putils_exe.return_value, output)"},{"line_number":72,"context_line":"        self.assertFalse(mock_get_helper.called)"},{"line_number":73,"context_line":"        mock_putils_exe.assert_called_once_with(\u0027a\u0027, 1, foo\u003d\u0027bar\u0027,"},{"line_number":74,"context_line":"                                                run_as_root\u003dTrue,"},{"line_number":75,"context_line":"                                                root_helper\u003dmock_helper)"}],"source_content_type":"text/x-python","patch_set":4,"id":"d3b6d5f2_a73ccf4c","line":72,"range":{"start_line":72,"start_character":8,"end_line":72,"end_character":48},"in_reply_to":"d758c7d5_d7356db1","updated":"2023-03-24 19:10:00.000000000","message":"Understood","commit_id":"9b146f4ebf341c9d8c897253e711fd5ba8c2d549"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"db5920297dcc2ffe681761a3b81d5496c814e233","unresolved":true,"context_lines":[{"line_number":62,"context_line":"                                                run_as_root\u003dTrue,"},{"line_number":63,"context_line":"                                                root_helper\u003dmock_helper)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    @mock.patch(\u0027cinder.utils.get_root_helper\u0027)"},{"line_number":66,"context_line":"    @mock.patch(\u0027cinder.utils.processutils.execute\u0027)"},{"line_number":67,"context_line":"    def test_execute_root_and_helper(self, mock_putils_exe, mock_get_helper):"},{"line_number":68,"context_line":"        mock_helper \u003d mock.sentinel"}],"source_content_type":"text/x-python","patch_set":5,"id":"e44a5d84_0a0c0d39","line":65,"range":{"start_line":65,"start_character":4,"end_line":65,"end_character":47},"updated":"2023-03-24 12:40:31.000000000","message":"Sorry I wasn\u0027t more clear on my last set of comments.  My point wasn\u0027t that you shouldn\u0027t use autospec\u0027ing, but rather that you were applying autospec\u003dTrue in the wrong place.  I think it makes sense to autospec here like this:\n\n  @mock.patch(\u0027cinder.utils.get_root_helper\u0027, autospec\u003dTrue)\n\nIt would be more important if we expected the get_root_helper() mock to be called during the test (since we\u0027re checking at line 72 to make sure it\u0027s not called, who cares what kind of mock it is, since it was never called), but you have to remember that a lot of people work on this code, and a lot of time when tests are added to a file, they are copied-and-pasted from existing tests.  So since part of your goal in fixing this bug is to have tests use autospec\u0027ing to prevent tests from passing when they shouldn\u0027t, I think you should make this change to set a good example.\n\nAnd while you\u0027re at it, I think you can make the same change to line 66.  That mock actually is called, so the autospec\u0027ing does matter there!","commit_id":"e446e4e8b24d62afee4cb4b4d77525475981e71a"}]}