unbridled-41 commented on PR #4902:
URL:
https://github.com/apache/rocketmq-dashboard/pull/4902#issuecomment-5775357652
### Verification pass — clean checkout of `c4c48bbb`
Re-verified from a fresh `git worktree add --detach
fork/fix/cloud-progress-unknown-offsets`, with an
empty `server/target` (full recompile) and no reused build output.
#### Description defects found and corrected
1. Three citations had been taken from a working copy that carried a
later-discarded local edit, so
they pointed six lines off on the base commit. Re-read with `grep -n` and
corrected:
`formatOffsetValue` is at `consumer.tsx:179-180` (not `185-186`), the
reset-preview guard
`targetOffset < 0 || consumerOffset < 0` at `:222` (not `228`), and the
two progress columns that
rendered `offset.toLocaleString()` at `:1296` and `:1306` (not
`1295-1309`). The description now
gives base-commit and branch numbers for every site, including the new
code
(`TencentInstanceProvider.java:540-541`, `AliyunConverters.java:197-198`
and `:211-212`,
`QueueProgressVO.java:36`, `consumer.tsx:1299` and `:1309`).
2. New evidence added for the sentinel, found while checking the tool
contract: `rmq.group.detail`
already **requires** `brokerOffset`/`consumerOffset` and declares
`minimum: -1` for both
(`server/src/main/resources/tool-catalog/tools/group.yaml:265-266`
required, `:275-280`
properties), so `-1` is the lowest value that output schema accepts and a
nullable offset could
not have been expressed in it at all.
#### Citations re-read
| Site | Base `1ef5d860` | Branch |
|------|-----------------|--------|
| `TencentInstanceProvider.getGroupProgress` offsets | `.brokerOffset(0L)`
`:537`, `.consumerOffset(0L)` `:538` | `QueueProgressVO.UNKNOWN_OFFSET`
`:540-541` |
| `AliyunConverters.toQueueProgressRows` topic rows | `:195-196` (block
`:191-198`) | `:197-198` |
| `AliyunConverters.toQueueProgressRows` aggregate row | `:209-210` (block
`:206-212`) | `:211-212` |
| `consumer.tsx` progress columns | `offset.toLocaleString()` `:1296`,
`:1306` | `formatOffsetValue(offset)` `:1299`, `:1309` |
| `formatOffsetValue` / reset-preview guard | `:179-180` / `:222` |
unchanged |
#### Tests re-executed in the clean worktree
```
$ cd server && mvn -o test
-Dtest='AliyunConvertersLagTest,TencentInstanceProviderTest'
Tests run: 55, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS
$ cd server && mvn -o test
Tests run: 3153, Failures: 0, Errors: 17, Skipped: 0
```
The 17 errors are the MySQL-backed `*IntegrationTest` context loads: 7
surefire reports contain
`Failed to load ApplicationContext`, all caused by
`com.mysql.cj.jdbc.exceptions.CommunicationsException: Communications link
failure` — this checkout
has no MySQL. The same 17-error count appears on the base commit, and none
of them is in a provider,
group or tool-contract class.
```
$ cd web && npx vitest run src/pages/instance/__tests__/ConsumerPage.test.tsx
Test Files 1 passed (1)
Tests 37 passed (37)
$ cd web && npx vitest run src/pages/instance src/components
Test Files 16 passed (16)
Tests 261 passed (261)
$ cd web && npx tsc -b && npx eslint src/pages/instance/consumer.tsx
src/pages/instance/__tests__/ConsumerPage.test.tsx
(no output, exit 0)
```
#### Pre-fix behaviour re-demonstrated
New Java tests kept, only the two provider/converters files restored to
`origin/rocketmq-studio`
(the tests reference `QueueProgressVO.UNKNOWN_OFFSET`, which the fix
introduces, so the constant and
the tests stay):
```
[ERROR]
TencentInstanceProviderTest.getGroupProgressShouldReportUnknownQueueOffsetsTest
<<< FAILURE!
expected: -1L
but was: 0L
[ERROR]
AliyunConvertersLagTest.queueOffsetsShouldBeUnknownWhenOnlyTheTopicLagIsKnownTest
<<< FAILURE!
expected: -1L
but was: 0L
[ERROR] AliyunConvertersLagTest.aggregateRowOffsetsShouldBeUnknownTooTest
<<< FAILURE!
expected: -1L
but was: 0L
Tests run: 55, Failures: 3, Errors: 0, Skipped: 0
```
New component test kept, only `web/src/pages/instance/consumer.tsx` restored:
```
× renders an offset the provider cannot report as unavailable
AssertionError: expected [] to have a length of 2 but got +0
Tests 1 failed | 36 skipped (37)
```
#### Coverage note
Covered: the two cloud providers' progress rows, the aggregate fallback row
of Aliyun, and the
component rendering of the sentinel. Deliberately not changed, as stated in
the description: the
pseudo-broker name (`topic:<topic>`), `queueId = 0` of these aggregate rows,
and the summary cards
that count rows as queues — those are presentation decisions rather than
fabricated measurements,
and changing them would alter what the columns mean.
No commit was needed for this PR: the corrections were description-only.
--
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]
