[crac] RFR: 8377562: [CRaC] Add better logging to RemoteJmxTest [v2]

[Radim Vansa]

On Fri, 20 Feb 2026 09:54:41 GMT, Timofei Pushkin <tpushkin at openjdk.org> wrote:

>> Makes `RemoteJmxTest` log stderr of the child process.
>
> Timofei Pushkin has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
> 
>  - Ensure resources live long enough in some tests
>  - Merge remote-tracking branch 'openjdk-crac/crac' into remote-jmx-test
>  - Always capture output
>  - Improve process synchronization in ImageScoreTest
>  - Make timed waiting more robust
>  - Merge remote-tracking branch 'openjdk-crac/crac' into remote-jmx-test
>  - Refactor CracProcess to allow multi-waiting
>  - Add better logging to RemoteJmxTest

If it were not for the default check of the exit status I would go with 3) (return OA from `doRestore()` & friends)  but while the `toAnalyze` suffix still seems awkward to me ('analysis' is rather pompous word for string match) I won't have objections.

Minor comments below.

test/lib/jdk/test/lib/crac/CracBuilderBase.java line 48:

> 46:     private static final Class<CracTestArg> dummyWorkaround = CracTestArg.class;
> 47: 
> 48:     static void log(String fmt, Object... args) {

I am not so sure how useful is this wrapper in the end, the code could probably just `System.err.print...` .
But if we keep this, it should probably move to `CracProcess`.

test/lib/jdk/test/lib/crac/CracProcess.java line 81:

> 79:         @Override
> 80:         public void close() {
> 81:             future.cancel(true);

You should probably call `lines.notifyAll()` in here and throw an `InterruptedException` from  `waitForLine`.

test/lib/jdk/test/lib/crac/CracProcess.java line 215:

> 213:     }
> 214: 
> 215:     public void waitForStdout(Predicate<String> predicate, long timeoutSec) throws InterruptedException, EOFException, TimeoutException {

Do we really need to pass the timeout, or would some common-sense default suffice?

-------------

PR Review: https://git.openjdk.org/crac/pull/295#pullrequestreview-3840238923
PR Review Comment: https://git.openjdk.org/crac/pull/295#discussion_r2840117599
PR Review Comment: https://git.openjdk.org/crac/pull/295#discussion_r2840093420
PR Review Comment: https://git.openjdk.org/crac/pull/295#discussion_r2840135457