)]}'
{"ironic/tests/conductor/test_manager.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5d53ba023a98dadbf78ec7eed7dca1d36df117d7","unresolved":false,"context_lines":[{"line_number":1002,"context_line":"            mock_spawn.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY)"},{"line_number":1003,"context_line":""},{"line_number":1004,"context_line":"    @mock.patch(\u0027ironic.drivers.modules.fake.FakeDeploy.deploy\u0027)"},{"line_number":1005,"context_line":"    def test_do_node_deploy_rebuild_active_state(self, mock_deploy):"},{"line_number":1006,"context_line":"        # This tests manager.do_node_deploy(), the \u0027else\u0027 path of"},{"line_number":1007,"context_line":"        # \u0027if new_state \u003d\u003d states.DEPLOYDONE\u0027. The node\u0027s states"},{"line_number":1008,"context_line":"        # aren\u0027t changed in this case."}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_d984b9b2","line":1005,"updated":"2015-01-09 01:38:56.000000000","message":"I changed this back to testing the code mentioned in the comment below. Otherwise it was the same test as test_do_node_deploy_rebuild_active_state_waiting().\n\nAnd anyway, that path should probably be tested, in case we change that code in the future...","commit_id":"9bd0eec3c666ba4313b0fc0678142e169639a6e4"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"05d7f8a0afd4f3a73d5e6fc224ea8257681376f5","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"    def test_do_node_deploy_rebuild_active_state(self, mock_deploy):"},{"line_number":1006,"context_line":"        # This tests manager.do_node_deploy(), the \u0027else\u0027 path of"},{"line_number":1007,"context_line":"        # \u0027if new_state \u003d\u003d states.DEPLOYDONE\u0027. The node\u0027s states"},{"line_number":1008,"context_line":"        # aren\u0027t changed in this case."},{"line_number":1009,"context_line":"        self._start_service()"},{"line_number":1010,"context_line":"        mock_deploy.return_value \u003d states.DEPLOYING"},{"line_number":1011,"context_line":"        node \u003d obj_utils.create_test_node(self.context, driver\u003d\u0027fake\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_49f0e666","line":1008,"updated":"2015-01-09 19:47:47.000000000","message":"The node state *is* changed by the call to task.process_event(\u0027done\u0027) or task.process_event(\u0027wait\u0027).\n\nIf the return value is neither of these, an error is logged.","commit_id":"9bd0eec3c666ba4313b0fc0678142e169639a6e4"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"9487838305d3e50c5894758f1d8b8a4e596d6c95","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"    def test_do_node_deploy_rebuild_active_state(self, mock_deploy):"},{"line_number":1006,"context_line":"        # This tests manager.do_node_deploy(), the \u0027else\u0027 path of"},{"line_number":1007,"context_line":"        # \u0027if new_state \u003d\u003d states.DEPLOYDONE\u0027. The node\u0027s states"},{"line_number":1008,"context_line":"        # aren\u0027t changed in this case."},{"line_number":1009,"context_line":"        self._start_service()"},{"line_number":1010,"context_line":"        mock_deploy.return_value \u003d states.DEPLOYING"},{"line_number":1011,"context_line":"        node \u003d obj_utils.create_test_node(self.context, driver\u003d\u0027fake\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_05c5ba09","line":1008,"in_reply_to":"3a961159_49f0e666","updated":"2015-01-12 16:13:25.000000000","message":"\u003e If the return value is neither of these, an error is logged.\n\nRight, and setting the return value to DEPLOYING is the only way to get to that code path where the error is logged.\n\nIt seems to me that even if no (existing in-tree) driver currently returns DEPLOYING from deploy(), this unit test is meant to test the code in the manager, and the manager has a code path [1] where an error is logged. (Well, that\u0027s my guess as to the intent of this test.)\n\n[1] https://github.com/openstack/ironic/blob/master/ironic/conductor/manager.py#L1392","commit_id":"9bd0eec3c666ba4313b0fc0678142e169639a6e4"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"05d7f8a0afd4f3a73d5e6fc224ea8257681376f5","unresolved":false,"context_lines":[{"line_number":1007,"context_line":"        # \u0027if new_state \u003d\u003d states.DEPLOYDONE\u0027. The node\u0027s states"},{"line_number":1008,"context_line":"        # aren\u0027t changed in this case."},{"line_number":1009,"context_line":"        self._start_service()"},{"line_number":1010,"context_line":"        mock_deploy.return_value \u003d states.DEPLOYING"},{"line_number":1011,"context_line":"        node \u003d obj_utils.create_test_node(self.context, driver\u003d\u0027fake\u0027,"},{"line_number":1012,"context_line":"                                         provision_state\u003dstates.ACTIVE,"},{"line_number":1013,"context_line":"                                         target_provision_state\u003dstates.NOSTATE)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_69e5a2a3","line":1010,"updated":"2015-01-09 19:47:47.000000000","message":"I had changed this to DEPLOYWAIT because there are no driver which return DEPLOYING from deploy()","commit_id":"9bd0eec3c666ba4313b0fc0678142e169639a6e4"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"9487838305d3e50c5894758f1d8b8a4e596d6c95","unresolved":false,"context_lines":[{"line_number":1007,"context_line":"        # \u0027if new_state \u003d\u003d states.DEPLOYDONE\u0027. The node\u0027s states"},{"line_number":1008,"context_line":"        # aren\u0027t changed in this case."},{"line_number":1009,"context_line":"        self._start_service()"},{"line_number":1010,"context_line":"        mock_deploy.return_value \u003d states.DEPLOYING"},{"line_number":1011,"context_line":"        node \u003d obj_utils.create_test_node(self.context, driver\u003d\u0027fake\u0027,"},{"line_number":1012,"context_line":"                                         provision_state\u003dstates.ACTIVE,"},{"line_number":1013,"context_line":"                                         target_provision_state\u003dstates.NOSTATE)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_45aee282","line":1010,"in_reply_to":"3a961159_69e5a2a3","updated":"2015-01-12 16:13:25.000000000","message":"Changing this to DEPLOYWAIT makes this test the same as the one below -- test_do_node_deploy_rebuild_active_state_waiting() \n\nSo we should delete this then?","commit_id":"9bd0eec3c666ba4313b0fc0678142e169639a6e4"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5d53ba023a98dadbf78ec7eed7dca1d36df117d7","unresolved":false,"context_lines":[{"line_number":1218,"context_line":"        self.assertEqual(states.NOSTATE, node.provision_state)"},{"line_number":1219,"context_line":"        # NOTE(deva): this only works because manager.do_node_tear_down()"},{"line_number":1220,"context_line":"        # explicitly sets target_provision_state to NOSTATE. Until we introduce"},{"line_number":1221,"context_line":"        # the AVAILABLE state, this override should remain."},{"line_number":1222,"context_line":"        self.assertEqual(states.NOSTATE, node.target_provision_state)"},{"line_number":1223,"context_line":"        self.assertIsNone(node.last_error)"},{"line_number":1224,"context_line":"        self.assertEqual({}, node.instance_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_39bdbd68","line":1221,"updated":"2015-01-09 01:38:56.000000000","message":"I\u0027m not sure we need this comment because it looks like we don\u0027t even need the workaround, but I suppose as long as the workaround is there, it is ok to leave this in.","commit_id":"9bd0eec3c666ba4313b0fc0678142e169639a6e4"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5d53ba023a98dadbf78ec7eed7dca1d36df117d7","unresolved":false,"context_lines":[{"line_number":2052,"context_line":"        get_nodeinfo_mock.return_value \u003d self._get_nodeinfo_list_response()"},{"line_number":2053,"context_line":"        get_node_mock.return_value \u003d self.node"},{"line_number":2054,"context_line":"        self.node.provision_state \u003d states.DEPLOYWAIT"},{"line_number":2055,"context_line":"        self.node.target_provision_state \u003d states.ACTIVE"},{"line_number":2056,"context_line":"        self.node.save()"},{"line_number":2057,"context_line":""},{"line_number":2058,"context_line":"        self.service._sync_power_states(self.context)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a961159_b9b0cd8b","line":2055,"updated":"2015-01-09 01:38:56.000000000","message":"This should be ACTIVE instead of DEPLOYDONE (because of the change in https://review.openstack.org/#/c/145389/). Similar thing for other tests below.\n\nThe tests pass regardless because nothing is testing for the provision states. I added them in for completeness (and to make it a habit), in case these tests are copy/pasted or changed so that the states do matter.","commit_id":"9bd0eec3c666ba4313b0fc0678142e169639a6e4"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"f7010b42cdf7f25e5f9c942470ac0fa610f322da","unresolved":false,"context_lines":[{"line_number":970,"context_line":"        self._start_service()"},{"line_number":971,"context_line":"        # test when driver.deploy.deploy returns DEPLOYDONE"},{"line_number":972,"context_line":"        mock_deploy.return_value \u003d states.DEPLOYDONE"},{"line_number":973,"context_line":"        node \u003d obj_utils.create_test_node(self.context, driver\u003d\u0027fake\u0027)"},{"line_number":974,"context_line":"        task \u003d task_manager.TaskManager(self.context, node.uuid)"},{"line_number":975,"context_line":"        task.process_event(\u0027deploy\u0027)"},{"line_number":976,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_a450fd5e","line":973,"updated":"2015-01-15 05:05:40.000000000","message":"This method is identical to test__do_node_deploy_ok, except for this line. Does this method test from NOSTATE -\u003e ACTIVE? Can we name it better than just taking on \"_2\", to make it obvious what\u0027s failing if it fails? :)","commit_id":"c888e9eda78ce75ce039db17fff7dc62417f4cd0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5b5b6295cc1073073c3543369ab680b6185b50c1","unresolved":false,"context_lines":[{"line_number":970,"context_line":"        self._start_service()"},{"line_number":971,"context_line":"        # test when driver.deploy.deploy returns DEPLOYDONE"},{"line_number":972,"context_line":"        mock_deploy.return_value \u003d states.DEPLOYDONE"},{"line_number":973,"context_line":"        node \u003d obj_utils.create_test_node(self.context, driver\u003d\u0027fake\u0027)"},{"line_number":974,"context_line":"        task \u003d task_manager.TaskManager(self.context, node.uuid)"},{"line_number":975,"context_line":"        task.process_event(\u0027deploy\u0027)"},{"line_number":976,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_d5e200d3","line":973,"in_reply_to":"3a961159_a450fd5e","updated":"2015-01-15 14:52:39.000000000","message":"Thx for noticing :-)\n\nThe difference between the two is how the node\u0027s states are initialized before manager.do_node_deploy() is called. This test calls task.process_event(\u0027deploy\u0027), whereas the first one explicitly initializes it.\n\nThe reason I had this second version is that I think it is a better test than the original in that for the first, the target state wasn\u0027t set correctly and it caused the test to pass when it should have failed (later corrected in [1]).\n\nWhen I added this 2nd test, I wasn\u0027t actually thinking that we\u0027d have both of them; it was to illustrate the shortcomings of the first. I\u0027m still not sure if we should A. replace tests having-the-first-pattern with the-second-pattern, B. delete this test; C. keep this test (and rename to eg do_node_deploy_ok_via_process_event()). \n\nNote that I went through all the unit tests, and wherever a test had the provision_state set, I added a corresponding setting for target_provision_state, so we shouldn\u0027t encounter this problem. Unless someone adds a new test and doesn\u0027t set both the states, or if we modify the code to eg have another state or something that task.process_event() modifies.\n\nThoughts if you\u0027ve read this far and I was able to explain. ;)\n\n[1] https://review.openstack.org/#/c/145389/","commit_id":"c888e9eda78ce75ce039db17fff7dc62417f4cd0"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"f7010b42cdf7f25e5f9c942470ac0fa610f322da","unresolved":false,"context_lines":[{"line_number":1117,"context_line":"    @mock.patch(\u0027ironic.drivers.modules.fake.FakeDeploy.clean_up\u0027)"},{"line_number":1118,"context_line":"    def test__check_deploy_timeouts(self, mock_cleanup):"},{"line_number":1119,"context_line":"        self._start_service()"},{"line_number":1120,"context_line":"        CONF.set_override(\u0027deploy_callback_timeout\u0027, 1, group\u003d\u0027conductor\u0027)"},{"line_number":1121,"context_line":"        node \u003d obj_utils.create_test_node(self.context, driver\u003d\u0027fake\u0027,"},{"line_number":1122,"context_line":"                provision_state\u003dstates.DEPLOYWAIT,"},{"line_number":1123,"context_line":"                target_provision_state\u003dstates.ACTIVE,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_049551e3","line":1120,"updated":"2015-01-15 05:05:40.000000000","message":"I don\u0027t think we need this line if provision_updated_at was 15 years ago, but I guess it can\u0027t hurt.","commit_id":"c888e9eda78ce75ce039db17fff7dc62417f4cd0"}],"ironic/tests/drivers/ilo/test_deploy.py":[{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"b5d43687eff70be2d253d8c626a86eb3c30fcbcb","unresolved":false,"context_lines":[{"line_number":516,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"},{"line_number":517,"context_line":"                                  shared\u003dFalse) as task:"},{"line_number":518,"context_line":"            task.node.provision_state \u003d states.DEPLOYWAIT"},{"line_number":519,"context_line":"            task.node.target_provision_state \u003d states.ACTIVE"},{"line_number":520,"context_line":"            task.driver.vendor._continue_deploy(task, **kwargs)"},{"line_number":521,"context_line":"            set_boot_device_mock.assert_called_with(task, boot_devices.PXE,"},{"line_number":522,"context_line":"                                                    True)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_062460c4","line":519,"updated":"2015-01-15 16:09:27.000000000","message":"These values don\u0027t actually seem to matter to the test since _continue_deploy() is mocked. Are these just for completeness?","commit_id":"c888e9eda78ce75ce039db17fff7dc62417f4cd0"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"d969bea46d4e5d7c85457613e8718f55b5527715","unresolved":false,"context_lines":[{"line_number":516,"context_line":"        with task_manager.acquire(self.context, self.node.uuid,"},{"line_number":517,"context_line":"                                  shared\u003dFalse) as task:"},{"line_number":518,"context_line":"            task.node.provision_state \u003d states.DEPLOYWAIT"},{"line_number":519,"context_line":"            task.node.target_provision_state \u003d states.ACTIVE"},{"line_number":520,"context_line":"            task.driver.vendor._continue_deploy(task, **kwargs)"},{"line_number":521,"context_line":"            set_boot_device_mock.assert_called_with(task, boot_devices.PXE,"},{"line_number":522,"context_line":"                                                    True)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_0cae41e7","line":519,"in_reply_to":"3a961159_062460c4","updated":"2015-01-15 16:34:48.000000000","message":"Yeah, I tried to go through all the unit tests, setting the states so just-in-case people copy/paste/change, they will be there so we don\u0027t forget to add them in case they really are needed.\n\nIt\u0027s hard to know whether it is easier to understand with or without, if it isn\u0027t actually being used. (eg, someone may later come by and add a patch to delete this cuz it doesn\u0027t seem to matter...)","commit_id":"c888e9eda78ce75ce039db17fff7dc62417f4cd0"}]}