)]}'
{"nova/tests/unit/virt/test_firewall.py":[{"author":{"_account_id":20411,"name":"Julian","email":"julian.sy@att.com","username":"syjulian"},"change_message_id":"2b665ad677bd8abcb7bf2cc033d3d82b0d22f7f5","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            dao_mock.assert_called_once_with()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def test_unfilter_instance_valid(self):"},{"line_number":62,"context_line":"        with mock.patch.object(self.driver, \u0027instance_info\u0027) \\"},{"line_number":63,"context_line":"            as ii_mock, \\"},{"line_number":64,"context_line":"            mock.patch.object(self.driver, \u0027remove_filters_for_instance\u0027) \\"},{"line_number":65,"context_line":"            as rfii_mock, \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"bab6814e_7f54dbc9","line":62,"updated":"2016-05-23 20:58:13.000000000","message":"I used this instead of the mock.patch decorator because it cannot mock instance methods. I used the with function so that the mocks reset afterwards","commit_id":"0b6824a4c7a4f808748ec2bb511168389836d333"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"ab8f55ff29a357aa29b67bd1553fd9f1bfa996f7","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            dao_mock.assert_called_once_with()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def test_unfilter_instance_valid(self):"},{"line_number":62,"context_line":"        with mock.patch.object(self.driver, \u0027instance_info\u0027) \\"},{"line_number":63,"context_line":"            as ii_mock, \\"},{"line_number":64,"context_line":"            mock.patch.object(self.driver, \u0027remove_filters_for_instance\u0027) \\"},{"line_number":65,"context_line":"            as rfii_mock, \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"bab6814e_5e59b959","line":62,"in_reply_to":"bab6814e_587106e0","updated":"2016-05-25 12:40:06.000000000","message":"Don\u0027t know what you mean by \"globally instantiate\" but you can just give the python path to the definition of the IptablesFirewallDriver instead of self. My bad for not specifying that.","commit_id":"0b6824a4c7a4f808748ec2bb511168389836d333"},{"author":{"_account_id":20411,"name":"Julian","email":"julian.sy@att.com","username":"syjulian"},"change_message_id":"0ded01c42790a6956af618f3460ba974329f4f1b","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            dao_mock.assert_called_once_with()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def test_unfilter_instance_valid(self):"},{"line_number":62,"context_line":"        with mock.patch.object(self.driver, \u0027instance_info\u0027) \\"},{"line_number":63,"context_line":"            as ii_mock, \\"},{"line_number":64,"context_line":"            mock.patch.object(self.driver, \u0027remove_filters_for_instance\u0027) \\"},{"line_number":65,"context_line":"            as rfii_mock, \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"bab6814e_b96d559b","line":62,"in_reply_to":"bab6814e_5e59b959","updated":"2016-05-25 14:50:09.000000000","message":"For this test I tried adding\n@mock.patch.object(firewall.IpTablesFirewallDriver, \u0027instance_info\u0027)\n\nI\u0027m assuming this is what you meant by putting the python path to IptablesFirewallDriver definition\n\nI would get an attribute error saying IpTablesFirewallDriver has no attribute \u0027instance_info\u0027\n\nThen I tried using an instance\nmock.patch.object(firewall.IpTablesFirewallDriver(), \u0027instance_info\u0027)\n\nIt didn\u0027t throw any errors, so \u0027instance_info\u0027 is probably an instance variable not a class variable. However it\u0027s not going to be from the same instance as self.driver\n\nWhat I meant by originally by globally instantiate was to have a global variable something like\n_IPT_DRIVER \u003d firewall.IpTablesFirewallDriver()\nso _IPT_DRIVER is an instance of the class\n\nBut that would mean I would have to use _IPT_DRIVER instead of self.driver ( which is defined everytime in setUp ) for every assert and re-define _IPT_DRIVER on every setUp instead.\n\nSorry if my terminology is a little off. Hopefully this conveys how I\u0027m understanding things","commit_id":"0b6824a4c7a4f808748ec2bb511168389836d333"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"e6bdb1617e9c1d4d770b5afcef172fbe898d92cb","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            dao_mock.assert_called_once_with()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def test_unfilter_instance_valid(self):"},{"line_number":62,"context_line":"        with mock.patch.object(self.driver, \u0027instance_info\u0027) \\"},{"line_number":63,"context_line":"            as ii_mock, \\"},{"line_number":64,"context_line":"            mock.patch.object(self.driver, \u0027remove_filters_for_instance\u0027) \\"},{"line_number":65,"context_line":"            as rfii_mock, \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"bab6814e_a140c808","line":62,"in_reply_to":"bab6814e_7f54dbc9","updated":"2016-05-24 12:43:25.000000000","message":"mock.patch.object can be used as function decorators as well. All of the long with/as statements can be replaced with these as follows:\n\n@mock.patch.object(self.driver, \u0027instance_info\u0027)\n@mock.patch.object(self.driver, \u0027remove_filters_for_instance\u0027)\ndef test_unfilter_instance_valid(self, mock_remove, mock_info):","commit_id":"0b6824a4c7a4f808748ec2bb511168389836d333"},{"author":{"_account_id":20411,"name":"Julian","email":"julian.sy@att.com","username":"syjulian"},"change_message_id":"1be14b94844bd57ca126e6eb0d796007457b529c","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            dao_mock.assert_called_once_with()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def test_unfilter_instance_valid(self):"},{"line_number":62,"context_line":"        with mock.patch.object(self.driver, \u0027instance_info\u0027) \\"},{"line_number":63,"context_line":"            as ii_mock, \\"},{"line_number":64,"context_line":"            mock.patch.object(self.driver, \u0027remove_filters_for_instance\u0027) \\"},{"line_number":65,"context_line":"            as rfii_mock, \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"bab6814e_587106e0","line":62,"in_reply_to":"bab6814e_a140c808","updated":"2016-05-24 14:42:53.000000000","message":"the decorator is outside the class, so I\u0027m getting \"self is not defined\" errors.\nI can also globally instantiate the IptablesFirewallDriver object and use that, but I would have to use that object for every test case. What do you think?","commit_id":"0b6824a4c7a4f808748ec2bb511168389836d333"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"d0a7ba7a3885b1c9f37e08a0a4723cb0ae98fb70","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            dao_mock.assert_called_once_with()"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    def test_unfilter_instance_valid(self):"},{"line_number":62,"context_line":"        with mock.patch.object(self.driver, \u0027instance_info\u0027) \\"},{"line_number":63,"context_line":"            as ii_mock, \\"},{"line_number":64,"context_line":"            mock.patch.object(self.driver, \u0027remove_filters_for_instance\u0027) \\"},{"line_number":65,"context_line":"            as rfii_mock, \\"}],"source_content_type":"text/x-python","patch_set":1,"id":"bab6814e_d06398af","line":62,"in_reply_to":"bab6814e_b96d559b","updated":"2016-05-26 12:31:30.000000000","message":"Ok here\u0027s how I\u0027d do these tests. Have a \"dummy\" IpTablesFirewall driver that I initialize in the setup function, then for each unit test, explicitly set the iptables and instance_info variables directly within the unit test function def. I\u0027d mock out any functions of the IpTablesFirewallDriver using function decorators.\n\nSo for this specific unit test here, I\u0027d have an @mock.patch decorator to mock out the remove_filters_for_instance method, and I\u0027d directly set the instance_info and iptables fields within the unit test. You can just set them to mocks using MagicMock() or you could define some useful fake objects to pop in there.","commit_id":"0b6824a4c7a4f808748ec2bb511168389836d333"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"e6bdb1617e9c1d4d770b5afcef172fbe898d92cb","unresolved":false,"context_lines":[{"line_number":64,"context_line":"            mock.patch.object(self.driver, \u0027remove_filters_for_instance\u0027) \\"},{"line_number":65,"context_line":"            as rfii_mock, \\"},{"line_number":66,"context_line":"            mock.patch.object(self.driver, \u0027iptables\u0027) as ipt_mock:"},{"line_number":67,"context_line":"            instance_mock \u003d mock.Mock()"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"            ii_mock.pop.return_value \u003d True"},{"line_number":70,"context_line":"            self.driver.unfilter_instance(instance_mock, \u0027fakenetinfo\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bab6814e_41091cdd","line":67,"updated":"2016-05-24 12:43:25.000000000","message":"This probably doesn\u0027t need to be mocked out.","commit_id":"0b6824a4c7a4f808748ec2bb511168389836d333"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"e6bdb1617e9c1d4d770b5afcef172fbe898d92cb","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            mock.patch.object(self.driver, \u0027remove_filters_for_instance\u0027) \\"},{"line_number":77,"context_line":"            as rfii_mock, \\"},{"line_number":78,"context_line":"            mock.patch.object(self.driver, \u0027iptables\u0027) as ipt_mock:"},{"line_number":79,"context_line":"            instance_mock \u003d mock.Mock()"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"            ii_mock.pop.return_value \u003d False"},{"line_number":82,"context_line":"            self.driver.unfilter_instance(instance_mock, \u0027fakenetinfo\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bab6814e_81b824e7","line":79,"updated":"2016-05-24 12:43:25.000000000","message":"Same here. Just use a fake instance object.","commit_id":"0b6824a4c7a4f808748ec2bb511168389836d333"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"689d982cef69dfdc8dd94ebe7c35a8920fe18a37","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            self.driver.filter_defer_apply_off()"},{"line_number":60,"context_line":"            dao_mock.assert_called_once_with()"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    @mock.patch.object(_IPT_DRIVER_CLS, \u0027remove_filters_for_instance\u0027)"},{"line_number":63,"context_line":"    def test_unfilter_instance_valid(self, rfii_mock):"},{"line_number":64,"context_line":"        with mock.patch.object(self.driver, \u0027instance_info\u0027) as ii_mock, \\"},{"line_number":65,"context_line":"            mock.patch.object(self.driver, \u0027iptables\u0027) as ipt_mock:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_a1a98592","line":62,"updated":"2016-05-31 12:06:06.000000000","message":"I think generally you want the class name written out when you\u0027re mocking. Makes the test more readable.","commit_id":"babf58d43a24cd74c3c653341f665e32c5c687a0"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"689d982cef69dfdc8dd94ebe7c35a8920fe18a37","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    @mock.patch.object(_IPT_DRIVER_CLS, \u0027remove_filters_for_instance\u0027)"},{"line_number":63,"context_line":"    def test_unfilter_instance_valid(self, rfii_mock):"},{"line_number":64,"context_line":"        with mock.patch.object(self.driver, \u0027instance_info\u0027) as ii_mock, \\"},{"line_number":65,"context_line":"            mock.patch.object(self.driver, \u0027iptables\u0027) as ipt_mock:"},{"line_number":66,"context_line":"            fake_instance \u003d objects.Instance(id\u003d123)"},{"line_number":67,"context_line":"            ii_mock.pop.return_value \u003d True"},{"line_number":68,"context_line":"            "}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_a18065f8","line":65,"updated":"2016-05-31 12:06:06.000000000","message":"I\u0027m still not totally clear on why these can\u0027t be mocked out as function decorators in some way. If at all possible, that would be very much preferable to this long with/as statement.","commit_id":"babf58d43a24cd74c3c653341f665e32c5c687a0"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"689d982cef69dfdc8dd94ebe7c35a8920fe18a37","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    def test_unfilter_instance_valid(self, rfii_mock):"},{"line_number":64,"context_line":"        with mock.patch.object(self.driver, \u0027instance_info\u0027) as ii_mock, \\"},{"line_number":65,"context_line":"            mock.patch.object(self.driver, \u0027iptables\u0027) as ipt_mock:"},{"line_number":66,"context_line":"            fake_instance \u003d objects.Instance(id\u003d123)"},{"line_number":67,"context_line":"            ii_mock.pop.return_value \u003d True"},{"line_number":68,"context_line":"            "},{"line_number":69,"context_line":"            self.driver.unfilter_instance(fake_instance, \u0027fakenetinfo\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_a1e505dd","line":66,"updated":"2016-05-31 12:06:06.000000000","message":"Thanks for switching to a fake instance, much better.","commit_id":"babf58d43a24cd74c3c653341f665e32c5c687a0"},{"author":{"_account_id":15524,"name":"dane-fichter","email":"dane.fichter@jhuapl.edu","username":"dane-fichter"},"change_message_id":"689d982cef69dfdc8dd94ebe7c35a8920fe18a37","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            mock.patch.object(self.driver, \u0027iptables\u0027) as ipt_mock:"},{"line_number":66,"context_line":"            fake_instance \u003d objects.Instance(id\u003d123)"},{"line_number":67,"context_line":"            ii_mock.pop.return_value \u003d True"},{"line_number":68,"context_line":"            "},{"line_number":69,"context_line":"            self.driver.unfilter_instance(fake_instance, \u0027fakenetinfo\u0027)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"            ii_mock.pop.assert_called_once_with(fake_instance.id, None)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_e198adc2","line":68,"updated":"2016-05-31 12:06:06.000000000","message":"This is your pep8 error right here. White space on a blank line.","commit_id":"babf58d43a24cd74c3c653341f665e32c5c687a0"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"1242502deb1e177788d0dc9536014bf50d72633f","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"import mock"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from nova import objects"},{"line_number":18,"context_line":"from nova import test"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_7fe8e67c","line":15,"updated":"2016-06-12 01:00:38.000000000","message":"There should only be a single blank line here, totally a nit.","commit_id":"76f80babb7c1b5cbf77750cd9282bdb876b188e8"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"75b574c4edc3b1a588c96062455cb7deb447d16d","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    @mock.patch(\u0027nova.network.linux_net.iptables_manager\u0027)"},{"line_number":30,"context_line":"    def test_constructor(self, iptm_mock):"},{"line_number":31,"context_line":"        self.driver.__init__()"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        self.assertEqual({}, self.driver.instance_info)"},{"line_number":34,"context_line":"        self.assertEqual(False, self.driver.dhcp_create)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_cb77ff9d","line":31,"updated":"2016-06-09 15:19:02.000000000","message":"The __init__() has already been called on line 27 so the above line won\u0027t do anything.","commit_id":"76f80babb7c1b5cbf77750cd9282bdb876b188e8"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"1242502deb1e177788d0dc9536014bf50d72633f","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    @mock.patch(\u0027nova.network.linux_net.iptables_manager\u0027)"},{"line_number":30,"context_line":"    def test_constructor(self, iptm_mock):"},{"line_number":31,"context_line":"        self.driver.__init__()"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"        self.assertEqual({}, self.driver.instance_info)"},{"line_number":34,"context_line":"        self.assertEqual(False, self.driver.dhcp_create)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_9ffdd23d","line":31,"in_reply_to":"7aa08908_cb77ff9d","updated":"2016-06-12 01:00:38.000000000","message":"Yeah, this line is a bit weird. Especially how it isn\u0027t done in any of the other tests either.","commit_id":"76f80babb7c1b5cbf77750cd9282bdb876b188e8"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"1242502deb1e177788d0dc9536014bf50d72633f","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        iptm_mock.ipv4.__getitem__.return_value \\"},{"line_number":47,"context_line":"            .assert_has_calls(expected)"},{"line_number":48,"context_line":"        iptm_mock.ipv6.__getitem__.return_value \\"},{"line_number":49,"context_line":"            .assert_has_calls(expected)"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def test_filter_defer_apply_on(self):"},{"line_number":52,"context_line":"        with mock.patch.object(self.driver.iptables,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_3f95bee3","line":49,"updated":"2016-06-12 01:00:38.000000000","message":"This syntax is really odd as well. I don\u0027t think I\u0027ve ever seen something like that before.","commit_id":"76f80babb7c1b5cbf77750cd9282bdb876b188e8"}]}