)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"2e02aa649ab93a727cc46e8f4f99094cfc272cfc","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Prevent tests from using utils.execute()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change introduces a new base test class that mocks out"},{"line_number":10,"context_line":"utils.execute and forces an exception if it gets called."},{"line_number":11,"context_line":"This has rooted out many tests that were doing this as a side effect of"},{"line_number":12,"context_line":"calling other functions, doing things like modprobe and running iscsi"},{"line_number":13,"context_line":"on the host\u0027s actual machine."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3f044301_502075ac","line":10,"updated":"2017-05-11 20:24:21.000000000","message":"Maybe something in here that if the test case mocks utils.execute then it will work. That this is preventing unmocked access to utils.execute.","commit_id":"acb840ee5d3f43b39d96a710e1425139f0e977ed"},{"author":{"_account_id":15750,"name":"Julian Edwards","email":"bigjools@gmail.com","username":"bigjools"},"change_message_id":"d7da0b2a8410b373809c6a93d0321a6d8ae2f38c","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Prevent tests from using utils.execute()"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This change introduces a new base test class that mocks out"},{"line_number":10,"context_line":"utils.execute and forces an exception if it gets called."},{"line_number":11,"context_line":"This has rooted out many tests that were doing this as a side effect of"},{"line_number":12,"context_line":"calling other functions, doing things like modprobe and running iscsi"},{"line_number":13,"context_line":"on the host\u0027s actual machine."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"1f013ff3_ed1747ed","line":10,"in_reply_to":"3f044301_502075ac","updated":"2017-05-15 00:54:24.000000000","message":"Done","commit_id":"acb840ee5d3f43b39d96a710e1425139f0e977ed"}],"ironic_python_agent/tests/unit/base.py":[{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"1f6a93b06d939adff815990eb0becf9d5be1ae2a","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        self._exec_patch \u003d mock.Mock()"},{"line_number":41,"context_line":"        self._exec_patch.side_effect \u003d Exception("},{"line_number":42,"context_line":"            \"Don\u0027t call utils.execute in tests!\")"},{"line_number":43,"context_line":"        self.patch(utils, \u0027execute\u0027, self._exec_patch)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1f013ff3_0280fa8a","line":43,"updated":"2017-05-15 08:29:43.000000000","message":"It seems like it will be called after a decorator on a test method. I\u0027m talking about the following case:\n\n @mock.patch(\u0027utils.execute\u0027)\n def test_case(exec_mock):\n   some_stuff_that_calls_execute()\n\nwill raise an exception, as setUp will be called after decorator is applied and will override the mock in the decorator. I guess it\u0027s better to check is execute is already a mock, and only if it is not, patch it. Will check this locally.","commit_id":"f57cbccf8b146152eacd99787a7ccc1097b80055"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"9faa67ff535b01a6399938b0d03d824a4a7de355","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        self._exec_patch \u003d mock.Mock()"},{"line_number":41,"context_line":"        self._exec_patch.side_effect \u003d Exception("},{"line_number":42,"context_line":"            \"Don\u0027t call utils.execute in tests!\")"},{"line_number":43,"context_line":"        self.patch(utils, \u0027execute\u0027, self._exec_patch)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1f013ff3_b6cbc7a3","line":43,"in_reply_to":"1f013ff3_0280fa8a","updated":"2017-05-15 08:46:10.000000000","message":"Nope, seems like I\u0027m wrong here, if decorator is present, everything runs fine, thanks for the patch!","commit_id":"f57cbccf8b146152eacd99787a7ccc1097b80055"}],"ironic_python_agent/tests/unit/common.py":[{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"1b1f0ae12d1efcefc0eea3f97741cabc4260f84a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3f044301_e59b4870","updated":"2017-05-11 16:01:55.000000000","message":"I wonder if this file should be called \u0027base.py\u0027\n\nWe have something similar in ironic. A base test class.","commit_id":"acb840ee5d3f43b39d96a710e1425139f0e977ed"},{"author":{"_account_id":15750,"name":"Julian Edwards","email":"bigjools@gmail.com","username":"bigjools"},"change_message_id":"d7da0b2a8410b373809c6a93d0321a6d8ae2f38c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"1f013ff3_ad093fde","in_reply_to":"3f044301_e59b4870","updated":"2017-05-15 00:54:24.000000000","message":"No problem, I renamed it to base. It clashes with two other things called \u0027base\u0027 now,  in different places, so I had to rename the import in those places.","commit_id":"acb840ee5d3f43b39d96a710e1425139f0e977ed"}],"ironic_python_agent/tests/unit/test_agent.py":[{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"1b29b634c5a8a2fe9e11afadd9ff2249efa7a73d","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        self.assertEqual(pkg_resources.get_distribution(\u0027ironic-python-agent\u0027)"},{"line_number":172,"context_line":"                         .version, status.version)"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    @mock.patch.object("},{"line_number":175,"context_line":"        hardware, \u0027_check_for_iscsi\u0027, mock.MagicMock(autospec\u003dTrue))"},{"line_number":176,"context_line":"    @mock.patch("},{"line_number":177,"context_line":"        \u0027ironic_python_agent.hardware_managers.cna._detect_cna_card\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_f5515a64","line":174,"updated":"2017-04-05 15:28:45.000000000","message":"These would have triggered real execution on the machine running the tests before, right? Just checking my understanding.","commit_id":"f8575f4e6e07751245896d1bee38b6289cbd400c"},{"author":{"_account_id":15750,"name":"Julian Edwards","email":"bigjools@gmail.com","username":"bigjools"},"change_message_id":"d0541aaecda6d855b66e498bfa0370bc0bc77dbc","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        self.assertEqual(pkg_resources.get_distribution(\u0027ironic-python-agent\u0027)"},{"line_number":172,"context_line":"                         .version, status.version)"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    @mock.patch.object("},{"line_number":175,"context_line":"        hardware, \u0027_check_for_iscsi\u0027, mock.MagicMock(autospec\u003dTrue))"},{"line_number":176,"context_line":"    @mock.patch("},{"line_number":177,"context_line":"        \u0027ironic_python_agent.hardware_managers.cna._detect_cna_card\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_dc99d01f","line":174,"in_reply_to":"bff0334d_f5515a64","updated":"2017-04-05 23:08:14.000000000","message":"Yes indeed. A bit shocking!","commit_id":"f8575f4e6e07751245896d1bee38b6289cbd400c"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"86501d78f0aa084db42fc85e868b98f803819921","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        self.agent.heartbeater.start.assert_called_once_with()"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @mock.patch.object("},{"line_number":212,"context_line":"        hardware, \u0027_check_for_iscsi\u0027, mock.MagicMock(autospec\u003dTrue))"},{"line_number":213,"context_line":"    @mock.patch(\u0027ironic_python_agent.hardware_managers.cna._detect_cna_card\u0027,"},{"line_number":214,"context_line":"                mock.MagicMock(autospec\u003dTrue))"},{"line_number":215,"context_line":"    @mock.patch.object(agent.IronicPythonAgent,"}],"source_content_type":"text/x-python","patch_set":5,"id":"5ff73747_e1c32756","line":212,"updated":"2017-04-28 10:22:56.000000000","message":"MagicMock does not have an autospec argument. also there is no need for MagicMock here. what you want to use, I think, is\n\n mock.Mock(spec\u003dhardware._check_for_iscsi)\n\nor you can use https://docs.python.org/3/library/unittest.mock.html#unittest.mock.create_autospec for the same purpose.\n\nthe same comment applies everywhere.","commit_id":"2956c4d098cb557a6824b5c5ba333523f8b1f0ff"},{"author":{"_account_id":15750,"name":"Julian Edwards","email":"bigjools@gmail.com","username":"bigjools"},"change_message_id":"081cb118ea75156a57277a54ebf544cc79109c5e","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        self.agent.heartbeater.start.assert_called_once_with()"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    @mock.patch.object("},{"line_number":212,"context_line":"        hardware, \u0027_check_for_iscsi\u0027, mock.MagicMock(autospec\u003dTrue))"},{"line_number":213,"context_line":"    @mock.patch(\u0027ironic_python_agent.hardware_managers.cna._detect_cna_card\u0027,"},{"line_number":214,"context_line":"                mock.MagicMock(autospec\u003dTrue))"},{"line_number":215,"context_line":"    @mock.patch.object(agent.IronicPythonAgent,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f044301_083e5ad9","line":212,"in_reply_to":"5ff73747_e1c32756","updated":"2017-05-11 04:01:45.000000000","message":"I realised there\u0027s no need for speccing at all, so I just turned everything into a Mock.\n\nAlso BTW, @patch() is very awkward when you want to autospec AND prevent passing its generated Mock as a function parameter :(","commit_id":"2956c4d098cb557a6824b5c5ba333523f8b1f0ff"}],"ironic_python_agent/tests/unit/test_hardware.py":[{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"1b1f0ae12d1efcefc0eea3f97741cabc4260f84a","unresolved":false,"context_lines":[{"line_number":258,"context_line":"        return self._hardware_support"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"class TestHardwareManagerLoading(test_base.BaseTestCase):"},{"line_number":262,"context_line":"    def setUp(self):"},{"line_number":263,"context_line":"        super(TestHardwareManagerLoading, self).setUp()"},{"line_number":264,"context_line":"        # In order to use ExtensionManager.make_test_instance() without"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_c5a60cac","line":261,"updated":"2017-05-11 16:01:55.000000000","message":"Why not this one?","commit_id":"acb840ee5d3f43b39d96a710e1425139f0e977ed"},{"author":{"_account_id":15750,"name":"Julian Edwards","email":"bigjools@gmail.com","username":"bigjools"},"change_message_id":"d7da0b2a8410b373809c6a93d0321a6d8ae2f38c","unresolved":false,"context_lines":[{"line_number":258,"context_line":"        return self._hardware_support"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"class TestHardwareManagerLoading(test_base.BaseTestCase):"},{"line_number":262,"context_line":"    def setUp(self):"},{"line_number":263,"context_line":"        super(TestHardwareManagerLoading, self).setUp()"},{"line_number":264,"context_line":"        # In order to use ExtensionManager.make_test_instance() without"}],"source_content_type":"text/x-python","patch_set":6,"id":"1f013ff3_2d91af53","line":261,"in_reply_to":"3f044301_c5a60cac","updated":"2017-05-15 00:54:24.000000000","message":"It wasn\u0027t doing any mocking so my exploratory tests didn\u0027t pick it up as a problem. I\u0027ve changed it now so future tests are protected.\n\nSame below.","commit_id":"acb840ee5d3f43b39d96a710e1425139f0e977ed"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"1b1f0ae12d1efcefc0eea3f97741cabc4260f84a","unresolved":false,"context_lines":[{"line_number":1619,"context_line":""},{"line_number":1620,"context_line":"@mock.patch.object(os, \u0027listdir\u0027, lambda *_: [])"},{"line_number":1621,"context_line":"@mock.patch.object(utils, \u0027execute\u0027, autospec\u003dTrue)"},{"line_number":1622,"context_line":"class TestModuleFunctions(test_base.BaseTestCase):"},{"line_number":1623,"context_line":""},{"line_number":1624,"context_line":"    @mock.patch.object(hardware, \u0027_get_device_info\u0027,"},{"line_number":1625,"context_line":"                       lambda x, y, z: \u0027FooTastic\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_45b43cd1","line":1622,"updated":"2017-05-11 16:01:55.000000000","message":"Why not this one?","commit_id":"acb840ee5d3f43b39d96a710e1425139f0e977ed"}],"ironic_python_agent/tests/unit/test_multi_hardware.py":[{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"1b29b634c5a8a2fe9e11afadd9ff2249efa7a73d","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        self.mocked_extension_mgr.return_value \u003d self.fake_ext_mgr"},{"line_number":125,"context_line":"        hardware._global_managers \u003d None"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    def tearDown(self):"},{"line_number":128,"context_line":"        super(TestMultipleHardwareManagerLoading, self).tearDown()"},{"line_number":129,"context_line":"        self.extension_mgr_patcher.stop()"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_55e9267e","side":"PARENT","line":127,"updated":"2017-04-05 15:28:45.000000000","message":"Why was this changed? Seems unrelated","commit_id":"4be702242eb48e09eb77e46d997908b7c519127d"},{"author":{"_account_id":15750,"name":"Julian Edwards","email":"bigjools@gmail.com","username":"bigjools"},"change_message_id":"d0541aaecda6d855b66e498bfa0370bc0bc77dbc","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        self.mocked_extension_mgr.return_value \u003d self.fake_ext_mgr"},{"line_number":125,"context_line":"        hardware._global_managers \u003d None"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    def tearDown(self):"},{"line_number":128,"context_line":"        super(TestMultipleHardwareManagerLoading, self).tearDown()"},{"line_number":129,"context_line":"        self.extension_mgr_patcher.stop()"},{"line_number":130,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_7c2404c7","side":"PARENT","line":127,"in_reply_to":"bff0334d_55e9267e","updated":"2017-04-05 23:08:14.000000000","message":"It is unrelated but it\u0027s a valuable cleanup to use addCleanup instead for several reasons:\n 1. It\u0027s less code than using tearDown\n 2. The cleanup request is now right next to the thing that was created that needs cleaning up, which leaves less room for future error and requires less mental effort to reconcile.\n 3. tearDown is incredibly distasteful. We should be using Fixtures, or addCleanup.\n\nIn fact in future we should probably change the code to just call TestCase.patch() which does it all for you (like I did in the new base class).","commit_id":"4be702242eb48e09eb77e46d997908b7c519127d"}],"ironic_python_agent/tests/unit/test_multi_hardware_clean_steps.py":[{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"1b29b634c5a8a2fe9e11afadd9ff2249efa7a73d","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        self.mocked_extension_mgr.return_value \u003d self.fake_ext_mgr"},{"line_number":78,"context_line":"        hardware._global_managers \u003d None"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def tearDown(self):"},{"line_number":81,"context_line":"        super(TestMultipleHardwareManagerCleanSteps, self).tearDown()"},{"line_number":82,"context_line":"        self.extension_mgr_patcher.stop()"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_95bd6e6a","side":"PARENT","line":80,"updated":"2017-04-05 15:28:45.000000000","message":"Why was this changed? Seems unrelated","commit_id":"4be702242eb48e09eb77e46d997908b7c519127d"},{"author":{"_account_id":15750,"name":"Julian Edwards","email":"bigjools@gmail.com","username":"bigjools"},"change_message_id":"d0541aaecda6d855b66e498bfa0370bc0bc77dbc","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        self.mocked_extension_mgr.return_value \u003d self.fake_ext_mgr"},{"line_number":78,"context_line":"        hardware._global_managers \u003d None"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def tearDown(self):"},{"line_number":81,"context_line":"        super(TestMultipleHardwareManagerCleanSteps, self).tearDown()"},{"line_number":82,"context_line":"        self.extension_mgr_patcher.stop()"},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bff0334d_fc193488","side":"PARENT","line":80,"in_reply_to":"bff0334d_95bd6e6a","updated":"2017-04-05 23:08:14.000000000","message":"Same comment as before. Cleaner code. Much cleaner.","commit_id":"4be702242eb48e09eb77e46d997908b7c519127d"}]}