[GitHub] [kafka] C0urante commented on pull request #12410: MINOR: Remove unused ShutdownableThread class and ineffective ThreadedTest classes

[GitBox <gi...@apache.org>]

C0urante commented on PR #12410:
URL: https://github.com/apache/kafka/pull/12410#issuecomment-1215679995

   The `ThreadedTest` class was intended to detect uncaught errors on other threads than the main thread used for the test. In many cases (including the `WorkerSinkTaskThreadedTest` suite), this was unnecessary since tests that inherited from it were all single-threaded. The only exceptions are:
   - With the `ExactlyOnceWorkerSourceTaskTest` and `WorkerSourceTaskTest` suites, we were spinning up at most one additional thread per test via `ExecutorService::submit`, and every time we did that, we were already making sure to invoke `get` on the resulting `Future` on the main testing thread, which caused any uncaught errors to fail the test
   - With the `DistributedHerderTest` suite, we were also spinning up at most one additional thread per test via `ExecutorService::submit`, but not checking the resulting `Future`. I've added a check in this PR
   
   I toyed with the idea of keeping the `ThreadedTest` class as a reusable utility, but I don't think it'd save us a lot of trouble without also potentially tying our hands and preventing us from doing things like ensuring that a certain thread or task dispatched on a separate thread has completed at a certain point in the test.
   
   The `ShutdownableThread` (and its test) are not used (effectively) anywhere in the code base and are completely safe to remove.


-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org