)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"341d665ce07c90a973ae3eb61af72dcb700a5bf2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"60f60d22_3986e962","updated":"2025-08-21 13:48:25.000000000","message":"If you don\u0027t have time for adding more tests (negative one) we can do as a follow up too. Thanks for proposing it.","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"0a3c85fff19b3d5d25ea8cdfb84221d4f8ff595f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"fe46ef9d_bd2390e1","updated":"2025-08-28 11:25:53.000000000","message":"Overall looks good.","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"0d4253415214917692a587c07c4b6ce1c29f262e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b17abe17_b9acc206","updated":"2025-08-18 21:01:05.000000000","message":"hi @Alfredo, haven\u0027t you considered a scenario test to validate the applier implementation?","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"4da48589f2771b60b2641dcc165bd5fcfc3e7eba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8c46a1c9_16dd57e1","updated":"2025-08-21 12:18:14.000000000","message":"recheck","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"ae5b38f74af1482353d407872df20f6960743f56","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"50188870_24db2018","updated":"2025-08-27 11:44:34.000000000","message":"recheck\n\nchanges merged and also a follow up change is now merged","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"62c4bb78984301f89d91c65ca41abb652c476454","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"55ad6a2a_83198c41","updated":"2025-08-28 12:20:27.000000000","message":"we agreeded to merge this for now to get some baselien testing in our irc meeting and we will adress the comment in a follow up review.","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"2b7a21e01876af46069d84544981d28475dfd46e","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7d0ec38c_29193b6e","in_reply_to":"5316b942_12de321d","updated":"2025-08-28 11:22:29.000000000","message":"Thank you Doug for the suggestions, I will work on adding the negative test.","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"341d665ce07c90a973ae3eb61af72dcb700a5bf2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"5316b942_12de321d","in_reply_to":"afaaa2b2_6e1e01b5","updated":"2025-08-21 13:48:25.000000000","message":"You are right, I missed that detail because I was looking only at the API validation point of view. Indeed it better to use the dummy strategy if you are not really testing the strategy itself, or an specific Action.\nAPI tests shouldn\u0027t be validating features, but it is fine that they need to create other resources/do some action to allow us to test the API.\nI would suggest to also include a negative test. You could try to patch an action that failed/succeeded, and for that, you will need to trigger the action plan yes.","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"eb7ed0bc12c25919108880ddadeecfc6f87f73ed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"afaaa2b2_6e1e01b5","in_reply_to":"b17abe17_b9acc206","updated":"2025-08-20 18:01:55.000000000","message":"I\u0027m not sure what would be the difference, tbh. In this test I\u0027m creating a dummy audit wich creates an actual actionplan (with nop and sleep actions, true), call patch api to skip, run the actionplan and check that result is expected so I guess I\u0027m exercising the entire workflow end to end including the applier implementation.\n\nIIUC, the only difference with a scenario test would be to use a more real strategy? or this test can be considered an scenario test as it\u0027s running the end-to-end workflow and I should move it?\n\nWhat would you propose? I\u0027m open to any change that improve coverage, but I want to avoid adding a more complex strategy that makes it less reliable and longer if we can test this particular behavior in a simpler way.","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"}],"watcher_tempest_plugin/tests/api/admin/test_action.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"341d665ce07c90a973ae3eb61af72dcb700a5bf2","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        self.assertEqual(\"PENDING\", action[\u0027state\u0027])"},{"line_number":152,"context_line":"        self.client.update_action("},{"line_number":153,"context_line":"            action[\u0027uuid\u0027],"},{"line_number":154,"context_line":"            [{\u0027op\u0027: \u0027replace\u0027, \u0027path\u0027: \u0027/state\u0027, \u0027value\u0027: \u0027SKIPPED\u0027},"},{"line_number":155,"context_line":"             {\u0027op\u0027: \u0027replace\u0027, \u0027path\u0027: \u0027/status_message\u0027,"},{"line_number":156,"context_line":"              \u0027value\u0027: \u0027skipped test\u0027}]"},{"line_number":157,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":4,"id":"342df29f_89f68e3b","line":154,"range":{"start_line":154,"start_character":13,"end_line":154,"end_character":69},"updated":"2025-08-21 13:48:25.000000000","message":"We could add some negative tests too, trying to patch a Action that is not on a valid state (maybe after a failed NOP?)\nAnd also avoid do everything in a single test too.","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"62c4bb78984301f89d91c65ca41abb652c476454","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        self.assertEqual(\"PENDING\", action[\u0027state\u0027])"},{"line_number":152,"context_line":"        self.client.update_action("},{"line_number":153,"context_line":"            action[\u0027uuid\u0027],"},{"line_number":154,"context_line":"            [{\u0027op\u0027: \u0027replace\u0027, \u0027path\u0027: \u0027/state\u0027, \u0027value\u0027: \u0027SKIPPED\u0027},"},{"line_number":155,"context_line":"             {\u0027op\u0027: \u0027replace\u0027, \u0027path\u0027: \u0027/status_message\u0027,"},{"line_number":156,"context_line":"              \u0027value\u0027: \u0027skipped test\u0027}]"},{"line_number":157,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":4,"id":"8bc85531_7bb6413a","line":154,"range":{"start_line":154,"start_character":13,"end_line":154,"end_character":69},"in_reply_to":"342df29f_89f68e3b","updated":"2025-08-28 12:20:27.000000000","message":"we could but each test shoudl test one thing..\n\nif we want to have a test that test multipel thing i would like to have a spereate test that test the other pats indepentely where reasonable.\n\n\nwe defintlly shoudl not be adding the negitive test cases to this test.\n\ndoing a multi stage test would eb better to do in the secnioar tests.\n\nteh api oen should be kept simple.\n\nbut +1 for adding negitive tests in a followup.","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"341d665ce07c90a973ae3eb61af72dcb700a5bf2","unresolved":true,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        self.start_action_plan(action_plan)"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        _, action_plan \u003d self.client.show_action_plan(action_plan)"},{"line_number":167,"context_line":"        self.assertEqual(\u0027SUCCEEDED\u0027, action_plan[\u0027state\u0027])"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        _, final_body \u003d self.client.list_actions(audit_uuid\u003dself.audit[\"uuid\"])"},{"line_number":170,"context_line":"        final_actions \u003d final_body[\u0027actions\u0027]"},{"line_number":171,"context_line":"        self.assertEqual(\"SKIPPED\", final_actions[0][\u0027state\u0027])"},{"line_number":172,"context_line":"        self.assertEqual(\"SUCCEEDED\", final_actions[1][\u0027state\u0027])"},{"line_number":173,"context_line":"        self.assertEqual(\"SUCCEEDED\", final_actions[2][\u0027state\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"f0aae32a_3bbf7d90","line":173,"range":{"start_line":166,"start_character":0,"end_line":173,"end_character":64},"updated":"2025-08-21 13:48:25.000000000","message":"Yeah, it is testing the Applier implementation here, but it is kind of hidden here in the API testing. Not sure if we should move this part to ``scenarios``, were we could create more tests for the applier worklfow itself, like failing in different workflow stages (using the nop failure).\nI wouldn\u0027t block this patch from merging because of this, but we may want to have them in other place in the future.","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"62c4bb78984301f89d91c65ca41abb652c476454","unresolved":true,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        self.start_action_plan(action_plan)"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        _, action_plan \u003d self.client.show_action_plan(action_plan)"},{"line_number":167,"context_line":"        self.assertEqual(\u0027SUCCEEDED\u0027, action_plan[\u0027state\u0027])"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        _, final_body \u003d self.client.list_actions(audit_uuid\u003dself.audit[\"uuid\"])"},{"line_number":170,"context_line":"        final_actions \u003d final_body[\u0027actions\u0027]"},{"line_number":171,"context_line":"        self.assertEqual(\"SKIPPED\", final_actions[0][\u0027state\u0027])"},{"line_number":172,"context_line":"        self.assertEqual(\"SUCCEEDED\", final_actions[1][\u0027state\u0027])"},{"line_number":173,"context_line":"        self.assertEqual(\"SUCCEEDED\", final_actions[2][\u0027state\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"4f6da327_cbd3cd15","line":173,"range":{"start_line":166,"start_character":0,"end_line":173,"end_character":64},"in_reply_to":"1ddce947_22738d1b","updated":"2025-08-28 12:20:27.000000000","message":"ya i agree.\n\nthe api test shoudl just update the value then read it back\n\nwe shoudl not be starting the action plan in the api tests.","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"0a3c85fff19b3d5d25ea8cdfb84221d4f8ff595f","unresolved":true,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        self.start_action_plan(action_plan)"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        _, action_plan \u003d self.client.show_action_plan(action_plan)"},{"line_number":167,"context_line":"        self.assertEqual(\u0027SUCCEEDED\u0027, action_plan[\u0027state\u0027])"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        _, final_body \u003d self.client.list_actions(audit_uuid\u003dself.audit[\"uuid\"])"},{"line_number":170,"context_line":"        final_actions \u003d final_body[\u0027actions\u0027]"},{"line_number":171,"context_line":"        self.assertEqual(\"SKIPPED\", final_actions[0][\u0027state\u0027])"},{"line_number":172,"context_line":"        self.assertEqual(\"SUCCEEDED\", final_actions[1][\u0027state\u0027])"},{"line_number":173,"context_line":"        self.assertEqual(\"SUCCEEDED\", final_actions[2][\u0027state\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ddce947_22738d1b","line":173,"range":{"start_line":166,"start_character":0,"end_line":173,"end_character":64},"in_reply_to":"f0aae32a_3bbf7d90","updated":"2025-08-28 11:25:53.000000000","message":"+1 to that, I will do a cleanup for this one.","commit_id":"af4e2f987d3f9716af0f893b07a518bd28009135"}]}