SEPURI-SAI-KRISHNA commented on issue #12647: URL: https://github.com/apache/seatunnel/issues/12647#issuecomment-6042087457
Thanks. All three are worth checking against the branch rather than asserting, so I did. **Limited to the `TOPIC_NOT_EXIST` plus route-available branch.** Confirmed. The `continue` sits inside `if (e.getResponseCode() == ResponseCode.TOPIC_NOT_EXIST && topicRouteAvailable(adminClient, topic))`, and the `throw new RocketMqConnectorException(...)` immediately below it is untouched, as are the `MQBrokerException | RemotingException` and `InterruptedException` catches. Two existing tests pin that other failures still surface: `testCurrentOffsets_routeOutageSurfacesAsFailure` and `testCurrentOffsets_otherResponseCodeSurfacesEvenWhenRouteHealthy`. **Asserting the actual offset rather than a non-empty map.** Confirmed: `assertEquals(Collections.singletonMap(firstQueue, 42L), offsets)`. **Every topic skipped must stay empty.** You found a real gap. The PR description claimed `testCurrentOffsets_retryTopicMissingWhileRouteHealthyReturnsEmpty` pinned this. It does not: that test passes a single-topic list, so it only covers the single-topic cold start. Skipping instead of returning is precisely what makes the multi-topic case run the loop to the end, and nothing covered that. Added `testCurrentOffsets_everyTopicMissingRetryRouteStillReportsAColdStart`, a two-topic list where both topics answer that way, asserting the result is exactly empty. The description is corrected rather than left standing. So the new test is not oversold, here is what it does and does not do, measured. The all-skipped case also returns an empty map under the old `return`, so it does not catch the original defect and I am not offering it as doing so; it is a contract guard for the loop shape this change introduces. Restoring the `return` fails only the later-topic test, 1 of 8 in the class. Mutating the skip to treat an unresolved topic as committed-at-zero fails the new test and the single-topic one together, 2 of 8. Module is 26 tests, 0 failures, 0 errors. `spotless:check` clean with no reformatting. Keeping the issue open until #12648 merges, as you asked. -- 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]
