)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b24cef3c7bafe7e3950e9f5f2a306f2b4186fd85","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ad1f1dcb_d8ccf8ce","updated":"2025-10-02 11:44:53.000000000","message":"This is truly awful - I guess it\u0027s all auto-generated. I stopped part way through.\n\nMany of the tests are superficial. Testing private methods makes refactoring harder.","commit_id":"1cd4456a1255eeb22adc1135e59586e9c20031ca"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"4f29a81d8253b2172d0becfc88d70bbcc04e9087","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c6c8e82b_aa77a573","in_reply_to":"97419456_4307e965","updated":"2025-10-02 11:52:51.000000000","message":"That makes more sense 😄","commit_id":"1cd4456a1255eeb22adc1135e59586e9c20031ca"},{"author":{"_account_id":22629,"name":"Michal Nasiadka","email":"mnasiadka@gmail.com","username":"mnasiadka"},"change_message_id":"c7558bc6dea49d56f7602568d75e27ea55dbbc7b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"97419456_4307e965","in_reply_to":"ad1f1dcb_d8ccf8ce","updated":"2025-10-02 11:47:30.000000000","message":"Sorry, forgot to mark it as WIP - it was only a base and autogenerated","commit_id":"1cd4456a1255eeb22adc1135e59586e9c20031ca"}],"kolla_ansible/tests/unit/test_cli.py":[{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b24cef3c7bafe7e3950e9f5f2a306f2b4186fd85","unresolved":true,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"class TestHelperFunctions(unittest.TestCase):"},{"line_number":36,"context_line":"    \"\"\"Test helper functions\"\"\""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    @mock.patch(\u0027kolla_ansible.cli.commands.utils.get_data_files_path\u0027)"},{"line_number":39,"context_line":"    def test_get_playbook_path(self, mock_get_path):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ade2d8fb_28538ecc","line":36,"updated":"2025-10-02 11:44:53.000000000","message":"nit: doc string needs to be more descriptive or removed","commit_id":"1cd4456a1255eeb22adc1135e59586e9c20031ca"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b24cef3c7bafe7e3950e9f5f2a306f2b4186fd85","unresolved":true,"context_lines":[{"line_number":38,"context_line":"    @mock.patch(\u0027kolla_ansible.cli.commands.utils.get_data_files_path\u0027)"},{"line_number":39,"context_line":"    def test_get_playbook_path(self, mock_get_path):"},{"line_number":40,"context_line":"        mock_get_path.return_value \u003d \u0027/path/to/playbook.yml\u0027"},{"line_number":41,"context_line":"        result \u003d commands._get_playbook_path(\u0027test-playbook\u0027)"},{"line_number":42,"context_line":"        mock_get_path.assert_called_once_with(\u0027ansible\u0027, \u0027test-playbook.yml\u0027)"},{"line_number":43,"context_line":"        self.assertEqual(result, \u0027/path/to/playbook.yml\u0027)"},{"line_number":44,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"c1be13f7_d6aaf36a","line":41,"updated":"2025-10-02 11:44:53.000000000","message":"It\u0027s generally bad practise to test private methods.\n\nYou could test it indirectly in this case via `take_action`\n\nAlso, because of the mock, this isn\u0027t actually testing anything of significance.","commit_id":"1cd4456a1255eeb22adc1135e59586e9c20031ca"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b24cef3c7bafe7e3950e9f5f2a306f2b4186fd85","unresolved":true,"context_lines":[{"line_number":43,"context_line":"        self.assertEqual(result, \u0027/path/to/playbook.yml\u0027)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @mock.patch(\u0027kolla_ansible.cli.commands._get_playbook_path\u0027)"},{"line_number":46,"context_line":"    def test_choose_playbooks_custom(self, mock_get_path):"},{"line_number":47,"context_line":"        parsed_args \u003d MockParsedArgs(playbook\u003d[\u0027/custom/playbook.yml\u0027])"},{"line_number":48,"context_line":"        result \u003d commands._choose_playbooks(parsed_args)"},{"line_number":49,"context_line":"        self.assertEqual(result, [\u0027/custom/playbook.yml\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"09d36f9c_aeec85bd","line":46,"updated":"2025-10-02 11:44:53.000000000","message":"Same as above\n - don\u0027t test private methods\n - test is trivial","commit_id":"1cd4456a1255eeb22adc1135e59586e9c20031ca"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b24cef3c7bafe7e3950e9f5f2a306f2b4186fd85","unresolved":true,"context_lines":[{"line_number":50,"context_line":"        mock_get_path.assert_not_called()"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    @mock.patch(\u0027kolla_ansible.cli.commands._get_playbook_path\u0027)"},{"line_number":53,"context_line":"    def test_choose_playbooks_default(self, mock_get_path):"},{"line_number":54,"context_line":"        mock_get_path.return_value \u003d \u0027/path/to/site.yml\u0027"},{"line_number":55,"context_line":"        parsed_args \u003d MockParsedArgs()"},{"line_number":56,"context_line":"        result \u003d commands._choose_playbooks(parsed_args, \u0027site\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"6aeb2f2b_3bca3347","line":53,"updated":"2025-10-02 11:44:53.000000000","message":"ditto","commit_id":"1cd4456a1255eeb22adc1135e59586e9c20031ca"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b24cef3c7bafe7e3950e9f5f2a306f2b4186fd85","unresolved":true,"context_lines":[{"line_number":61,"context_line":"class TestKollaAnsibleMixin(unittest.TestCase):"},{"line_number":62,"context_line":"    \"\"\"Test KollaAnsibleMixin class\"\"\""},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def test_get_verbosity_args_verbose(self):"},{"line_number":65,"context_line":"        mixin \u003d commands.KollaAnsibleMixin()"},{"line_number":66,"context_line":"        mixin.app \u003d MockApp(verbose_level\u003d3)"},{"line_number":67,"context_line":"        result \u003d mixin._get_verbosity_args()"}],"source_content_type":"text/x-python","patch_set":2,"id":"6c0ff8f0_21f71458","line":64,"updated":"2025-10-02 11:44:53.000000000","message":"ditto; test it via `run_playbooks` instead","commit_id":"1cd4456a1255eeb22adc1135e59586e9c20031ca"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b24cef3c7bafe7e3950e9f5f2a306f2b4186fd85","unresolved":true,"context_lines":[{"line_number":83,"context_line":""},{"line_number":84,"context_line":"        mock_run.assert_called_once()"},{"line_number":85,"context_line":"        call_kwargs \u003d mock_run.call_args[1]"},{"line_number":86,"context_line":"        self.assertTrue(\u0027verbose_level\u0027 in call_kwargs or"},{"line_number":87,"context_line":"                        \u0027quiet\u0027 in call_kwargs)"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    @mock.patch(\u0027kolla_ansible.cli.commands.ansible.run_playbooks\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"435d845a_c6276285","line":86,"updated":"2025-10-02 11:44:53.000000000","message":"why is there an `or` here?! when is quiet in call_kwargs?","commit_id":"1cd4456a1255eeb22adc1135e59586e9c20031ca"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b24cef3c7bafe7e3950e9f5f2a306f2b4186fd85","unresolved":true,"context_lines":[{"line_number":107,"context_line":"class TestGatherFacts(unittest.TestCase):"},{"line_number":108,"context_line":"    \"\"\"Test GatherFacts command\"\"\""},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    @mock.patch(\u0027kolla_ansible.cli.commands._choose_playbooks\u0027)"},{"line_number":111,"context_line":"    def test_take_action(self, mock_choose):"},{"line_number":112,"context_line":"        cmd \u003d commands.GatherFacts(mock.Mock(), mock.Mock())"},{"line_number":113,"context_line":"        cmd.app \u003d MockApp()"}],"source_content_type":"text/x-python","patch_set":2,"id":"ab0998bd_b6085617","line":110,"updated":"2025-10-02 11:44:53.000000000","message":"Rather than mocking `_choose_playbooks` here, it would be better to call so it is tested here indirectly. Then delete `test_choose_playbooks*`","commit_id":"1cd4456a1255eeb22adc1135e59586e9c20031ca"},{"author":{"_account_id":17669,"name":"Doug Szumski","email":"doug@stackhpc.com","username":"DougSzumski"},"change_message_id":"b24cef3c7bafe7e3950e9f5f2a306f2b4186fd85","unresolved":true,"context_lines":[{"line_number":157,"context_line":""},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"class TestGenConfig(unittest.TestCase):"},{"line_number":160,"context_line":"    \"\"\"Test GenConfig command\"\"\""},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    @mock.patch(\u0027kolla_ansible.cli.commands._choose_playbooks\u0027)"},{"line_number":163,"context_line":"    def test_take_action(self, mock_choose):"}],"source_content_type":"text/x-python","patch_set":2,"id":"432616d7_2a73bb81","line":160,"updated":"2025-10-02 11:44:53.000000000","message":"What is this docstring supposed to convey beyond the class name?","commit_id":"1cd4456a1255eeb22adc1135e59586e9c20031ca"}]}