)]}'
{"nova/tests/unit/virt/test_firewall.py":[{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"abc4f2893a49f0929dac914f5ab909b79033bdbf","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    def setUp(self, iptm_mock):"},{"line_number":31,"context_line":"        super(TestIptablesFirewallDriver, self).setUp()"},{"line_number":32,"context_line":"        self.driver \u003d firewall.IptablesFirewallDriver()"},{"line_number":33,"context_line":"        self.ipt_mock \u003d iptm_mock()"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @mock.patch(\u0027nova.network.linux_net.get_iptables_manager\u0027)"},{"line_number":36,"context_line":"    def test_constructor(self, iptm_mock):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_f6c85424","line":33,"updated":"2016-05-20 15:22:24.000000000","message":"What\u0027s going on here? You\u0027ve mocked out a function, and are calling this mocked function to set self.ipt_mock? This essentially results in self.ipt_mock being nothing or a MagicMock(). If I\u0027m interpreting this correctly, this isn\u0027t what you want to do. If this get_iptables_manager needs to be mocked in order for self.driver to be initiated correctly, you don\u0027t need this last line.","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"abc4f2893a49f0929dac914f5ab909b79033bdbf","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    @mock.patch(\u0027nova.network.linux_net.get_iptables_manager\u0027)"},{"line_number":36,"context_line":"    def test_constructor(self, iptm_mock):"},{"line_number":37,"context_line":"        self.assertEqual(self.driver.dhcp_create, False)"},{"line_number":38,"context_line":"        self.assertEqual(self.driver.dhcp_created, False)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        # NOTE(jaypipes): Here we are not testing the IptablesManager"},{"line_number":41,"context_line":"        # constructor. We are only testing the calls made against the"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_eb05117a","line":38,"updated":"2016-05-20 15:22:24.000000000","message":"assertEquals follow the convention assertEqual(expected, actual) so the order of these arguments should be reversed.","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dc0ed2f81657d0a7ff0e1bfe70b05d1c2928057f","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        pass"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def test_apply_instance_filter(self):"},{"line_number":57,"context_line":"        pass"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_filter_defer_apply_on(self):"},{"line_number":60,"context_line":"        self.driver.iptables.defer_apply_on \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_67a05b44","line":57,"updated":"2016-05-19 22:01:10.000000000","message":"Please remove empty tests.","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":20411,"name":"Julian","email":"julian.sy@att.com","username":"syjulian"},"change_message_id":"9466d2004d0a1237889217e7eea8f8032a1fb233","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        pass"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def test_apply_instance_filter(self):"},{"line_number":57,"context_line":"        pass"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def test_filter_defer_apply_on(self):"},{"line_number":60,"context_line":"        self.driver.iptables.defer_apply_on \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_e5c8a024","line":57,"in_reply_to":"bab6814e_67a05b44","updated":"2016-05-20 14:23:19.000000000","message":"WIll do","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dc0ed2f81657d0a7ff0e1bfe70b05d1c2928057f","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    def test_filter_defer_apply_on(self):"},{"line_number":60,"context_line":"        self.driver.iptables.defer_apply_on \u003d mock.Mock()"},{"line_number":61,"context_line":"        self.driver.filter_defer_apply_on()"},{"line_number":62,"context_line":"        self.driver.iptables.defer_apply_on.assert_called_once()"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def test_filter_defer_apply_off(self):"},{"line_number":65,"context_line":"        self.driver.iptables.defer_apply_off \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_47a3974f","line":62,"updated":"2016-05-19 22:01:10.000000000","message":"assert_called_once() is actually not a method of mock.Mock() and therefore will be a MagicMock()\u0027d method itself. The above needs to be:\n\n self.driver.iptables.defer_apply_on.assert_called_once_with()","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"abc4f2893a49f0929dac914f5ab909b79033bdbf","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    def test_filter_defer_apply_on(self):"},{"line_number":60,"context_line":"        self.driver.iptables.defer_apply_on \u003d mock.Mock()"},{"line_number":61,"context_line":"        self.driver.filter_defer_apply_on()"},{"line_number":62,"context_line":"        self.driver.iptables.defer_apply_on.assert_called_once()"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def test_filter_defer_apply_off(self):"},{"line_number":65,"context_line":"        self.driver.iptables.defer_apply_off \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_91b47235","line":62,"in_reply_to":"bab6814e_22f1fe01","updated":"2016-05-20 15:22:24.000000000","message":"This test and others like it can be simplified by using the @mock.patch syntax. Simply put the @mock.patch call above the function declaration like you did with Setup(), call the filter_defer_apply_on() method, and assert that the mock was called.","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":20411,"name":"Julian","email":"julian.sy@att.com","username":"syjulian"},"change_message_id":"9466d2004d0a1237889217e7eea8f8032a1fb233","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    def test_filter_defer_apply_on(self):"},{"line_number":60,"context_line":"        self.driver.iptables.defer_apply_on \u003d mock.Mock()"},{"line_number":61,"context_line":"        self.driver.filter_defer_apply_on()"},{"line_number":62,"context_line":"        self.driver.iptables.defer_apply_on.assert_called_once()"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def test_filter_defer_apply_off(self):"},{"line_number":65,"context_line":"        self.driver.iptables.defer_apply_off \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_22f1fe01","line":62,"in_reply_to":"bab6814e_47a3974f","updated":"2016-05-20 14:23:19.000000000","message":"Can you explain the difference between the two? I\u0027m new to the python mock library. From what I understand, the method I used doesn\u0027t care what the arguments are while the other does.","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dc0ed2f81657d0a7ff0e1bfe70b05d1c2928057f","unresolved":false,"context_lines":[{"line_number":64,"context_line":"    def test_filter_defer_apply_off(self):"},{"line_number":65,"context_line":"        self.driver.iptables.defer_apply_off \u003d mock.Mock()"},{"line_number":66,"context_line":"        self.driver.filter_defer_apply_off()"},{"line_number":67,"context_line":"        self.driver.iptables.defer_apply_off.assert_called_once()"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def test_unfilter_instance(self):"},{"line_number":70,"context_line":"        instance \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_a78a43bd","line":67,"updated":"2016-05-19 22:01:10.000000000","message":"ditto about assert_called_once() -\u003e assert_called_once_with()","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"abc4f2893a49f0929dac914f5ab909b79033bdbf","unresolved":false,"context_lines":[{"line_number":64,"context_line":"    def test_filter_defer_apply_off(self):"},{"line_number":65,"context_line":"        self.driver.iptables.defer_apply_off \u003d mock.Mock()"},{"line_number":66,"context_line":"        self.driver.filter_defer_apply_off()"},{"line_number":67,"context_line":"        self.driver.iptables.defer_apply_off.assert_called_once()"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def test_unfilter_instance(self):"},{"line_number":70,"context_line":"        instance \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_31f14602","line":67,"in_reply_to":"bab6814e_a78a43bd","updated":"2016-05-20 15:22:24.000000000","message":"See my previous comment about simplifying these tests.","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dc0ed2f81657d0a7ff0e1bfe70b05d1c2928057f","unresolved":false,"context_lines":[{"line_number":69,"context_line":"    def test_unfilter_instance(self):"},{"line_number":70,"context_line":"        instance \u003d mock.Mock()"},{"line_number":71,"context_line":"        network_info \u003d mock.Mock()"},{"line_number":72,"context_line":"        self.driver.instance_info \u003d mock.Mock()"},{"line_number":73,"context_line":"        self.driver.remove_filters_for_instance \u003d mock.Mock()"},{"line_number":74,"context_line":"        self.driver.iptables \u003d mock.Mock()"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_87857fae","line":72,"updated":"2016-05-19 22:01:10.000000000","message":"Instead of creating mocks like this (which do not unset themselves after the function scope), it is better to use @mock.patch() decorators on the test method:\n\n @mock.patch.object(IPT_DRIVER_CLS, \u0027instance_info\u0027)\n @mock.patch.object(IPT_DRIVER_CLS, \u0027remove_filters_for_instance\u0027)\n def test_unfilter_instance(self, remove_mock, instance_info_mock):\n     ...","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"abc4f2893a49f0929dac914f5ab909b79033bdbf","unresolved":false,"context_lines":[{"line_number":69,"context_line":"    def test_unfilter_instance(self):"},{"line_number":70,"context_line":"        instance \u003d mock.Mock()"},{"line_number":71,"context_line":"        network_info \u003d mock.Mock()"},{"line_number":72,"context_line":"        self.driver.instance_info \u003d mock.Mock()"},{"line_number":73,"context_line":"        self.driver.remove_filters_for_instance \u003d mock.Mock()"},{"line_number":74,"context_line":"        self.driver.iptables \u003d mock.Mock()"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_9150f2d5","line":72,"in_reply_to":"bab6814e_85e0b497","updated":"2016-05-20 15:22:24.000000000","message":"Jay Pipes\u0027 comment above gives an example of what I was referring to.","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":20411,"name":"Julian","email":"julian.sy@att.com","username":"syjulian"},"change_message_id":"9466d2004d0a1237889217e7eea8f8032a1fb233","unresolved":false,"context_lines":[{"line_number":69,"context_line":"    def test_unfilter_instance(self):"},{"line_number":70,"context_line":"        instance \u003d mock.Mock()"},{"line_number":71,"context_line":"        network_info \u003d mock.Mock()"},{"line_number":72,"context_line":"        self.driver.instance_info \u003d mock.Mock()"},{"line_number":73,"context_line":"        self.driver.remove_filters_for_instance \u003d mock.Mock()"},{"line_number":74,"context_line":"        self.driver.iptables \u003d mock.Mock()"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_85e0b497","line":72,"in_reply_to":"bab6814e_87857fae","updated":"2016-05-20 14:23:19.000000000","message":"Will do","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dc0ed2f81657d0a7ff0e1bfe70b05d1c2928057f","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        network_info \u003d mock.Mock()"},{"line_number":72,"context_line":"        self.driver.instance_info \u003d mock.Mock()"},{"line_number":73,"context_line":"        self.driver.remove_filters_for_instance \u003d mock.Mock()"},{"line_number":74,"context_line":"        self.driver.iptables \u003d mock.Mock()"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # instance info is truthy"},{"line_number":77,"context_line":"        self.driver.instance_info.pop.return_value \u003d True"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_e7944be3","line":74,"updated":"2016-05-19 22:01:10.000000000","message":"The above is already done in the setUp() method of the test class... it\u0027s the self.ipt_mock attribute.","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"abc4f2893a49f0929dac914f5ab909b79033bdbf","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        self.driver.remove_filters_for_instance \u003d mock.Mock()"},{"line_number":74,"context_line":"        self.driver.iptables \u003d mock.Mock()"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        # instance info is truthy"},{"line_number":77,"context_line":"        self.driver.instance_info.pop.return_value \u003d True"},{"line_number":78,"context_line":"        self.driver.unfilter_instance(instance, network_info)"},{"line_number":79,"context_line":"        self.driver.remove_filters_for_instance \\"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_d1327aa6","line":76,"updated":"2016-05-20 15:22:24.000000000","message":"I\u0027d get rid of this comment.","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"abc4f2893a49f0929dac914f5ab909b79033bdbf","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        self.driver.instance_info.pop.return_value \u003d True"},{"line_number":78,"context_line":"        self.driver.unfilter_instance(instance, network_info)"},{"line_number":79,"context_line":"        self.driver.remove_filters_for_instance \\"},{"line_number":80,"context_line":"            .assert_called_once_with(instance)"},{"line_number":81,"context_line":"        self.driver.iptables.apply.assert_called_once()"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        # instance info is falsy"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_f1b7bed5","line":80,"updated":"2016-05-20 15:22:24.000000000","message":"This\u0027ll be way shorter if you use mock.patch syntax, but for future reference you can avoid continuations by doing:\n\nself.driver.remove_filters.assert_called_once_with(\n    instance\n)","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dc0ed2f81657d0a7ff0e1bfe70b05d1c2928057f","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        self.driver.unfilter_instance(instance, network_info)"},{"line_number":79,"context_line":"        self.driver.remove_filters_for_instance \\"},{"line_number":80,"context_line":"            .assert_called_once_with(instance)"},{"line_number":81,"context_line":"        self.driver.iptables.apply.assert_called_once()"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        # instance info is falsy"},{"line_number":84,"context_line":"        self.driver.remove_filters_for_instance.reset_mock()"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_c78f87ca","line":81,"updated":"2016-05-19 22:01:10.000000000","message":"ditto.","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dc0ed2f81657d0a7ff0e1bfe70b05d1c2928057f","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            .assert_called_once_with(instance)"},{"line_number":81,"context_line":"        self.driver.iptables.apply.assert_called_once()"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        # instance info is falsy"},{"line_number":84,"context_line":"        self.driver.remove_filters_for_instance.reset_mock()"},{"line_number":85,"context_line":"        self.driver.iptables.apply.reset_mock()"},{"line_number":86,"context_line":"        self.driver.instance_info.pop.return_value \u003d False"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_27bdb368","line":83,"updated":"2016-05-19 22:01:10.000000000","message":"What is \"falsy\"? :)","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":20411,"name":"Julian","email":"julian.sy@att.com","username":"syjulian"},"change_message_id":"9466d2004d0a1237889217e7eea8f8032a1fb233","unresolved":false,"context_lines":[{"line_number":80,"context_line":"            .assert_called_once_with(instance)"},{"line_number":81,"context_line":"        self.driver.iptables.apply.assert_called_once()"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"        # instance info is falsy"},{"line_number":84,"context_line":"        self.driver.remove_filters_for_instance.reset_mock()"},{"line_number":85,"context_line":"        self.driver.iptables.apply.reset_mock()"},{"line_number":86,"context_line":"        self.driver.instance_info.pop.return_value \u003d False"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_82538a38","line":83,"in_reply_to":"bab6814e_27bdb368","updated":"2016-05-20 14:23:19.000000000","message":"lol I wasn\u0027t sure what the actual return value is, so I used False to cover anything that would be False in an if condition ( {}, 0, \"\", None, etc)","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"dc0ed2f81657d0a7ff0e1bfe70b05d1c2928057f","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        self.driver.instance_info.pop.return_value \u003d False"},{"line_number":87,"context_line":"        self.driver.unfilter_instance(instance, network_info)"},{"line_number":88,"context_line":"        self.driver.remove_filters_for_instance.assert_not_called()"},{"line_number":89,"context_line":"        self.driver.iptables.apply.assert_not_called()"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def setup_instance_filter(self, iptm_mock, i_rules_mock):"},{"line_number":92,"context_line":"        ipt_mock, driver \u003d self.ipt_mock, self.driver"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_07c06fdf","line":89,"updated":"2016-05-19 22:01:10.000000000","message":"assert_not_called() is also not a method of mock.Mock(). The above should be changed to:\n\n self.assertFalse(self.driver.remove_filters_for_instance.called)\n self.assertFalse(self.driver.iptables.apply.called)\n\nAnd yes, I understand that this mistake was in the original code from me :)","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"},{"author":{"_account_id":20411,"name":"Julian","email":"julian.sy@att.com","username":"syjulian"},"change_message_id":"9466d2004d0a1237889217e7eea8f8032a1fb233","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        self.driver.instance_info.pop.return_value \u003d False"},{"line_number":87,"context_line":"        self.driver.unfilter_instance(instance, network_info)"},{"line_number":88,"context_line":"        self.driver.remove_filters_for_instance.assert_not_called()"},{"line_number":89,"context_line":"        self.driver.iptables.apply.assert_not_called()"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def setup_instance_filter(self, iptm_mock, i_rules_mock):"},{"line_number":92,"context_line":"        ipt_mock, driver \u003d self.ipt_mock, self.driver"}],"source_content_type":"text/x-python","patch_set":5,"id":"bab6814e_055de478","line":89,"in_reply_to":"bab6814e_07c06fdf","updated":"2016-05-20 14:23:19.000000000","message":"I found assert_not_called() here. https://docs.python.org/dev/library/unittest.mock.html\nare we using an older version of mock without this?","commit_id":"10d2ad5e116f6cd96444d2f92509cc56a06ec2a0"}]}