Copilot commented on code in PR #866:
URL:
https://github.com/apache/rocketmq-dashboard/pull/866#discussion_r3704986781
##########
web/src/pages/instance/topic.tsx:
##########
@@ -633,7 +644,7 @@ const TopicPage = () => {
setSendModalOpen(false);
sendForm.resetFields();
} catch {
- // validation error, do nothing
+ message.error('发送测试消息失败，请稍后重试');
Review Comment:
This introduces a hard-coded, non-localized UI string. Since the page
already uses `useLang()` (`t`), consider moving this message into the i18n
layer (e.g., `t('topic.testMessage.sendFailed')`) so it’s consistent with the
rest of the page and doesn’t block future localization changes.
##########
web/src/pages/instance/__tests__/TopicPage.test.tsx:
##########
@@ -144,4 +144,28 @@ describe('TopicPage', () => {
expect(screen.getByRole('button', { name: /删除 \(1\)$/
})).toBeInTheDocument();
expect(screen.getByText('已删除 2 个 Topic，1 个删除失败')).toBeInTheDocument();
});
+
+ it('shows an error when sending a test message fails', async () => {
+ const user = userEvent.setup();
+ const errorSpy = vi.spyOn(message, 'error').mockImplementation(vi.fn());
Review Comment:
If an assertion throws before `errorSpy.mockRestore()` runs, this spy can
leak into later tests and cause cascading failures. Prefer restoring in
`afterEach` (or using `vi.restoreAllMocks()` in existing cleanup) or a
`try/finally` within the test to guarantee restoration.
##########
web/src/pages/instance/__tests__/TopicPage.test.tsx:
##########
@@ -144,4 +144,28 @@ describe('TopicPage', () => {
expect(screen.getByRole('button', { name: /删除 \(1\)$/
})).toBeInTheDocument();
expect(screen.getByText('已删除 2 个 Topic，1 个删除失败')).toBeInTheDocument();
});
+
+ it('shows an error when sending a test message fails', async () => {
+ const user = userEvent.setup();
+ const errorSpy = vi.spyOn(message, 'error').mockImplementation(vi.fn());
+ topicServiceMocks.listTopics.mockResolvedValue(buildTopics(1));
+ topicServiceMocks.sendTopicMessage.mockRejectedValue(new Error('send
failed'));
+
+ renderWithProviders();
+
+ expect(await screen.findByText('topic-01')).toBeInTheDocument();
+ const row = within(getTableBody()).getByText('topic-01').closest('tr');
+ expect(row).not.toBeNull();
+ await user.click(within(row as HTMLElement).getByRole('button', { name:
/发送/ }));
+ const dialogTitle = await screen.findByText('发送消息到 topic-01');
+ const dialog = dialogTitle.closest('[role="dialog"]') as HTMLElement;
+ expect(dialog).not.toBeNull();
+ await user.type(within(dialog).getByLabelText('消息体 Body'), 'test message');
+ await user.click(within(dialog).getByRole('button', { name: /^发\s*送$/ }));
+
+ await waitFor(() =>
expect(topicServiceMocks.sendTopicMessage).toHaveBeenCalledTimes(1));
+ expect(errorSpy).toHaveBeenCalledWith('发送测试消息失败，请稍后重试');
+ expect(dialogTitle).toBeInTheDocument();
+ errorSpy.mockRestore();
Review Comment:
If an assertion throws before `errorSpy.mockRestore()` runs, this spy can
leak into later tests and cause cascading failures. Prefer restoring in
`afterEach` (or using `vi.restoreAllMocks()` in existing cleanup) or a
`try/finally` within the test to guarantee restoration.
--
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]
