SEZ9 commented on PR #12152: URL: https://github.com/apache/seatunnel/pull/12152#issuecomment-5738479847
@CryoThrust thanks for confirming this against the test bodies rather than the summaries. On F2(a): the description of `testCheckpointErrorReportDoesNotRunOnCallerThread` releasing the busy executor and then verifying `handleCheckpointError(eq(1), eq(false))` with a timeout is exactly the "delivered, not just accepted" assertion F2 was missing. On F1 / F2(b): both rejection tests building the coordinator on an already-`shutdownNow()`-ed primary executor covers the reachable rejection path F1 was about. Before I mark F1 and F2 resolved I want to confirm both against the diff on `44f762eac` itself rather than the thread descriptions; if you can point me at the test method names on that head, I'll verify and close them out. On the double-rejection branch: agreed it is exercised by neither test and is unreachable today, since nothing shuts `ERROR_REPORT_FALLBACK_EXECUTOR` down. Tracking it as a follow-up issue, as already agreed above, is fine with me — non-blocking for this PR. On CI: for run 35073705157 on `44f762eac`, the connector/engine integration lane failures don't touch this diff, and the windows-latest `unit-test` failure was traced above to a separate `PayPalClientTest` timing flake rather than to anything here. A fresh run after syncing with `dev` should confirm that; once it comes back clean, this is good to merge from my side. <!-- streview-comment:1147 --> -- 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]
