)]}'
{"nova/tests/unit/virt/test_firewall.py":[{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"8a21625e49837ffed271c58f2b5aad97d305c13c","unresolved":false,"context_lines":[{"line_number":19,"context_line":"_IPT_DRIVER_CLS \u003d firewall.IptablesFirewallDriver"},{"line_number":20,"context_line":"_FN_REF_PROV_RULES \u003d \u0027refresh_provider_fw_rules\u0027"},{"line_number":21,"context_line":"_FN_INS_RULES \u003d \u0027instance_rules\u0027"},{"line_number":22,"context_line":"_FN_ADD_FILTERS_INS \u003d \u0027add_filters_for_instance\u0027"},{"line_number":23,"context_line":"_FN_DO_BASIC_RULES \u003d \u0027_do_basic_rules\u0027"},{"line_number":24,"context_line":"_FN_DO_DHCP_RULES \u003d \u0027_do_dhcp_rules\u0027"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_4e721d7e","line":22,"updated":"2015-11-23 14:16:26.000000000","message":"nit: these aren\u0027t the most obvious abbreviations to me... I\u0027d go with _FN_REFRESH_FW, _FN_INSTANCE_RULES and _FN_ADD_FILTERS.","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"29543631930db488088dc7f7462a898d0f9a7387","unresolved":false,"context_lines":[{"line_number":19,"context_line":"_IPT_DRIVER_CLS \u003d firewall.IptablesFirewallDriver"},{"line_number":20,"context_line":"_FN_REF_PROV_RULES \u003d \u0027refresh_provider_fw_rules\u0027"},{"line_number":21,"context_line":"_FN_INS_RULES \u003d \u0027instance_rules\u0027"},{"line_number":22,"context_line":"_FN_ADD_FILTERS_INS \u003d \u0027add_filters_for_instance\u0027"},{"line_number":23,"context_line":"_FN_DO_BASIC_RULES \u003d \u0027_do_basic_rules\u0027"},{"line_number":24,"context_line":"_FN_DO_DHCP_RULES \u003d \u0027_do_dhcp_rules\u0027"},{"line_number":25,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_305832fa","line":22,"in_reply_to":"ba8a016a_4e721d7e","updated":"2015-11-24 01:56:41.000000000","message":"Done","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"8a21625e49837ffed271c58f2b5aad97d305c13c","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    @mock.patch(\u0027nova.network.linux_net.get_iptables_manager\u0027)"},{"line_number":30,"context_line":"    def test_constructor(self, iptm_mock):"},{"line_number":31,"context_line":"        ipt_mock \u003d mock.MagicMock()"},{"line_number":32,"context_line":"        iptm_mock.return_value \u003d ipt_mock"},{"line_number":33,"context_line":"        vapi \u003d mock.MagicMock()"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"        driver \u003d firewall.IptablesFirewallDriver(vapi)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_0ef175cd","line":32,"updated":"2015-11-23 14:16:26.000000000","message":"You can probably just do:\n\n  ipt_mock \u003d iptm_mock()","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"29543631930db488088dc7f7462a898d0f9a7387","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    @mock.patch(\u0027nova.network.linux_net.get_iptables_manager\u0027)"},{"line_number":30,"context_line":"    def test_constructor(self, iptm_mock):"},{"line_number":31,"context_line":"        ipt_mock \u003d mock.MagicMock()"},{"line_number":32,"context_line":"        iptm_mock.return_value \u003d ipt_mock"},{"line_number":33,"context_line":"        vapi \u003d mock.MagicMock()"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"        driver \u003d firewall.IptablesFirewallDriver(vapi)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_30dd9242","line":32,"in_reply_to":"ba8a016a_0ef175cd","updated":"2015-11-24 01:56:41.000000000","message":"Done","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"8a21625e49837ffed271c58f2b5aad97d305c13c","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"        driver \u003d firewall.IptablesFirewallDriver(vapi)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"        self.assertEqual(driver.basically_filtered, False)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"        # NOTE(jaypipes): Here we are not testing the IptablesManager"},{"line_number":40,"context_line":"        # constructor. We are only testing the calls made against the"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_eea3d1af","line":37,"updated":"2015-11-23 14:16:26.000000000","message":"nit: line 33 seems more related to lines 35+37, blank lines should reflect that","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"29543631930db488088dc7f7462a898d0f9a7387","unresolved":false,"context_lines":[{"line_number":34,"context_line":""},{"line_number":35,"context_line":"        driver \u003d firewall.IptablesFirewallDriver(vapi)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"        self.assertEqual(driver.basically_filtered, False)"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"        # NOTE(jaypipes): Here we are not testing the IptablesManager"},{"line_number":40,"context_line":"        # constructor. We are only testing the calls made against the"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_908b1e1e","line":37,"in_reply_to":"ba8a016a_eea3d1af","updated":"2015-11-24 01:56:41.000000000","message":"Done","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"8a21625e49837ffed271c58f2b5aad97d305c13c","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                                     add_filters_mock, refresh_mock):"},{"line_number":56,"context_line":"        ipt_mock \u003d mock.MagicMock()"},{"line_number":57,"context_line":"        iptm_mock.return_value \u003d ipt_mock"},{"line_number":58,"context_line":"        vapi \u003d mock.MagicMock()"},{"line_number":59,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":60,"context_line":"        i_mock.id \u003d \u0027fake_id\u0027"},{"line_number":61,"context_line":"        i_rules_mock.return_value \u003d (mock.sentinel.v4_rules,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_6ef361a0","line":58,"updated":"2015-11-23 14:16:26.000000000","message":"put declarations next to usages (L64) where possible","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"29543631930db488088dc7f7462a898d0f9a7387","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                                     add_filters_mock, refresh_mock):"},{"line_number":56,"context_line":"        ipt_mock \u003d mock.MagicMock()"},{"line_number":57,"context_line":"        iptm_mock.return_value \u003d ipt_mock"},{"line_number":58,"context_line":"        vapi \u003d mock.MagicMock()"},{"line_number":59,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":60,"context_line":"        i_mock.id \u003d \u0027fake_id\u0027"},{"line_number":61,"context_line":"        i_rules_mock.return_value \u003d (mock.sentinel.v4_rules,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_ebc4ff92","line":58,"in_reply_to":"ba8a016a_6ef361a0","updated":"2015-11-24 01:56:41.000000000","message":"Done","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"8a21625e49837ffed271c58f2b5aad97d305c13c","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        driver \u003d firewall.IptablesFirewallDriver(vapi)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        ipt_mock.ipv4.__getitem__.reset_mock()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_ee7bd1f2","line":66,"updated":"2015-11-23 14:16:26.000000000","message":"the mock shouldn\u0027t have been called yet, why reset?","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"29543631930db488088dc7f7462a898d0f9a7387","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        driver \u003d firewall.IptablesFirewallDriver(vapi)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"        ipt_mock.ipv4.__getitem__.reset_mock()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_b020a2f8","line":66,"in_reply_to":"ba8a016a_ee7bd1f2","updated":"2015-11-24 01:56:41.000000000","message":"It has been called at  https://github.com/openstack/nova/blob/master/nova/virt/firewall.py#L142-L143","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fce17e36d6072f057f1d80f7feffe21d47f5d10a","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        ipt_mock.ipv4.__getitem__.reset_mock()"},{"line_number":82,"context_line":"        driver.dhcp_create \u003d True"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        expected \u003d ["},{"line_number":87,"context_line":"            mock.call.add_rule("}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_c47b497a","line":84,"updated":"2015-11-23 15:40:21.000000000","message":"it seems to me that there are two test cases hidden in this function. One is testing with \ndriver.dhcp_create \u003d False and then with driver.dhcp_create \u003d True lets split this test to two.","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"29543631930db488088dc7f7462a898d0f9a7387","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        ipt_mock.ipv4.__getitem__.reset_mock()"},{"line_number":82,"context_line":"        driver.dhcp_create \u003d True"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"        expected \u003d ["},{"line_number":87,"context_line":"            mock.call.add_rule("}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_b0818291","line":84,"in_reply_to":"ba8a016a_c47b497a","updated":"2015-11-24 01:56:41.000000000","message":"Done","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"fce17e36d6072f057f1d80f7feffe21d47f5d10a","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        # set when prepare_instance_filters() first creates them)"},{"line_number":101,"context_line":"        ipt_mock.ipv4.__getitem__.reset_mock()"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"},{"line_number":104,"context_line":"        ipt_mock.ipv4.__getitem__.return_value.assert_not_called()"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    @mock.patch(\u0027nova.objects.SecurityGroupList\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_84954141","line":103,"updated":"2015-11-23 15:40:21.000000000","message":"Or even a third test case","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"29543631930db488088dc7f7462a898d0f9a7387","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        # set when prepare_instance_filters() first creates them)"},{"line_number":101,"context_line":"        ipt_mock.ipv4.__getitem__.reset_mock()"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"},{"line_number":104,"context_line":"        ipt_mock.ipv4.__getitem__.return_value.assert_not_called()"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    @mock.patch(\u0027nova.objects.SecurityGroupList\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_d07c46b7","line":103,"in_reply_to":"ba8a016a_84954141","updated":"2015-11-24 01:56:41.000000000","message":"Done","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"8a21625e49837ffed271c58f2b5aad97d305c13c","unresolved":false,"context_lines":[{"line_number":112,"context_line":"    @mock.patch(\u0027nova.network.linux_net.get_iptables_manager\u0027)"},{"line_number":113,"context_line":"    def test_instance_rules_no_secgroups(self, _iptm_mock, ctx_mock,"},{"line_number":114,"context_line":"            ins_obj_cls_mock, _do_basic_mock, _do_dhcp_mock,"},{"line_number":115,"context_line":"            sec_grp_list_mock):"},{"line_number":116,"context_line":"        # Simple unit test that verifies that the fallback jump"},{"line_number":117,"context_line":"        # is the only rule added to the returned list of rules if"},{"line_number":118,"context_line":"        # no secgroups are found (we ignore the basic and DHCP"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_6984bbb0","line":115,"updated":"2015-11-23 14:16:26.000000000","message":"You don\u0027t need _iptm_mock here, I tested. Please remove and check for other mocks you don\u0027t need (throughout the class).","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"29543631930db488088dc7f7462a898d0f9a7387","unresolved":false,"context_lines":[{"line_number":112,"context_line":"    @mock.patch(\u0027nova.network.linux_net.get_iptables_manager\u0027)"},{"line_number":113,"context_line":"    def test_instance_rules_no_secgroups(self, _iptm_mock, ctx_mock,"},{"line_number":114,"context_line":"            ins_obj_cls_mock, _do_basic_mock, _do_dhcp_mock,"},{"line_number":115,"context_line":"            sec_grp_list_mock):"},{"line_number":116,"context_line":"        # Simple unit test that verifies that the fallback jump"},{"line_number":117,"context_line":"        # is the only rule added to the returned list of rules if"},{"line_number":118,"context_line":"        # no secgroups are found (we ignore the basic and DHCP"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba8a016a_7031da0a","line":115,"in_reply_to":"ba8a016a_6984bbb0","updated":"2015-11-24 01:56:41.000000000","message":"yes, if remove _iptm_mock, the test would still pass.\n\nBut, it passed only because it do call the  get_iptables_manager method in nova.network.linux_net.\n\nI think as for unit test, we should always avoid to call methods outside current module. So,  I would like to leave it here.","commit_id":"ca8cf7ff8372a55da7b8bafd183cbf6dda6573f2"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a23d3d3a7b8cc9569cf08e35c7ce8717f6ac9478","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":88,"context_line":"        i_mock.id \u003d \u0027fake_id\u0027"},{"line_number":89,"context_line":"        i_rules_mock.return_value \u003d (mock.sentinel.v4_rules,"},{"line_number":90,"context_line":"                                     mock.sentinel.v6_rules)"},{"line_number":91,"context_line":"        # add rules when DHCP create is set"},{"line_number":92,"context_line":"        driver.dhcp_create \u003d True"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba8a016a_d45c2c1b","line":90,"updated":"2015-11-24 09:22:19.000000000","message":"It seems that L82-L90 is the same as L53-L61 so it can be moved to a common place.","commit_id":"f20f747d2daf88e0ab469187075d925b8bf64bb6"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"a6bbec3a2947cd5ce6a67097457ef74c13f23ed4","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":88,"context_line":"        i_mock.id \u003d \u0027fake_id\u0027"},{"line_number":89,"context_line":"        i_rules_mock.return_value \u003d (mock.sentinel.v4_rules,"},{"line_number":90,"context_line":"                                     mock.sentinel.v6_rules)"},{"line_number":91,"context_line":"        # add rules when DHCP create is set"},{"line_number":92,"context_line":"        driver.dhcp_create \u003d True"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_1d746d33","line":90,"in_reply_to":"ba8a016a_d45c2c1b","updated":"2015-11-25 03:16:49.000000000","message":"Done","commit_id":"f20f747d2daf88e0ab469187075d925b8bf64bb6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a23d3d3a7b8cc9569cf08e35c7ce8717f6ac9478","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":120,"context_line":"        i_mock.id \u003d \u0027fake_id\u0027"},{"line_number":121,"context_line":"        i_rules_mock.return_value \u003d (mock.sentinel.v4_rules,"},{"line_number":122,"context_line":"                                     mock.sentinel.v6_rules)"},{"line_number":123,"context_line":"        # add rules when DHCP create is set"},{"line_number":124,"context_line":"        driver.dhcp_create \u003d True"},{"line_number":125,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba8a016a_74ea2011","line":122,"updated":"2015-11-24 09:22:19.000000000","message":"It seems that L114-L122 is the same as L52-L61","commit_id":"f20f747d2daf88e0ab469187075d925b8bf64bb6"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"a6bbec3a2947cd5ce6a67097457ef74c13f23ed4","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":120,"context_line":"        i_mock.id \u003d \u0027fake_id\u0027"},{"line_number":121,"context_line":"        i_rules_mock.return_value \u003d (mock.sentinel.v4_rules,"},{"line_number":122,"context_line":"                                     mock.sentinel.v6_rules)"},{"line_number":123,"context_line":"        # add rules when DHCP create is set"},{"line_number":124,"context_line":"        driver.dhcp_create \u003d True"},{"line_number":125,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_bd8f6122","line":122,"in_reply_to":"ba8a016a_74ea2011","updated":"2015-11-25 03:16:49.000000000","message":"Done","commit_id":"f20f747d2daf88e0ab469187075d925b8bf64bb6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a23d3d3a7b8cc9569cf08e35c7ce8717f6ac9478","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":188,"context_line":"        i_obj_mock \u003d mock.MagicMock()"},{"line_number":189,"context_line":"        ni_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":190,"context_line":"        ins_obj_cls_mock._from_db_object.return_value \u003d i_obj_mock"},{"line_number":191,"context_line":"        sec_grp_list_mock.get_by_instance.return_value \u003d ["},{"line_number":192,"context_line":"            mock.sentinel.sec_grp"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba8a016a_b40088ac","line":189,"updated":"2015-11-24 09:22:19.000000000","message":"L185-L189 seems the same as L149-154 also some other parts like initializing v4_rules and v6_rules and initializin ins_obj_cls_mock seems the same as the previous test so I feel some of these duplication can be moved to a generic function","commit_id":"f20f747d2daf88e0ab469187075d925b8bf64bb6"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"a6bbec3a2947cd5ce6a67097457ef74c13f23ed4","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":188,"context_line":"        i_obj_mock \u003d mock.MagicMock()"},{"line_number":189,"context_line":"        ni_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":190,"context_line":"        ins_obj_cls_mock._from_db_object.return_value \u003d i_obj_mock"},{"line_number":191,"context_line":"        sec_grp_list_mock.get_by_instance.return_value \u003d ["},{"line_number":192,"context_line":"            mock.sentinel.sec_grp"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_9d9e1d55","line":189,"in_reply_to":"ba8a016a_b40088ac","updated":"2015-11-25 03:16:49.000000000","message":"Done","commit_id":"f20f747d2daf88e0ab469187075d925b8bf64bb6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a23d3d3a7b8cc9569cf08e35c7ce8717f6ac9478","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":243,"context_line":"        i_obj_mock \u003d mock.MagicMock()"},{"line_number":244,"context_line":"        ni_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":245,"context_line":"        ins_obj_cls_mock._from_db_object.return_value \u003d i_obj_mock"},{"line_number":246,"context_line":"        sec_grp_list_mock.get_by_instance.return_value \u003d ["},{"line_number":247,"context_line":"            mock.sentinel.sec_grp"},{"line_number":248,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba8a016a_b49c086b","line":245,"updated":"2015-11-24 09:22:19.000000000","message":"some of this init code is the same as in the previous test so I feel this duplication can be removed","commit_id":"f20f747d2daf88e0ab469187075d925b8bf64bb6"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"a6bbec3a2947cd5ce6a67097457ef74c13f23ed4","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":243,"context_line":"        i_obj_mock \u003d mock.MagicMock()"},{"line_number":244,"context_line":"        ni_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":245,"context_line":"        ins_obj_cls_mock._from_db_object.return_value \u003d i_obj_mock"},{"line_number":246,"context_line":"        sec_grp_list_mock.get_by_instance.return_value \u003d ["},{"line_number":247,"context_line":"            mock.sentinel.sec_grp"},{"line_number":248,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_3da4d1a4","line":245,"in_reply_to":"ba8a016a_b49c086b","updated":"2015-11-25 03:16:49.000000000","message":"Done","commit_id":"f20f747d2daf88e0ab469187075d925b8bf64bb6"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"a23d3d3a7b8cc9569cf08e35c7ce8717f6ac9478","unresolved":false,"context_lines":[{"line_number":285,"context_line":"        # Emulate one of the instances in the grantee group being deleted"},{"line_number":286,"context_line":"        # in between when the spawn of this instance and when we set up"},{"line_number":287,"context_line":"        # network for that instance, and ensure that we do not crash and"},{"line_number":288,"context_line":"        # burn but just skip the deleted instance from the iptables filters"},{"line_number":289,"context_line":"        get_nw_info_mock.side_effect \u003d exception.InstanceNotFound("},{"line_number":290,"context_line":"                instance_id\u003d\"_ignored\")"},{"line_number":291,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ba8a016a_549b5c66","line":288,"updated":"2015-11-24 09:22:19.000000000","message":"Can it be a separate test one for the happy case when instances are exists, and one for the edge case when one of the instance is deleted in the meantime?","commit_id":"f20f747d2daf88e0ab469187075d925b8bf64bb6"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"a6bbec3a2947cd5ce6a67097457ef74c13f23ed4","unresolved":false,"context_lines":[{"line_number":285,"context_line":"        # Emulate one of the instances in the grantee group being deleted"},{"line_number":286,"context_line":"        # in between when the spawn of this instance and when we set up"},{"line_number":287,"context_line":"        # network for that instance, and ensure that we do not crash and"},{"line_number":288,"context_line":"        # burn but just skip the deleted instance from the iptables filters"},{"line_number":289,"context_line":"        get_nw_info_mock.side_effect \u003d exception.InstanceNotFound("},{"line_number":290,"context_line":"                instance_id\u003d\"_ignored\")"},{"line_number":291,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9a8ffd7b_c0ffdeff","line":288,"in_reply_to":"ba8a016a_549b5c66","updated":"2015-11-25 03:16:49.000000000","message":"Done","commit_id":"f20f747d2daf88e0ab469187075d925b8bf64bb6"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"58399dd7c39ecff12b6de6175ec6583682ad40b3","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def setup_instance_filter(self, iptm_mock, i_rules_mock):"},{"line_number":53,"context_line":"        ipt_mock, driver \u003d self.setup_driver(iptm_mock)"},{"line_number":54,"context_line":"        ipt_mock.ipv4.__getitem__.reset_mock()"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":57,"context_line":"        i_mock.id \u003d \u0027fake_id\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_4a30db01","line":54,"updated":"2015-11-25 11:30:09.000000000","message":"nit: could add a comment like \"The IptablesFirewallDriver init method calls the iptables manager so we must reset here\"","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"cbfe9024055a4ccf53d4e622980b18910936cada","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def setup_instance_filter(self, iptm_mock, i_rules_mock):"},{"line_number":53,"context_line":"        ipt_mock, driver \u003d self.setup_driver(iptm_mock)"},{"line_number":54,"context_line":"        ipt_mock.ipv4.__getitem__.reset_mock()"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":57,"context_line":"        i_mock.id \u003d \u0027fake_id\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_8f1e155e","line":54,"in_reply_to":"9a8ffd7b_4a30db01","updated":"2015-11-30 13:53:03.000000000","message":"Done","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"58399dd7c39ecff12b6de6175ec6583682ad40b3","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    def test_prepare_instance_filter(self, iptm_mock, i_rules_mock,"},{"line_number":67,"context_line":"                                     add_filters_mock, refresh_mock):"},{"line_number":68,"context_line":"        ipt_mock, i_mock, driver \u003d self.setup_instance_filter("},{"line_number":69,"context_line":"            iptm_mock, i_rules_mock)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_8d3b3dd1","line":69,"updated":"2015-11-25 11:30:09.000000000","message":"nit: 8-indent continuation lines (or visual indent)","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"cbfe9024055a4ccf53d4e622980b18910936cada","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    def test_prepare_instance_filter(self, iptm_mock, i_rules_mock,"},{"line_number":67,"context_line":"                                     add_filters_mock, refresh_mock):"},{"line_number":68,"context_line":"        ipt_mock, i_mock, driver \u003d self.setup_instance_filter("},{"line_number":69,"context_line":"            iptm_mock, i_rules_mock)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_cfb41dd6","line":69,"in_reply_to":"9a8ffd7b_8d3b3dd1","updated":"2015-11-30 13:53:03.000000000","message":"Done","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"58399dd7c39ecff12b6de6175ec6583682ad40b3","unresolved":false,"context_lines":[{"line_number":84,"context_line":"    @mock.patch.object(_IPT_DRIVER_CLS, _FN_INSTANCE_RULES)"},{"line_number":85,"context_line":"    @mock.patch(\u0027nova.network.linux_net.get_iptables_manager\u0027)"},{"line_number":86,"context_line":"    def test_prepare_instance_filter_with_dhcp_create("},{"line_number":87,"context_line":"        self, iptm_mock, i_rules_mock, add_filters_mock, refresh_mock):"},{"line_number":88,"context_line":"        ipt_mock, i_mock, driver \u003d self.setup_instance_filter("},{"line_number":89,"context_line":"            iptm_mock, i_rules_mock)"},{"line_number":90,"context_line":"        # add rules when DHCP create is set"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_4dba3525","line":87,"updated":"2015-11-25 11:30:09.000000000","message":"nit: this should be 8-indented to distinguish it from the next line","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"cbfe9024055a4ccf53d4e622980b18910936cada","unresolved":false,"context_lines":[{"line_number":84,"context_line":"    @mock.patch.object(_IPT_DRIVER_CLS, _FN_INSTANCE_RULES)"},{"line_number":85,"context_line":"    @mock.patch(\u0027nova.network.linux_net.get_iptables_manager\u0027)"},{"line_number":86,"context_line":"    def test_prepare_instance_filter_with_dhcp_create("},{"line_number":87,"context_line":"        self, iptm_mock, i_rules_mock, add_filters_mock, refresh_mock):"},{"line_number":88,"context_line":"        ipt_mock, i_mock, driver \u003d self.setup_instance_filter("},{"line_number":89,"context_line":"            iptm_mock, i_rules_mock)"},{"line_number":90,"context_line":"        # add rules when DHCP create is set"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_0fc3053a","line":87,"in_reply_to":"9a8ffd7b_4dba3525","updated":"2015-11-30 13:53:03.000000000","message":"Done","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"58399dd7c39ecff12b6de6175ec6583682ad40b3","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    @mock.patch.object(_IPT_DRIVER_CLS, _FN_INSTANCE_RULES)"},{"line_number":110,"context_line":"    @mock.patch(\u0027nova.network.linux_net.get_iptables_manager\u0027)"},{"line_number":111,"context_line":"    def test_prepare_instance_filter_recreate(self, iptm_mock, i_rules_mock,"},{"line_number":112,"context_line":"                                     add_filters_mock, refresh_mock):"},{"line_number":113,"context_line":"        ipt_mock, i_mock, driver \u003d self.setup_instance_filter("},{"line_number":114,"context_line":"            iptm_mock, i_rules_mock)"},{"line_number":115,"context_line":"        # add rules when DHCP create is set and create the rule"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_4d91d598","line":112,"updated":"2015-11-25 11:30:09.000000000","message":"nit: 8-indent","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"cbfe9024055a4ccf53d4e622980b18910936cada","unresolved":false,"context_lines":[{"line_number":109,"context_line":"    @mock.patch.object(_IPT_DRIVER_CLS, _FN_INSTANCE_RULES)"},{"line_number":110,"context_line":"    @mock.patch(\u0027nova.network.linux_net.get_iptables_manager\u0027)"},{"line_number":111,"context_line":"    def test_prepare_instance_filter_recreate(self, iptm_mock, i_rules_mock,"},{"line_number":112,"context_line":"                                     add_filters_mock, refresh_mock):"},{"line_number":113,"context_line":"        ipt_mock, i_mock, driver \u003d self.setup_instance_filter("},{"line_number":114,"context_line":"            iptm_mock, i_rules_mock)"},{"line_number":115,"context_line":"        # add rules when DHCP create is set and create the rule"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_2fd7e9ee","line":112,"in_reply_to":"9a8ffd7b_4d91d598","updated":"2015-11-30 13:53:03.000000000","message":"Done","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"58399dd7c39ecff12b6de6175ec6583682ad40b3","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"},{"line_number":124,"context_line":"        ipt_mock.ipv4.__getitem__.return_value.assert_not_called()"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def setup_instance_rules(self, ins_obj_cls_mock):"},{"line_number":127,"context_line":"        vapi \u003d mock.MagicMock()"},{"line_number":128,"context_line":"        driver \u003d firewall.IptablesFirewallDriver(vapi)"},{"line_number":129,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_ed44491c","line":126,"updated":"2015-11-25 11:30:09.000000000","message":"nit: could we have a docstring saying what these mocks refer to please?","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"cbfe9024055a4ccf53d4e622980b18910936cada","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        driver.prepare_instance_filter(i_mock, mock.sentinel.net_info)"},{"line_number":124,"context_line":"        ipt_mock.ipv4.__getitem__.return_value.assert_not_called()"},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"    def setup_instance_rules(self, ins_obj_cls_mock):"},{"line_number":127,"context_line":"        vapi \u003d mock.MagicMock()"},{"line_number":128,"context_line":"        driver \u003d firewall.IptablesFirewallDriver(vapi)"},{"line_number":129,"context_line":"        i_mock \u003d mock.MagicMock(spec\u003ddict)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_d2160258","line":126,"in_reply_to":"9a8ffd7b_ed44491c","updated":"2015-11-30 13:53:03.000000000","message":"Done","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"58399dd7c39ecff12b6de6175ec6583682ad40b3","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        i_obj_mock \u003d mock.MagicMock()"},{"line_number":131,"context_line":"        ni_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":132,"context_line":"        ins_obj_cls_mock._from_db_object.return_value \u003d i_obj_mock"},{"line_number":133,"context_line":"        return i_mock, i_obj_mock, ni_mock, driver"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    @mock.patch(\u0027nova.objects.SecurityGroupList\u0027)"},{"line_number":136,"context_line":"    @mock.patch.object(_IPT_DRIVER_CLS, _FN_DO_DHCP_RULES)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_cd5865fd","line":133,"updated":"2015-11-25 11:30:09.000000000","message":"nit: order lines 127-132 according to the return order","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"cbfe9024055a4ccf53d4e622980b18910936cada","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        i_obj_mock \u003d mock.MagicMock()"},{"line_number":131,"context_line":"        ni_mock \u003d mock.MagicMock(spec\u003ddict)"},{"line_number":132,"context_line":"        ins_obj_cls_mock._from_db_object.return_value \u003d i_obj_mock"},{"line_number":133,"context_line":"        return i_mock, i_obj_mock, ni_mock, driver"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    @mock.patch(\u0027nova.objects.SecurityGroupList\u0027)"},{"line_number":136,"context_line":"    @mock.patch.object(_IPT_DRIVER_CLS, _FN_DO_DHCP_RULES)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_5512fc59","line":133,"in_reply_to":"9a8ffd7b_cd5865fd","updated":"2015-11-30 13:53:03.000000000","message":"Done","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"58399dd7c39ecff12b6de6175ec6583682ad40b3","unresolved":false,"context_lines":[{"line_number":277,"context_line":"        sec_grp_list_mock.get_by_instance.assert_called_once_with("},{"line_number":278,"context_line":"                mock.sentinel.ctx, i_obj_mock)"},{"line_number":279,"context_line":"        sec_grp_rule_list_mock.get_by_security_group.assert_called_once_with("},{"line_number":280,"context_line":"                mock.sentinel.ctx, mock.sentinel.sec_grp)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"        expected \u003d [\u0027-j $sg-fallback\u0027]"},{"line_number":283,"context_line":"        self.assertEqual(expected, v4_rules)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_4ddc7512","line":280,"updated":"2015-11-25 11:30:09.000000000","message":"seen these six lines a few times now, another helper candidate?","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":6116,"name":"li,chen","email":"shchenli@cn.ibm.com","username":"chen-li"},"change_message_id":"cbfe9024055a4ccf53d4e622980b18910936cada","unresolved":false,"context_lines":[{"line_number":277,"context_line":"        sec_grp_list_mock.get_by_instance.assert_called_once_with("},{"line_number":278,"context_line":"                mock.sentinel.ctx, i_obj_mock)"},{"line_number":279,"context_line":"        sec_grp_rule_list_mock.get_by_security_group.assert_called_once_with("},{"line_number":280,"context_line":"                mock.sentinel.ctx, mock.sentinel.sec_grp)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"        expected \u003d [\u0027-j $sg-fallback\u0027]"},{"line_number":283,"context_line":"        self.assertEqual(expected, v4_rules)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a8ffd7b_58cd5185","line":280,"in_reply_to":"9a8ffd7b_4ddc7512","updated":"2015-11-30 13:53:03.000000000","message":"actually, they have been called several times are checking exactly the same thing. So, I just removed the duplicated check.","commit_id":"8768510e0bc309fab564b2e7221590f755248398"},{"author":{"_account_id":17185,"name":"Diego Adolfo","email":"diegoado@gmail.com","username":"diegoado"},"change_message_id":"7620bfa6b6bbeb2edd44744bb40bfe099d53fb64","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        ]"},{"line_number":108,"context_line":"        ipt_mock.ipv4.__getitem__.return_value.assert_has_calls(expected)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    @mock.patch.object(_IPT_DRIVER_CLS, _FN_REFRESH_FW)"},{"line_number":111,"context_line":"    @mock.patch.object(_IPT_DRIVER_CLS, _FN_ADD_FILTERS)"},{"line_number":112,"context_line":"    @mock.patch.object(_IPT_DRIVER_CLS, _FN_INSTANCE_RULES)"},{"line_number":113,"context_line":"    @mock.patch(\u0027nova.network.linux_net.get_iptables_manager\u0027)"},{"line_number":114,"context_line":"    def test_prepare_instance_filter_recreate(self, iptm_mock, i_rules_mock,"},{"line_number":115,"context_line":"            add_filters_mock, refresh_mock):"},{"line_number":116,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7af24918_0bb7293a","line":113,"range":{"start_line":110,"start_character":2,"end_line":113,"end_character":62},"updated":"2016-03-01 18:31:06.000000000","message":"These signatures \"mock.patch ()\" are really needed in so many different methods, one would be better to create a setup for your class test or a configuration file because it is leaving its dirty code.","commit_id":"7d0ddeffbd9c84c704027653586c02cdaf90660e"}]}