SEZ9 commented on PR #11900: URL: https://github.com/apache/seatunnel/pull/11900#issuecomment-5629310950
Thanks for the update. Where things stand from my side: - **F1 (scope/title mismatch)** — resolved with the retitle and the narrowed description. Nothing further needed. - **F3 (missing lifecycle test)** — the pushed change to `JobHistoryServiceTest.java` adds `testShutdownRemovesListeners()`, which calls `jobHistoryService.shutdown()` directly. It does not go through `clearCoordinatorService()`, and nothing is mocked to make `resourceManager`/`eventProcessor`/`manager.close()` throw, so the ordering-under-throw scenario is still untested. Please add a test that drives `clearCoordinatorService()` with one of the preceding closes throwing and asserts the listeners are still removed. - The new test also reuses the shared `server.getCoordinatorService().getJobHistoryService()` instance that the other tests in the file rely on. Shutting it down in one test can affect the others; please use an independent `JobHistoryService` instance for the shutdown test. - Could you also confirm whether #11809 has been checked for overlap with this PR? Happy to do a fresh pass once those land. <!-- streview-comment:965 --> -- 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]
