)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":35345,"name":"Grzegorz Koper","email":"grzegorzk@stackhpc.com","username":"gkoper"},"change_message_id":"3f560e0a17db823902589444c2ad7ade39ef6a15","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"11ac7c4f_35cb6b19","updated":"2026-05-08 10:44:05.000000000","message":"I wanted to start a discussion around amount of tests vs reviewability, especially in the context of agentic coding tools.\n\n- agents make it cheap to generate many tests\n- they do not make it cheap to review, maintain, or rerun many low-signal tests\n- so the repository needs a stronger bar for which tests are worth carrying upstream long-term\n\nA proposed distinction could be:\n\nUpstream/review-worthy tests\n- cover distinct user-visible behavior\n- cover documented or intentionally supported input forms\n- cover important validation failures\n- protect against known or plausible regressions\n- provide high signal during future code review\n\nTDD artifacts\n- repeat the same behavior through many nearby permutations\n- primarily mirror implementation structure rather than externally meaningful behavior\n- were useful while developing/debugging, but add little unique regression signal now\n- create ongoing review churn disproportionate to their value\n\nI\u0027ve removed some tests based on that criteria, but it\u0027s a lot less than I originally intended :(\n\nI would appreciate all perspectives (especially Core reviewer ones )","commit_id":"a880f6092669907880da84802529eacdf0af007b"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"c9d38839a35b5fec5b923a7a166d92e86d1dc5ea","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b25757dd_b9fb15c6","in_reply_to":"11ac7c4f_35cb6b19","updated":"2026-05-14 10:37:02.000000000","message":"This seems like a good set of criteria. Something we can add to agents.md?","commit_id":"a880f6092669907880da84802529eacdf0af007b"},{"author":{"_account_id":35345,"name":"Grzegorz Koper","email":"grzegorzk@stackhpc.com","username":"gkoper"},"change_message_id":"c8a4162726a5a0b1c520ae445cae8e940f7aef5d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"512b613f_a2dfce3c","in_reply_to":"b25757dd_b9fb15c6","updated":"2026-05-14 11:33:23.000000000","message":"Hmm good idea. Needs rewording so we dont bloat AGENTS.md. I\u0027ll think about adding that. It would be good to have https://review.opendev.org/c/openstack/kayobe/+/983903 merged already","commit_id":"a880f6092669907880da84802529eacdf0af007b"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"f11fe3c1d17e324f79f7b2b484d1b1583e64262e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fa7851a7_7c11cd8a","updated":"2026-05-11 12:11:29.000000000","message":"I\u0027d be tempted to keep all of these. I don\u0027t see any of them causing additional maintenance burden for now. If there is a strong case for removal, could you add some rationale for removal in a comment?","commit_id":"230481d2ffcedb61cd25ec1af638dcda427979bf"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"b6e4e3913a5905c6980a9cafad86f80ffa2861dd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"91091db0_1e15a4e7","in_reply_to":"7d2f7eae_4f9ad440","updated":"2026-05-11 15:01:28.000000000","message":"Done","commit_id":"230481d2ffcedb61cd25ec1af638dcda427979bf"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"1c61a6e3a56eca1039bacb696856a9ccc461ff2a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7d2f7eae_4f9ad440","in_reply_to":"fa7851a7_7c11cd8a","updated":"2026-05-11 14:00:30.000000000","message":"OK, I looked a bit harder and found existing coverage for most. It seems like test_route_without_table might be useful though.","commit_id":"230481d2ffcedb61cd25ec1af638dcda427979bf"}],"kayobe/tests/unit/plugins/filter/test_nmstate.py":[{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"f11fe3c1d17e324f79f7b2b484d1b1583e64262e","unresolved":true,"context_lines":[{"line_number":327,"context_line":"        }"},{"line_number":328,"context_line":"        self.assertEqual(eth_iface[\"ethtool\"][\"feature\"], expected_features)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    def test_ethtool_combined_config(self):"},{"line_number":331,"context_line":"        \"\"\"Test combined ring and feature configuration.\"\"\""},{"line_number":332,"context_line":"        variables \u003d {"},{"line_number":333,"context_line":"            \"inventory_hostname\": \"test-host\","}],"source_content_type":"text/x-python","patch_set":2,"id":"3d2d7aba_ccb3e81e","side":"PARENT","line":330,"updated":"2026-05-11 12:11:29.000000000","message":"Potentially the combined version could be kept? E.g move ring into test above.","commit_id":"1486c00bdb3e66d88a61e7a8cd70957d6c403774"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"1c61a6e3a56eca1039bacb696856a9ccc461ff2a","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        }"},{"line_number":328,"context_line":"        self.assertEqual(eth_iface[\"ethtool\"][\"feature\"], expected_features)"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    def test_ethtool_combined_config(self):"},{"line_number":331,"context_line":"        \"\"\"Test combined ring and feature configuration.\"\"\""},{"line_number":332,"context_line":"        variables \u003d {"},{"line_number":333,"context_line":"            \"inventory_hostname\": \"test-host\","}],"source_content_type":"text/x-python","patch_set":2,"id":"156bbd0d_678248f6","side":"PARENT","line":330,"in_reply_to":"3d2d7aba_ccb3e81e","updated":"2026-05-11 14:00:30.000000000","message":"I found an existing test that covers both: test_nmstate_config_ethernet","commit_id":"1486c00bdb3e66d88a61e7a8cd70957d6c403774"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"f11fe3c1d17e324f79f7b2b484d1b1583e64262e","unresolved":true,"context_lines":[{"line_number":472,"context_line":"            if i[\"name\"] \u003d\u003d \"eth0\")"},{"line_number":473,"context_line":"        self.assertEqual(eth_iface[\"type\"], \"ethernet\")"},{"line_number":474,"context_line":""},{"line_number":475,"context_line":"    def test_vlan_interface_parent_derivation(self):"},{"line_number":476,"context_line":"        \"\"\"Test VLAN parent derivation from interface name.\"\"\""},{"line_number":477,"context_line":"        variables \u003d {"},{"line_number":478,"context_line":"            \"inventory_hostname\": \"test-host\","}],"source_content_type":"text/x-python","patch_set":2,"id":"32e97657_443c932f","side":"PARENT","line":475,"updated":"2026-05-11 12:11:29.000000000","message":"This one seems useful and I didn\u0027t see another test that covers this e.g eth0.100","commit_id":"1486c00bdb3e66d88a61e7a8cd70957d6c403774"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"1c61a6e3a56eca1039bacb696856a9ccc461ff2a","unresolved":false,"context_lines":[{"line_number":472,"context_line":"            if i[\"name\"] \u003d\u003d \"eth0\")"},{"line_number":473,"context_line":"        self.assertEqual(eth_iface[\"type\"], \"ethernet\")"},{"line_number":474,"context_line":""},{"line_number":475,"context_line":"    def test_vlan_interface_parent_derivation(self):"},{"line_number":476,"context_line":"        \"\"\"Test VLAN parent derivation from interface name.\"\"\""},{"line_number":477,"context_line":"        variables \u003d {"},{"line_number":478,"context_line":"            \"inventory_hostname\": \"test-host\","}],"source_content_type":"text/x-python","patch_set":2,"id":"1349221b_5e84b1ca","side":"PARENT","line":475,"in_reply_to":"32e97657_443c932f","updated":"2026-05-11 14:00:30.000000000","message":"I see it: test_vlan_interface_naming_heuristic. OK, seems like this one can be removed.","commit_id":"1486c00bdb3e66d88a61e7a8cd70957d6c403774"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"1c61a6e3a56eca1039bacb696856a9ccc461ff2a","unresolved":false,"context_lines":[{"line_number":639,"context_line":"        self.assertEqual(default_routes[0][\"next-hop-address\"],"},{"line_number":640,"context_line":"                         \"10.0.0.254\")"},{"line_number":641,"context_line":""},{"line_number":642,"context_line":"    def test_defroute_unset_static_ip(self):"},{"line_number":643,"context_line":"        \"\"\"Test defroute unset (None) adds default route for static IP.\"\"\""},{"line_number":644,"context_line":"        variables \u003d {"},{"line_number":645,"context_line":"            \"inventory_hostname\": \"test-host\","}],"source_content_type":"text/x-python","patch_set":2,"id":"f446eb8e_225837c4","side":"PARENT","line":642,"updated":"2026-05-11 14:00:30.000000000","message":"Covered in test_nmstate_config_ethernet which doesn\u0027t set defroute","commit_id":"1486c00bdb3e66d88a61e7a8cd70957d6c403774"},{"author":{"_account_id":28048,"name":"Will Szumski","email":"will@stackhpc.com","username":"jovial"},"change_message_id":"1c61a6e3a56eca1039bacb696856a9ccc461ff2a","unresolved":true,"context_lines":[{"line_number":744,"context_line":"        self.assertIn({\"name\": \"eth1\"}, bridge_ports)"},{"line_number":745,"context_line":"        self.assertIn({\"name\": \"p-br0-phy\"}, bridge_ports)"},{"line_number":746,"context_line":""},{"line_number":747,"context_line":"    def test_route_without_table(self):"},{"line_number":748,"context_line":"        \"\"\"Test route without table-id omits the field.\"\"\""},{"line_number":749,"context_line":"        variables \u003d {"},{"line_number":750,"context_line":"            \"inventory_hostname\": \"test-host\","}],"source_content_type":"text/x-python","patch_set":2,"id":"9a5c17a2_a391589a","side":"PARENT","line":747,"updated":"2026-05-11 14:00:30.000000000","message":"This one seems like it could be useful.","commit_id":"1486c00bdb3e66d88a61e7a8cd70957d6c403774"},{"author":{"_account_id":35345,"name":"Grzegorz Koper","email":"grzegorzk@stackhpc.com","username":"gkoper"},"change_message_id":"0f7ddec59c4629e58f34c506d04c2a97787917d3","unresolved":false,"context_lines":[{"line_number":744,"context_line":"        self.assertIn({\"name\": \"eth1\"}, bridge_ports)"},{"line_number":745,"context_line":"        self.assertIn({\"name\": \"p-br0-phy\"}, bridge_ports)"},{"line_number":746,"context_line":""},{"line_number":747,"context_line":"    def test_route_without_table(self):"},{"line_number":748,"context_line":"        \"\"\"Test route without table-id omits the field.\"\"\""},{"line_number":749,"context_line":"        variables \u003d {"},{"line_number":750,"context_line":"            \"inventory_hostname\": \"test-host\","}],"source_content_type":"text/x-python","patch_set":2,"id":"16d7ace9_8d9ae7fe","side":"PARENT","line":747,"in_reply_to":"9a5c17a2_a391589a","updated":"2026-05-14 10:29:53.000000000","message":"Fair point. Rather than restoring whole test, I\u0027ve added a single assertion\ninto test_route_with_supported_attributes","commit_id":"1486c00bdb3e66d88a61e7a8cd70957d6c403774"}]}