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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
