zjncs commented on PR #5626: URL: https://github.com/apache/rocketmq-dashboard/pull/5626#issuecomment-6076901303
Thanks @lizhimins — all points addressed. The branch is rebased onto rocketmq-studio c99b9ad5 and the PR has been retargeted; the full diff still touches only the test file. **Rebase and evidence re-run.** Restoring the trailing newline indeed made the merge with #4968's appended tests clean, exactly as you verified with `git merge-file` — the only conflict hunk was the file tail. Note the file at c99b9ad5 now carries **43** tests, not 39: your count matches the pre-#5238 state (that PR merged onto rocketmq-studio at 03:53 UTC, a few hours before the review, adding `renders the page subtitle…` and the three import-toast tests). Evidence re-run on the 43-test file: - Full file in isolation: **43/43** - Targeted 4-file parallel pressure run (ConsumerPage + ConsumerPageDiagnosticsRace + TopicPage + AlertsPage): **107/107, twice** - Full-suite parallel run: **1409/1409**; a companion run had 3 failures in the known TopicPage/AlertsPage timeout-headroom family — zero ConsumerPage failures in either run **The #4968 test.** You were right — `ignores stale subscription responses after a newer diagnostic request completes` does need the same treatment, and it now gets it. It queues two `mockReturnValueOnce` responses and asserts an exact `toHaveBeenCalledTimes(2)` after the 重新诊断 click; a stray 2s tick landing between the modal open and the click consumes the second queued response, and as you note the `subscriptionRequestIdRef` guard only keeps the out-of-order response from clobbering state — the mock the click needed is still gone. With the interval faked the test is deterministic (it never depends on a tick actually firing; the deferreds are resolved manually). The other additions don't open the details modal (#4902's test does, but uses an idempotent `mockResolvedValue` with no exact counts, so a stray tick is harmless there). **Duplicated comment.** Hoisted into a single `fakeModalDiagnosticInterval()` helper defined once next to the other test helpers, with the rationale comment on that definition. All seven call sites use it — the five modal tests, the #4968 test, and the auto-refresh test (which keeps its own comment about driving the tick explicitly with `advanceTimersByTimeAsync`). `vi.useRealTimers()` is still restored in the shared `afterEach`, `tsc --noEmit` and prettier are clean, and the product file remains untouched. -- 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]
