)]}'
{"neutron/tests/unit/notifiers/test_ironic.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"67eb76740154835592d1a4dfab10fd43a386bee1","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"class TestIronicNotifier(base.BaseTestCase):"},{"line_number":41,"context_line":"    def setUp(self):"},{"line_number":42,"context_line":"        super(TestIronicNotifier, self).setUp()"},{"line_number":43,"context_line":"        self.ironic_notifier \u003d ironic.Notifier()"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_3475f932","side":"PARENT","line":42,"updated":"2019-09-12 16:01:53.000000000","message":"Why can\u0027t we add a mock.patch.object() here?\n\nself.mock_client \u003d mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse)\n\nThen just tweak the two tests that check the calls?","commit_id":"43256c290cd83ba6461411cb419ee5a41379c012"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"ca2510d0ee493ea14a7152656bb735c4aec013fa","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"class TestIronicNotifier(base.BaseTestCase):"},{"line_number":41,"context_line":"    def setUp(self):"},{"line_number":42,"context_line":"        super(TestIronicNotifier, self).setUp()"},{"line_number":43,"context_line":"        self.ironic_notifier \u003d ironic.Notifier()"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_1f96a44d","side":"PARENT","line":42,"in_reply_to":"5faad753_0643c1b0","updated":"2019-09-16 06:59:12.000000000","message":"You can, but it\u0027s quite an anti-pattern IMO. This is what the decorator-based approach is for.","commit_id":"43256c290cd83ba6461411cb419ee5a41379c012"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"800e44e722bbf19f18929486bbc759262188b797","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"class TestIronicNotifier(base.BaseTestCase):"},{"line_number":41,"context_line":"    def setUp(self):"},{"line_number":42,"context_line":"        super(TestIronicNotifier, self).setUp()"},{"line_number":43,"context_line":"        self.ironic_notifier \u003d ironic.Notifier()"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_37197b21","side":"PARENT","line":42,"in_reply_to":"5faad753_14f79d61","updated":"2019-09-12 16:27:51.000000000","message":"Maybe my original comment was a typo, but this works (along with test changes below):\n\nself.mock_client \u003d mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse).start()\n\nI can push my changes if you like.","commit_id":"43256c290cd83ba6461411cb419ee5a41379c012"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"08acb76e74b8fb8829f8868c66044b8bbbe74aa8","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"class TestIronicNotifier(base.BaseTestCase):"},{"line_number":41,"context_line":"    def setUp(self):"},{"line_number":42,"context_line":"        super(TestIronicNotifier, self).setUp()"},{"line_number":43,"context_line":"        self.ironic_notifier \u003d ironic.Notifier()"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_14f79d61","side":"PARENT","line":42,"in_reply_to":"5faad753_3475f932","updated":"2019-09-12 16:10:55.000000000","message":"I doesn\u0027t work this way, but you can use fixtures with something like\n\n self.mock_client \u003d self.useFixture(fixtures.MockPatchObject(client, \u0027Client\u0027, autospec\u003dTrue)).mock","commit_id":"43256c290cd83ba6461411cb419ee5a41379c012"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"de1da96735a56289596da513d36cc2f8eda4046d","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"class TestIronicNotifier(base.BaseTestCase):"},{"line_number":41,"context_line":"    def setUp(self):"},{"line_number":42,"context_line":"        super(TestIronicNotifier, self).setUp()"},{"line_number":43,"context_line":"        self.ironic_notifier \u003d ironic.Notifier()"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_60dab94c","side":"PARENT","line":42,"in_reply_to":"5faad753_37197b21","updated":"2019-09-13 10:05:59.000000000","message":"This will leave the mock running, affecting other tests. I think moving the decorator to a class level is the cleanest way of handling mocks.","commit_id":"43256c290cd83ba6461411cb419ee5a41379c012"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"07fff8a881aff1429eadc982bc28d45790b7963f","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"class TestIronicNotifier(base.BaseTestCase):"},{"line_number":41,"context_line":"    def setUp(self):"},{"line_number":42,"context_line":"        super(TestIronicNotifier, self).setUp()"},{"line_number":43,"context_line":"        self.ironic_notifier \u003d ironic.Notifier()"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_0643c1b0","side":"PARENT","line":42,"in_reply_to":"5faad753_60dab94c","updated":"2019-09-15 23:20:15.000000000","message":"Can we not add self.addCleanup() to ensure the moch is stopped?\n\nWe can also define self.ironic_notifier in setUp? As it was prior to this change. Instead of defining it in each test method.\n\n    def setUp(self):\n        super(TestIronicNotifier, self).setUp()\n        self.mock_client \u003d mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse).start()\n        self.ironic_notifier \u003d ironic.Notifier()\n        self.addCleanup(self.mock_client.stop)","commit_id":"43256c290cd83ba6461411cb419ee5a41379c012"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"bbfb3c13476c13fb68450bb11d8a51861b3fc338","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"},{"line_number":45,"context_line":"                       autospec\u003dTrue)"},{"line_number":46,"context_line":"    @mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse)"},{"line_number":47,"context_line":"    def test_process_port_update_event_bind_port(self, mock_client,"},{"line_number":48,"context_line":"                                                 mock_queue_event):"},{"line_number":49,"context_line":"        self.ironic_notifier \u003d ironic.Notifier()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5faad753_3571800d","line":46,"range":{"start_line":46,"start_character":0,"end_line":46,"end_character":56},"updated":"2019-09-12 10:17:06.000000000","message":"nit: we could put the mock on the class now","commit_id":"ae4e0e36b1d300bd38bb5021271bb97ff04ee637"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0c49c3bb13a223ffde0d35991cec32a0bb3e215f","unresolved":false,"context_lines":[{"line_number":40,"context_line":"class TestIronicNotifier(base.BaseTestCase):"},{"line_number":41,"context_line":"    def setUp(self):"},{"line_number":42,"context_line":"        super(TestIronicNotifier, self).setUp()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"},{"line_number":45,"context_line":"                       autospec\u003dTrue)"},{"line_number":46,"context_line":"    @mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_95c223e9","line":43,"updated":"2019-09-17 08:04:25.000000000","message":"If You would here do something like:\n\n    with mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse):\n        self.ironic_notifier \u003d ironic.Notifier()\n\nthat should be also enough and patch would be much smaller.\nAlso this variable \"mock_client\" added to many tests is not used anywhere.","commit_id":"50af02536aab8a2c51bf3ad9f3cfe875088bc807"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e9fb53b6f02f709e2c93413d749054bcf1ac93c9","unresolved":false,"context_lines":[{"line_number":40,"context_line":"class TestIronicNotifier(base.BaseTestCase):"},{"line_number":41,"context_line":"    def setUp(self):"},{"line_number":42,"context_line":"        super(TestIronicNotifier, self).setUp()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"},{"line_number":45,"context_line":"                       autospec\u003dTrue)"},{"line_number":46,"context_line":"    @mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_b1fe4503","line":43,"in_reply_to":"3fa7e38b_50e6f2ea","updated":"2019-09-17 19:49:53.000000000","message":"Ok, let\u0027s go with it as it is now. It\u0027s \"only\" unit tests module and we shouldn\u0027t block it too long :)","commit_id":"50af02536aab8a2c51bf3ad9f3cfe875088bc807"},{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"256d116c224c21112bfe783dd6bd16e582d81690","unresolved":false,"context_lines":[{"line_number":40,"context_line":"class TestIronicNotifier(base.BaseTestCase):"},{"line_number":41,"context_line":"    def setUp(self):"},{"line_number":42,"context_line":"        super(TestIronicNotifier, self).setUp()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"},{"line_number":45,"context_line":"                       autospec\u003dTrue)"},{"line_number":46,"context_line":"    @mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_50e6f2ea","line":43,"in_reply_to":"3fa7e38b_7013f5b9","updated":"2019-09-17 14:53:46.000000000","message":"This patch works for me locally. @slawek, let me know if you want me to push this revision.\n\n  --- a/neutron/tests/unit/notifiers/test_ironic.py\n  +++ b/neutron/tests/unit/notifiers/test_ironic.py\n  @@ -40,7 +40,9 @@ def get_fake_port():\n   class TestIronicNotifier(base.BaseTestCase):\n       def setUp(self):\n           super(TestIronicNotifier, self).setUp()\n  -        self.ironic_notifier \u003d ironic.Notifier()\n  +        with mock.patch.object(\n  +                client, \u0027Client\u0027, autospec\u003dFalse) as self.mock_client:\n  +            self.ironic_notifier \u003d ironic.Notifier()\n \n       @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,\n                          autospec\u003dTrue)\n  @@ -175,18 +177,17 @@ class TestIronicNotifier(base.BaseTestCase):\n               2, len(self.ironic_notifier.batch_notifier._pending_events.queue))\n           self.assertEqual(2, mock_spawn_n.call_count)\n \n  -    @mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse)\n  -    def test_send_events(self, mock_client):\n  -        self.ironic_notifier.irclient \u003d mock_client\n  +    def test_send_events(self):\n  +        self.ironic_notifier.irclient \u003d self.mock_client\n           self.ironic_notifier.send_events([\u0027test\u0027, \u0027events\u0027])\n  -        mock_client.events.create.assert_called_with(events\u003d[\u0027test\u0027, \u0027events\u0027])\n  +        self.mock_client.events.create.assert_called_with(\n  +            events\u003d[\u0027test\u0027, \u0027events\u0027])\n \n       @mock.patch.object(ironic.LOG, \u0027error\u0027, autospec\u003dTrue)\n  -    @mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse)\n  -    def test_send_event_method_not_found(self, mock_client, mock_log):\n  -        self.ironic_notifier.irclient \u003d mock_client\n  +    def test_send_event_method_not_found(self, mock_log):\n  +        self.ironic_notifier.irclient \u003d self.mock_client\n           exception \u003d ironic_exc.NotFound()\n  -        mock_client.events.create.side_effect \u003d exception\n  +        self.mock_client.events.create.side_effect \u003d exception\n           self.ironic_notifier.send_events([\u0027test\u0027, \u0027events\u0027])\n           self.assertEqual(1, mock_log.call_count)\n           mock_log.assert_called_with(\u0027The ironic API appears to not support \u0027\n  @@ -194,9 +195,8 @@ class TestIronicNotifier(base.BaseTestCase):\n                                       \u0027be upgraded.\u0027)\n \n       @mock.patch.object(ironic.LOG, \u0027error\u0027, autospec\u003dTrue)\n  -    @mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse)\n  -    def test_send_event_exception(self, mock_client, mock_log):\n  -        self.ironic_notifier.irclient \u003d mock_client\n  -        mock_client.events.create.side_effect \u003d Exception()\n  +    def test_send_event_exception(self, mock_log):\n  +        self.ironic_notifier.irclient \u003d self.mock_client\n  +        self.mock_client.events.create.side_effect \u003d Exception()\n           self.ironic_notifier.send_events([\u0027test\u0027, \u0027events\u0027])\n           self.assertEqual(1, mock_log.call_count)","commit_id":"50af02536aab8a2c51bf3ad9f3cfe875088bc807"},{"author":{"_account_id":23851,"name":"Riccardo Pittau","email":"elfosardo@gmail.com","username":"elfosardo"},"change_message_id":"da984a4d8e81f312315725921b07b09074c58b79","unresolved":false,"context_lines":[{"line_number":40,"context_line":"class TestIronicNotifier(base.BaseTestCase):"},{"line_number":41,"context_line":"    def setUp(self):"},{"line_number":42,"context_line":"        super(TestIronicNotifier, self).setUp()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"},{"line_number":45,"context_line":"                       autospec\u003dTrue)"},{"line_number":46,"context_line":"    @mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_b52ddfa9","line":43,"in_reply_to":"3fa7e38b_95c223e9","updated":"2019-09-17 08:11:24.000000000","message":"mock_client is invoked in some tests, but yeah, it would make sense to remove it where it\u0027s not needed","commit_id":"50af02536aab8a2c51bf3ad9f3cfe875088bc807"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"6d00ec31c6d3d69386928939e436fe889685a13e","unresolved":false,"context_lines":[{"line_number":40,"context_line":"class TestIronicNotifier(base.BaseTestCase):"},{"line_number":41,"context_line":"    def setUp(self):"},{"line_number":42,"context_line":"        super(TestIronicNotifier, self).setUp()"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @mock.patch.object(batch_notifier.BatchNotifier, \u0027queue_event\u0027,"},{"line_number":45,"context_line":"                       autospec\u003dTrue)"},{"line_number":46,"context_line":"    @mock.patch.object(client, \u0027Client\u0027, autospec\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_7013f5b9","line":43,"in_reply_to":"3fa7e38b_b52ddfa9","updated":"2019-09-17 08:45:39.000000000","message":"The reason I didn\u0027t do this is that some tests modify the mock_client with side effects and I don\u0027t think that this will work without conflicts depending on the execution order of the tests.\n\nBut feel free to update the patch as you think.","commit_id":"50af02536aab8a2c51bf3ad9f3cfe875088bc807"}]}