On Wed, 17 Jul 2024 20:00:18 GMT, ExE Boss <d...@openjdk.org> wrote: >>> waitFor can be overridden by pre-24 subclasses to provide a better >>> implementation while ... >> >> It doesn't really make sense to extend Process, except maybe for mocking or >> other testing. Process is really for JDK implementations, it's just >> historical the constructor is public. Just saying that the compatibility >> concerns with adding methods aren't significant here. > > This method needs to be overridden in > `test/lib/jdk/test/lib/process/ProcessTools.java` to call > `ProcessTools.ProcessImpl::waitForStreams`.
I don't think current `ProcessTools.startProcess()` even calls `waitFor(long, TimeUnit)` let alone `waitFor(Duration)`. It is polling the process by itself to implement timeout. But I agree it is a good measure to add the override there, for the test to use it in the future. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20220#discussion_r1681801768
