)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"4245ddf1e895863047d625ad6cb9ab9f0bcb858d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"81cb9040_8b41a9e4","updated":"2024-02-14 11:51:43.000000000","message":"Couple of remarks around paging, otherwise this looks good.","commit_id":"39354ccd0c6af839f29f19546cda14f54a879d20"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"f6ff4a96e63676b1452e35e8d95b93b9371dc6d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"78729771_68f0ee2b","updated":"2024-03-07 11:18:02.000000000","message":"Thanks Jaromir","commit_id":"31fec512e3ba3c596866c234eefc7a33506c8ef7"}],"aodhclient/tests/functional/test_alarm.py":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"4245ddf1e895863047d625ad6cb9ab9f0bcb858d","unresolved":true,"context_lines":[{"line_number":994,"context_line":"        result \u003d self.aodh(\u0027alarm\u0027,"},{"line_number":995,"context_line":"                           params\u003d\"list --filter all_projects\u003dtrue --limit 1\")"},{"line_number":996,"context_line":"        alarm_list \u003d self.parser.listing(result)"},{"line_number":997,"context_line":"        self.assertEqual(1, len(alarm_list))"},{"line_number":998,"context_line":"        # list with sort with key\u003dname dir\u003dasc"},{"line_number":999,"context_line":"        result \u003d self.aodh("},{"line_number":1000,"context_line":"            \u0027alarm\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"89cc392b_74748b14","line":997,"updated":"2024-02-14 11:51:43.000000000","message":"Could we get second page here too? This verifies that the limit works but I don\u0027t see actual pagination tested as indicated on L#992.","commit_id":"39354ccd0c6af839f29f19546cda14f54a879d20"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"9cfb0421a6e0c0b780f9f43c512ca77978d75be7","unresolved":true,"context_lines":[{"line_number":994,"context_line":"        result \u003d self.aodh(\u0027alarm\u0027,"},{"line_number":995,"context_line":"                           params\u003d\"list --filter all_projects\u003dtrue --limit 1\")"},{"line_number":996,"context_line":"        alarm_list \u003d self.parser.listing(result)"},{"line_number":997,"context_line":"        self.assertEqual(1, len(alarm_list))"},{"line_number":998,"context_line":"        # list with sort with key\u003dname dir\u003dasc"},{"line_number":999,"context_line":"        result \u003d self.aodh("},{"line_number":1000,"context_line":"            \u0027alarm\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ce68f548_0d6f6cf5","line":997,"in_reply_to":"406c5338_6a5b4303","updated":"2024-02-15 12:02:31.000000000","message":"Thanks Jaromir,\n\nIndeed pagination is normally done by setting the limit, page size, and marker which is the last resource we have seen. As client testing we are generally interested about couple of things:\n1) Do we include the limit correctly into the request (which this part demonstrates)\n2) Do we represent the returned info correctly (say we request sorted list with limit of 3, it\u0027s the server\u0027s job to return us the correct subsection of the sorted list, but do we represent it to the caller still in correct order as well. I think we can assume that based on the test below)\n3) Do we handle correctly non-full pages. As in if we do request with limit of 2 to a alarm list with 3 items, first call we get two alarms, second call we specify that last as marker, server returns us 1 item, are we ok with that?\n4) And then we send yet another list call where marker is on the last item of the list (we should get empty response from the server), do we handle that correctly too?","commit_id":"39354ccd0c6af839f29f19546cda14f54a879d20"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"377d73ce84f16f27ff43b992af2cfc9135d26012","unresolved":true,"context_lines":[{"line_number":994,"context_line":"        result \u003d self.aodh(\u0027alarm\u0027,"},{"line_number":995,"context_line":"                           params\u003d\"list --filter all_projects\u003dtrue --limit 1\")"},{"line_number":996,"context_line":"        alarm_list \u003d self.parser.listing(result)"},{"line_number":997,"context_line":"        self.assertEqual(1, len(alarm_list))"},{"line_number":998,"context_line":"        # list with sort with key\u003dname dir\u003dasc"},{"line_number":999,"context_line":"        result \u003d self.aodh("},{"line_number":1000,"context_line":"            \u0027alarm\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"406c5338_6a5b4303","line":997,"in_reply_to":"89cc392b_74748b14","updated":"2024-02-15 07:53:59.000000000","message":"If I do `openstack alarm list --help` I don\u0027t see an option to \"print second page\". The only similar thing I see is the --marker parameter, which is exercised below. The \"LIST WITH PAGINATION\" comment refers to this whole section (lines 993 - 1036)","commit_id":"39354ccd0c6af839f29f19546cda14f54a879d20"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"373ce0770f4697afd52373bc25a740284277e9b6","unresolved":true,"context_lines":[{"line_number":994,"context_line":"        result \u003d self.aodh(\u0027alarm\u0027,"},{"line_number":995,"context_line":"                           params\u003d\"list --filter all_projects\u003dtrue --limit 1\")"},{"line_number":996,"context_line":"        alarm_list \u003d self.parser.listing(result)"},{"line_number":997,"context_line":"        self.assertEqual(1, len(alarm_list))"},{"line_number":998,"context_line":"        # list with sort with key\u003dname dir\u003dasc"},{"line_number":999,"context_line":"        result \u003d self.aodh("},{"line_number":1000,"context_line":"            \u0027alarm\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"e7f88a73_9274c3b2","line":997,"in_reply_to":"ce68f548_0d6f6cf5","updated":"2024-02-29 10:35:40.000000000","message":"I extended the part around L1033 to add your 3) and 4). It now has 3 alarms in aodh. It executes 3 requests. Everytime it lists with --limit\u003d2 and uses marker with the last alarm returned. At first request it checks, that 2 alarms were returned and the last alarm isn\u0027t among them. After second request it check, that only the last alarm was returned. After third request it check, that an empty page was returned.\n\nI also had to add another filter to filter by type, because alarms from other concurrently running tests interfered with the counts.","commit_id":"39354ccd0c6af839f29f19546cda14f54a879d20"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"4245ddf1e895863047d625ad6cb9ab9f0bcb858d","unresolved":true,"context_lines":[{"line_number":1030,"context_line":"        result \u003d self.aodh("},{"line_number":1031,"context_line":"            \u0027alarm\u0027,"},{"line_number":1032,"context_line":"            params\u003d\"list --filter all_projects\u003dtrue --sort name:asc \""},{"line_number":1033,"context_line":"                   \"--marker %s\" % created_alarm_id)"},{"line_number":1034,"context_line":"        self.assertIn(\u0027alarm_ptc\u0027,"},{"line_number":1035,"context_line":"                      [r[\u0027name\u0027] for r in self.parser.listing(result)])"},{"line_number":1036,"context_line":"        self.aodh(\u0027alarm\u0027, params\u003d\"delete %s\" % created_alarm_id)"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"        # DELETE"}],"source_content_type":"text/x-python","patch_set":2,"id":"21fd4606_e955a370","line":1035,"range":{"start_line":1033,"start_character":8,"end_line":1035,"end_character":71},"updated":"2024-02-14 11:51:43.000000000","message":"I don\u0027t think the \"--marker\" is exercised in any way and would assume the result will not change if we remove it from this call.","commit_id":"39354ccd0c6af839f29f19546cda14f54a879d20"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"373ce0770f4697afd52373bc25a740284277e9b6","unresolved":true,"context_lines":[{"line_number":1030,"context_line":"        result \u003d self.aodh("},{"line_number":1031,"context_line":"            \u0027alarm\u0027,"},{"line_number":1032,"context_line":"            params\u003d\"list --filter all_projects\u003dtrue --sort name:asc \""},{"line_number":1033,"context_line":"                   \"--marker %s\" % created_alarm_id)"},{"line_number":1034,"context_line":"        self.assertIn(\u0027alarm_ptc\u0027,"},{"line_number":1035,"context_line":"                      [r[\u0027name\u0027] for r in self.parser.listing(result)])"},{"line_number":1036,"context_line":"        self.aodh(\u0027alarm\u0027, params\u003d\"delete %s\" % created_alarm_id)"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"        # DELETE"}],"source_content_type":"text/x-python","patch_set":2,"id":"40d54195_7b3773ff","line":1035,"range":{"start_line":1033,"start_character":8,"end_line":1035,"end_character":71},"in_reply_to":"108f2303_e2e7c8c3","updated":"2024-02-29 10:35:40.000000000","message":"A added the check, that the operation returns only one item. So no we can be sure, that we get only the last item.","commit_id":"39354ccd0c6af839f29f19546cda14f54a879d20"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"377d73ce84f16f27ff43b992af2cfc9135d26012","unresolved":true,"context_lines":[{"line_number":1030,"context_line":"        result \u003d self.aodh("},{"line_number":1031,"context_line":"            \u0027alarm\u0027,"},{"line_number":1032,"context_line":"            params\u003d\"list --filter all_projects\u003dtrue --sort name:asc \""},{"line_number":1033,"context_line":"                   \"--marker %s\" % created_alarm_id)"},{"line_number":1034,"context_line":"        self.assertIn(\u0027alarm_ptc\u0027,"},{"line_number":1035,"context_line":"                      [r[\u0027name\u0027] for r in self.parser.listing(result)])"},{"line_number":1036,"context_line":"        self.aodh(\u0027alarm\u0027, params\u003d\"delete %s\" % created_alarm_id)"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"        # DELETE"}],"source_content_type":"text/x-python","patch_set":2,"id":"cb5214eb_ba52cd7c","line":1035,"range":{"start_line":1033,"start_character":8,"end_line":1035,"end_character":71},"in_reply_to":"21fd4606_e955a370","updated":"2024-02-15 07:53:59.000000000","message":"At this point of the test we have 3 alarms. Sorted ascending their names are: alarm_p1, alarm_p2 and alarm_ptc. We use the id of alarm_p2 as the marker. Without \"--marker\" it returns all of the alarms. With \"--marker \u003cid of alarm_p2\u003e\" it returns only alarm_ptc. I could add another assert here to test that alarm_p2 and alarm_p1 didn\u0027t get returned if you want. This behavior I believe could be thought about as pagination (alarm_p1, alarm_p2 are on 1st page, while we are looking at 2nd page with only alarm_ptc)","commit_id":"39354ccd0c6af839f29f19546cda14f54a879d20"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"9cfb0421a6e0c0b780f9f43c512ca77978d75be7","unresolved":true,"context_lines":[{"line_number":1030,"context_line":"        result \u003d self.aodh("},{"line_number":1031,"context_line":"            \u0027alarm\u0027,"},{"line_number":1032,"context_line":"            params\u003d\"list --filter all_projects\u003dtrue --sort name:asc \""},{"line_number":1033,"context_line":"                   \"--marker %s\" % created_alarm_id)"},{"line_number":1034,"context_line":"        self.assertIn(\u0027alarm_ptc\u0027,"},{"line_number":1035,"context_line":"                      [r[\u0027name\u0027] for r in self.parser.listing(result)])"},{"line_number":1036,"context_line":"        self.aodh(\u0027alarm\u0027, params\u003d\"delete %s\" % created_alarm_id)"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":"        # DELETE"}],"source_content_type":"text/x-python","patch_set":2,"id":"108f2303_e2e7c8c3","line":1035,"range":{"start_line":1033,"start_character":8,"end_line":1035,"end_character":71},"in_reply_to":"cb5214eb_ba52cd7c","updated":"2024-02-15 12:02:31.000000000","message":"Yes, for example we could test that the response contains only 1 item like on L#997. That would indeed confirm that we sent the marker correctly as part of the request. As I said in my initial comment, if you remove the marker part from the call, effectively eliminating the L#1033, this test would still pass. So only thing we currently test is that the marker does not break the request to the server, but we have no idea if it was provided correctly as part of the request for the server to understand it and actually give us list after that marker point. This can very easily happen as restful (http) servers generally ignore headers they don\u0027t care about and still process the request, so we could be sending \"makrer\" that the server just ignored, but we are not any wiser as we just test if this one alarm happens to be in the response. Even more so as we expect 1 item in the response only, but yet the test loops through all the items in the response.","commit_id":"39354ccd0c6af839f29f19546cda14f54a879d20"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"9cfb0421a6e0c0b780f9f43c512ca77978d75be7","unresolved":true,"context_lines":[{"line_number":1050,"context_line":"                           fail_ok\u003dTrue, merge_stderr\u003dTrue)"},{"line_number":1051,"context_line":"        self.assertFirstLineStartsWith(result.splitlines(), expected)"},{"line_number":1052,"context_line":""},{"line_number":1053,"context_line":"        # LIST DOES NOT HAVE ALARM"},{"line_number":1054,"context_line":"        result \u003d self.aodh(\u0027alarm\u0027, params\u003d\"list\")"},{"line_number":1055,"context_line":"        self.assertNotIn(ALARM_ID,"},{"line_number":1056,"context_line":"                         [r[\u0027alarm_id\u0027] for r in self.parser.listing(result)])"}],"source_content_type":"text/x-python","patch_set":2,"id":"d7ac2afc_a87b18d4","line":1053,"updated":"2024-02-15 12:02:31.000000000","message":"Why is this relevant in the scope of testing the client?","commit_id":"39354ccd0c6af839f29f19546cda14f54a879d20"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"373ce0770f4697afd52373bc25a740284277e9b6","unresolved":false,"context_lines":[{"line_number":1050,"context_line":"                           fail_ok\u003dTrue, merge_stderr\u003dTrue)"},{"line_number":1051,"context_line":"        self.assertFirstLineStartsWith(result.splitlines(), expected)"},{"line_number":1052,"context_line":""},{"line_number":1053,"context_line":"        # LIST DOES NOT HAVE ALARM"},{"line_number":1054,"context_line":"        result \u003d self.aodh(\u0027alarm\u0027, params\u003d\"list\")"},{"line_number":1055,"context_line":"        self.assertNotIn(ALARM_ID,"},{"line_number":1056,"context_line":"                         [r[\u0027alarm_id\u0027] for r in self.parser.listing(result)])"}],"source_content_type":"text/x-python","patch_set":2,"id":"94ba36c2_6da032ac","line":1053,"in_reply_to":"d7ac2afc_a87b18d4","updated":"2024-02-29 10:35:40.000000000","message":"deleted","commit_id":"39354ccd0c6af839f29f19546cda14f54a879d20"}]}