SEPURI-SAI-KRISHNA commented on PR #12701: URL: https://github.com/apache/seatunnel/pull/12701#issuecomment-6106548572
Thanks @DanielLeens and @SEZ9. **@SEZ9, the failing run you asked for:** https://github.com/SEPURI-SAI-KRISHNA/seatunnel/actions/runs/37759695509/job/113300595365 That is `rocketmq-connector-it (11, ubuntu-latest)` on #12495's head `4b2cd492e`, 87 tests with 2 failures, both in `testTextFormatSinkRocketMq`: invocation `[1]` read `<6>` and `[2]` read `<14>`. Worth pairing it with the sibling leg in the same run, `rocketmq-connector-it (8, ubuntu-latest)`, which **passed**. A deterministic break would have taken both legs, so the asymmetry is itself evidence for the race rather than for a broken assertion. **@DanielLeens, Issue 1 taken.** The six-line rationale and the wait call are now a single `awaitSinkRowsVisible(String topicName)` helper sitting next to `awaitTopicMaxOffset`, and the three tests each call it on one line. The rationale lives in the helper's Javadoc, including why retrying the read is not an option, so there is one copy rather than three. All three blocks were byte-identical before the change, so nothing was lost in consolidating them. **One correction to your review, in my favour and I would rather not keep the credit.** You wrote that `EXPECTED_SINK_ROWS` already documents that it must stay in sync with `row.num = 10`. It did not. The Javadoc read only "Rows the three fake-source sink jobs each write, asserted after the sink completes", which says nothing about the confs. It does now, naming all three files and spelling out that the visibility wait and the count assertion both read the constant, so changing one side alone would make the wait pass early or never finish. Also noted and left alone, since you flagged it as pre-existing: `getRocketMqConsumerData` still ends on the first empty five second poll. With the pre-wait in front of it the rows are known to be readable before the read starts, so I have not touched it in this PR. Verified after the change: `test-compile` and `spotless:check` clean on `connector-rocketmq-e2e`, no non-comment change to the three tests beyond the one-line call, and the diff is one file, +27 -22. Still no Docker on this machine, so CI remains the first execution of these tests. For what it is worth, at the previous head both `rocketmq-connector-it` legs passed, which is the signal I would want before merge rather than proof the flake is gone. -- 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]
