)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"3d1ac7a9eea990b49e03c66412153b800e04729a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"415bcba9_156c3029","updated":"2024-10-04 17:57:07.000000000","message":"I am not sure some of these checks are even needed; or that switching from assertTrue to assertEqual is always for the better. See details in-line.","commit_id":"6c2f5dc67becc3f5b1750057987d70eac969d246"},{"author":{"_account_id":36966,"name":"Myles Penner","display_name":"Myles Penner","email":"myles.penner@canonical.com","username":"mylesjp"},"change_message_id":"da3caa72e61a1f60a6b9560a1782f4e038114426","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ad05c590_2d1065b7","updated":"2024-10-03 22:31:40.000000000","message":"Looks good to me.","commit_id":"6c2f5dc67becc3f5b1750057987d70eac969d246"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1cd851c192952b9fcc30831ddd238288caa6ef29","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8ab0a7bd_89f97cdd","in_reply_to":"415bcba9_156c3029","updated":"2024-10-04 21:49:51.000000000","message":"Typically we check the number of calls and the expected output, checking both can help catch a random edge case IMO, and it\u0027s not like it\u0027s adding any time to the test.","commit_id":"6c2f5dc67becc3f5b1750057987d70eac969d246"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"647301b29977bb34e68abee25b74a90fb6889a46","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"64ba8973_64186951","in_reply_to":"8ab0a7bd_89f97cdd","updated":"2024-10-04 23:05:26.000000000","message":"In a lot of cases, checking that a function was called per se is wrong, unless the checked function is part of some internal interface between components (e.g. callback function triggered for a particular payload).","commit_id":"6c2f5dc67becc3f5b1750057987d70eac969d246"}],"neutron/tests/unit/extensions/test_l3.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"3d1ac7a9eea990b49e03c66412153b800e04729a","unresolved":true,"context_lines":[{"line_number":4422,"context_line":"        self.assertEqual(2, mock_log.call_count)"},{"line_number":4423,"context_line":"        expected_message \u003d (\u0027Port foo_port_id not found while updating \u0027"},{"line_number":4424,"context_line":"                            \u0027agent binding for router foo_router_id.\u0027)"},{"line_number":4425,"context_line":"        actual_message \u003d mock_log.call_args[0][0] % mock_log.call_args[0][1]"},{"line_number":4426,"context_line":"        self.assertEqual(expected_message, actual_message)"},{"line_number":4427,"context_line":""},{"line_number":4428,"context_line":"    def test__ensure_host_set_on_ports_dvr_ha_router_with_gatway(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"4ec3e8cb_daa45d70","line":4425,"updated":"2024-10-04 17:57:07.000000000","message":"we don\u0027t seem to care about the second message though. Should the test then check that there\u0027s \"more or equals to 1\" call (which is already the case here)? (The test case should probably even not assume the order of calls; as long as log is called as expected - whatever the order -, it should be satisfied).","commit_id":"6c2f5dc67becc3f5b1750057987d70eac969d246"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1cd851c192952b9fcc30831ddd238288caa6ef29","unresolved":true,"context_lines":[{"line_number":4422,"context_line":"        self.assertEqual(2, mock_log.call_count)"},{"line_number":4423,"context_line":"        expected_message \u003d (\u0027Port foo_port_id not found while updating \u0027"},{"line_number":4424,"context_line":"                            \u0027agent binding for router foo_router_id.\u0027)"},{"line_number":4425,"context_line":"        actual_message \u003d mock_log.call_args[0][0] % mock_log.call_args[0][1]"},{"line_number":4426,"context_line":"        self.assertEqual(expected_message, actual_message)"},{"line_number":4427,"context_line":""},{"line_number":4428,"context_line":"    def test__ensure_host_set_on_ports_dvr_ha_router_with_gatway(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"974626b7_a296f8df","line":4425,"in_reply_to":"4ec3e8cb_daa45d70","updated":"2024-10-04 21:49:51.000000000","message":"I can work on changing to assert_has_calls()","commit_id":"6c2f5dc67becc3f5b1750057987d70eac969d246"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"647301b29977bb34e68abee25b74a90fb6889a46","unresolved":true,"context_lines":[{"line_number":4422,"context_line":"        self.assertEqual(2, mock_log.call_count)"},{"line_number":4423,"context_line":"        expected_message \u003d (\u0027Port foo_port_id not found while updating \u0027"},{"line_number":4424,"context_line":"                            \u0027agent binding for router foo_router_id.\u0027)"},{"line_number":4425,"context_line":"        actual_message \u003d mock_log.call_args[0][0] % mock_log.call_args[0][1]"},{"line_number":4426,"context_line":"        self.assertEqual(expected_message, actual_message)"},{"line_number":4427,"context_line":""},{"line_number":4428,"context_line":"    def test__ensure_host_set_on_ports_dvr_ha_router_with_gatway(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"55605ca3_eaffba2b","line":4425,"in_reply_to":"974626b7_a296f8df","updated":"2024-10-04 23:05:26.000000000","message":"AFAIU the goal of the test case is to make sure that the _ensure_host_set_on_port function doesn\u0027t raise on PortNotFound. I think this can be achieved without any message checks. They seem redundant. Thoughts?","commit_id":"6c2f5dc67becc3f5b1750057987d70eac969d246"}],"neutron/tests/unit/plugins/ml2/test_plugin.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"3d1ac7a9eea990b49e03c66412153b800e04729a","unresolved":true,"context_lines":[{"line_number":1798,"context_line":"            ])"},{"line_number":1799,"context_line":""},{"line_number":1800,"context_line":"            # check that publisher was still triggered"},{"line_number":1801,"context_line":"            self.assertEqual(3, publish.call_count)"},{"line_number":1802,"context_line":""},{"line_number":1803,"context_line":"    def test_registry_publish_before_after_port_binding(self):"},{"line_number":1804,"context_line":"        plugin \u003d directory.get_plugin()"}],"source_content_type":"text/x-python","patch_set":1,"id":"5da50a2e_9f5e4d40","line":1801,"updated":"2024-10-04 17:57:07.000000000","message":"Should the test case care about whether publish was called *with the expected payload* instead?","commit_id":"6c2f5dc67becc3f5b1750057987d70eac969d246"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"077db810b86af0062796213c22b09d1759965a0e","unresolved":true,"context_lines":[{"line_number":1798,"context_line":"            ])"},{"line_number":1799,"context_line":""},{"line_number":1800,"context_line":"            # check that publisher was still triggered"},{"line_number":1801,"context_line":"            self.assertEqual(3, publish.call_count)"},{"line_number":1802,"context_line":""},{"line_number":1803,"context_line":"    def test_registry_publish_before_after_port_binding(self):"},{"line_number":1804,"context_line":"        plugin \u003d directory.get_plugin()"}],"source_content_type":"text/x-python","patch_set":1,"id":"6e9c22f2_d83f0df6","line":1801,"in_reply_to":"48811aa5_066cd02b","updated":"2024-10-04 23:21:56.000000000","message":"https://review.opendev.org/c/openstack/neutron/+/931532","commit_id":"6c2f5dc67becc3f5b1750057987d70eac969d246"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1cd851c192952b9fcc30831ddd238288caa6ef29","unresolved":true,"context_lines":[{"line_number":1798,"context_line":"            ])"},{"line_number":1799,"context_line":""},{"line_number":1800,"context_line":"            # check that publisher was still triggered"},{"line_number":1801,"context_line":"            self.assertEqual(3, publish.call_count)"},{"line_number":1802,"context_line":""},{"line_number":1803,"context_line":"    def test_registry_publish_before_after_port_binding(self):"},{"line_number":1804,"context_line":"        plugin \u003d directory.get_plugin()"}],"source_content_type":"text/x-python","patch_set":1,"id":"e2e57810_ad45d0cc","line":1801,"in_reply_to":"5da50a2e_9f5e4d40","updated":"2024-10-04 21:49:51.000000000","message":"It could check both.","commit_id":"6c2f5dc67becc3f5b1750057987d70eac969d246"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"647301b29977bb34e68abee25b74a90fb6889a46","unresolved":true,"context_lines":[{"line_number":1798,"context_line":"            ])"},{"line_number":1799,"context_line":""},{"line_number":1800,"context_line":"            # check that publisher was still triggered"},{"line_number":1801,"context_line":"            self.assertEqual(3, publish.call_count)"},{"line_number":1802,"context_line":""},{"line_number":1803,"context_line":"    def test_registry_publish_before_after_port_binding(self):"},{"line_number":1804,"context_line":"        plugin \u003d directory.get_plugin()"}],"source_content_type":"text/x-python","patch_set":1,"id":"48811aa5_066cd02b","line":1801,"in_reply_to":"e2e57810_ad45d0cc","updated":"2024-10-04 23:05:26.000000000","message":"I was curious why it\u0027s triggered 3 times.\n\nThe calls are:\n\n```\nActual: [call(\u0027port\u0027, \u0027before_delete\u0027, \u003cneutron.plugins.ml2.plugin.Ml2Plugin object at 0x11c6bf990\u003e, payload\u003d\u003cneutron_lib.callbacks.events.DBEventPayload object at 0x11a444f90\u003e),\n call(\u0027port\u0027, \u0027precommit_delete\u0027, \u003cneutron.plugins.ml2.plugin.Ml2Plugin object at 0x11c6bf990\u003e, payload\u003d\u003cneutron_lib.callbacks.events.DBEventPayload object at 0x11a37f9d0\u003e),\n call(\u0027port\u0027, \u0027after_delete\u0027, \u003cneutron.plugins.ml2.plugin.Ml2Plugin object at 0x11c6bf990\u003e, payload\u003d\u003cneutron_lib.callbacks.events.DBEventPayload object at 0x11a582fd0\u003e)]\n```\n\n\n---\n\nI\u0027ve checked history for this test case, and the goal here was to make sure that disassociate_floatingips is called with do_notify\u003dFalse. This check is now useless because the argument is ignored now. The argument should now probably be removed; and with it, the whole test case. I will send a patch for this.","commit_id":"6c2f5dc67becc3f5b1750057987d70eac969d246"}],"neutron/tests/unit/services/trunk/test_rules.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"3d1ac7a9eea990b49e03c66412153b800e04729a","unresolved":true,"context_lines":[{"line_number":315,"context_line":"            validator \u003d rules.TrunkPortValidator(port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":316,"context_line":"            self.assertTrue("},{"line_number":317,"context_line":"                validator.can_be_trunked_or_untrunked(self.context))"},{"line_number":318,"context_line":"            self.assertTrue(g.call_count)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def test_can_be_trunked_or_untrunked_unbound_port(self):"},{"line_number":321,"context_line":"        with self.port() as port:"}],"source_content_type":"text/x-python","patch_set":1,"id":"a1a8bfe9_e49613cc","side":"PARENT","line":318,"updated":"2024-10-04 17:57:07.000000000","message":"Why do we care how this is implemented here? The check seems like something that could be just dropped.","commit_id":"1a4f311be73b2607150770de236068b3e3cd6ffd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1cd851c192952b9fcc30831ddd238288caa6ef29","unresolved":true,"context_lines":[{"line_number":315,"context_line":"            validator \u003d rules.TrunkPortValidator(port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":316,"context_line":"            self.assertTrue("},{"line_number":317,"context_line":"                validator.can_be_trunked_or_untrunked(self.context))"},{"line_number":318,"context_line":"            self.assertTrue(g.call_count)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def test_can_be_trunked_or_untrunked_unbound_port(self):"},{"line_number":321,"context_line":"        with self.port() as port:"}],"source_content_type":"text/x-python","patch_set":1,"id":"a3facd7d_c591c150","side":"PARENT","line":318,"in_reply_to":"a1a8bfe9_e49613cc","updated":"2024-10-04 21:49:51.000000000","message":"If a tree falls in the woods...\n\nTechnically, it\u0027s checking we made it past the first line of can_be_trunked_or_untrunked(), which can return True. Do we care that True \u003d\u003d 1 in this case? Probably doesn\u0027t matter but I was trying to make them all consistent once I noticed the first one.","commit_id":"1a4f311be73b2607150770de236068b3e3cd6ffd"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"79aabeb3e878dc001c15f11af9fb7acd817ee2bc","unresolved":true,"context_lines":[{"line_number":315,"context_line":"            validator \u003d rules.TrunkPortValidator(port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":316,"context_line":"            self.assertTrue("},{"line_number":317,"context_line":"                validator.can_be_trunked_or_untrunked(self.context))"},{"line_number":318,"context_line":"            self.assertTrue(g.call_count)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def test_can_be_trunked_or_untrunked_unbound_port(self):"},{"line_number":321,"context_line":"        with self.port() as port:"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfebafaf_dbf1e3a2","side":"PARENT","line":318,"in_reply_to":"a3facd7d_c591c150","updated":"2024-10-04 22:44:50.000000000","message":"Right. But the fact that the check relies on a particular implementation (\"made it past the first line\") suggests to me the check shouldn\u0027t exist / should be replaced with something that would not break with a reasonable refactoring. We have lots of test cases checking the implementation and not effects, and it looks like this is one of them. (The unbound case is already covered in the next test case here.)","commit_id":"1a4f311be73b2607150770de236068b3e3cd6ffd"}]}