You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "yyqdbngt (via GitHub)" <gi...@apache.org> on 2026/09/05 18:30:31 UTC

[PR] test: deepen QueryHistoryControllerTest with trace list, results and page guard [rocketmq-dashboard]

yyqdbngt opened a new pull request, #3581:
URL: https://github.com/apache/rocketmq-dashboard/pull/3581

   ### Summary
   
   Deepens `QueryHistoryControllerTest` with three uncovered controller paths: the trace-query listing defaults, the per-query results endpoint and the lower page bound.
   
   ### Changes
   
   - `server/src/test/java/org/apache/rocketmq/studio/instance/message/QueryHistoryControllerTest.java`
     - `listsTraceQueriesWithDefaults`: no-param request delegates to the service with null filters and the 1/20 defaults.
     - `returnsMessageQueryResultsById`: `/messages/{id}/results` returns the stored message records for the id.
     - `rejectsPageBelowOne`: page 0 is rejected with the `page must be at least 1` message.
   
   Suite grows from 4 to 7 tests.
   
   ### Testing
   
   - `mvn -B test -Dtest=QueryHistoryControllerTest` passes (7 tests, all green).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test: deepen QueryHistoryControllerTest with trace list, results and page guard [rocketmq-dashboard]

Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins closed pull request #3581: test: deepen QueryHistoryControllerTest with trace list, results and page guard
URL: https://github.com/apache/rocketmq-dashboard/pull/3581


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test: deepen QueryHistoryControllerTest with trace list, results and page guard [rocketmq-dashboard]

Posted by "RockteMQ-AI (via GitHub)" <gi...@apache.org>.
RockteMQ-AI commented on code in PR #3581:
URL: https://github.com/apache/rocketmq-dashboard/pull/3581#discussion_r3941661580


##########
server/src/test/java/org/apache/rocketmq/studio/instance/message/QueryHistoryControllerTest.java:
##########
@@ -97,4 +97,35 @@ void normalizesOptionalHistoryFilters() throws Exception {
         verify(queryHistoryService).listTraceQueries("instance-a", null, 1, 20);
         verify(queryHistoryService).summarize(null);
     }

Review Comment:
   Missing blank line between the closing brace of `normalizesOptionalHistoryFilters` (line 98) and the `@Test` annotation on line 99. Add an empty line for readability and consistency with the spacing between other test methods.



##########
server/src/test/java/org/apache/rocketmq/studio/instance/message/QueryHistoryControllerTest.java:
##########
@@ -97,4 +97,35 @@ void normalizesOptionalHistoryFilters() throws Exception {
         verify(queryHistoryService).listTraceQueries("instance-a", null, 1, 20);
         verify(queryHistoryService).summarize(null);
     }

Review Comment:
   `listsTraceQueriesWithDefaults` verifies the same service call `listTraceQueries(null, null, 1, 20)` as the existing `normalizesOptionalHistoryFilters` test (line 95). The new test hits a different controller path (`/traces` without params vs. the optional-filters path), which adds value, but consider documenting in a comment what distinguishes this scenario so future readers don't see them as duplicates.



##########
server/src/test/java/org/apache/rocketmq/studio/instance/message/QueryHistoryControllerTest.java:
##########
@@ -97,4 +97,35 @@ void normalizesOptionalHistoryFilters() throws Exception {
         verify(queryHistoryService).listTraceQueries("instance-a", null, 1, 20);
         verify(queryHistoryService).summarize(null);
     }
+    @Test
+    void listsTraceQueriesWithDefaults() throws Exception {
+        when(queryHistoryService.listTraceQueries(null, null, 1, 20))
+                .thenReturn(PageResult.of(List.of(), 0, 1, 20));
+
+        mockMvc.perform(get("/api/query-history/traces"))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.data.items").isEmpty())
+                .andExpect(jsonPath("$.data.page").value(1));
+
+        verify(queryHistoryService).listTraceQueries(null, null, 1, 20);
+    }
+
+    @Test
+    void returnsMessageQueryResultsById() throws Exception {
+        when(queryHistoryService.getMessageQueryResults(7L)).thenReturn(List.of());
+
+        mockMvc.perform(get("/api/query-history/messages/7/results"))
+                .andExpect(status().isOk())
+                .andExpect(jsonPath("$.data").isEmpty());

Review Comment:
   `rejectsPageBelowOne` only covers page=0. Consider also asserting that negative values (e.g. page=-1) are rejected, to strengthen the lower-bound guard coverage.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org