SEZ9 commented on issue #12647: URL: https://github.com/apache/seatunnel/issues/12647#issuecomment-6052093170
Thanks @SEPURI-SAI-KRISHNA, this is exactly the kind of verification I was hoping for. On the three points: - **Scope of the skip.** Good — keeping the `continue` strictly inside the `TOPIC_NOT_EXIST` + `topicRouteAvailable` branch, with the `throw new RocketMqConnectorException(...)` and the `MQBrokerException | RemotingException` / `InterruptedException` catches untouched, is the containment I wanted. Having `testCurrentOffsets_routeOutageSurfacesAsFailure` and `testCurrentOffsets_otherResponseCodeSurfacesEvenWhenRouteHealthy` already pin that other failures still surface is reassuring. - **Asserting the real offset.** `assertEquals(Collections.singletonMap(firstQueue, 42L), offsets)` is the right shape — it proves the earlier topic's offset survives rather than just that the map is non-empty. - **All-topics-skipped stays empty.** Appreciate you checking rather than trusting the description. You're right that a single-topic list only exercises the cold-start path and never runs the loop past the first skip. `testCurrentOffsets_everyTopicMissingRetryRouteStillReportsAColdStart` with a two-topic list closes that gap, and I agree with how you characterise it: a contract guard for the new loop shape, not a reproducer for the original defect. The honest framing (restoring the `return` fails only the later-topic test; treating an unresolved topic as committed-at-zero fails two) is useful and worth keeping in the PR description so reviewers know what each test does and does not catch. Remaining asks: 1. Push the corrected PR description so it no longer claims the single-topic test covers the all-skipped multi-topic case, and include the measured mutation notes you listed above. 2. Make sure the new test is in the PR branch alongside the earlier-topic offset test, and ping me on the PR once that's up so I can do the final pass. 3. Leave this issue open until the PR is merged into `dev`; we'll close it from there. <!-- streview-comment:1602 --> -- 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]
