RFR: 8336332: Rework tests to avoid unrelated stderr output [v4]

[Kevin Rushforth]

On Mon, 15 Sep 2025 19:25:37 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> This PR removes unrelated `stderr` output in the headless test logs by redirecting it to an in-memory buffer.  Exceptions found in the buffer can be checked against the expected list.
>> 
>> In the case when any mismatch is detected, whether the type or the number of exceptions of particular type, the accumulated buffer gets dumped to `stderr` (without failing the test).
>> 
>> ## How To
>> 
>> To redirect stderr and later check the exceptions, surround your code with
>> 
>> `ErrorLoggingUtility.suppressStderr()` and either `ErrorLoggingUtility.checkStderr()` or `ErrorLoggingUtility.checkAndRestoreStderr()` (ideally, in the `finally` block).
>> 
>> To simply undo redirection, without checking, call  `ErrorLoggingUtility.restoreStderr()`.
>> 
>> To add the check to all the tests in the file, one can call the above mentioned methods inside  `@BeforeEach` and `@AfterEach`.
>> 
>> ## Miscellaneous
>> 
>> For reviewers' convenience, the first commit contains the main change, the second fixes the misspelt name of the utility class, the rest are trivial.
>> 
>> ## Questions
>> 
>> - should we fail the current test with `Assertions.fail()` in case of a mismatch?
>
> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   moved utility

I left a couple quick comments in line. I won't do a thorough review until you decide whether to create a new utility class (which would have the advantage of removing a number of unrelated changes in this PR).

> > 1. Since you are only addressing the `javafx.base`
> 
> so that's where it gets complicated: turns out the utility gets used in more tests (graphics, fxml, see #1905 ), so two things came up:
> 
> 1. the utility needs to be moved to another package
> 2. the utility probably needs to be decoupled from Logging, as it creates unneeded dependency with `--add-exports javafx.base/com.sun.javafx.binding=ALL-UNNAMED` in [8367567: Rework system tests to suppress unrelated stderr output #1905](https://github.com/openjdk/jfx/pull/1905)

Yes, I think this seems best. Rather than extend the existing javafx.base ErrorLoggingUtility, which is dependent on the (IMO rather hacky) Logging class in javafx.base itself, a separate test utility that just focuses on capturing and parsing stderr and has no dependencies on logging seems best.

> 3. perhaps the two PRs need to be merged into one larger one

Maybe that would be best, although I think it would be OK to keep them separate as long as you do a proof of concept that other modules can use it without needing changes to the exports, etc.

modules/javafx.base/src/test/java/test/util/ErrorLoggingUtility.java line 81:

> 79:     private static PrintStream stderr;
> 80:     private static AccumulatingPrintStream stderrCapture;
> 81:     private static int checked;

This is unused (only ever written) and can be removed.

modules/javafx.base/src/test/java/test/util/ErrorLoggingUtility.java line 127:

> 125:                 stderr.println("Mismatch in thrown exceptions:\n  expected=" + exp + "\n  observed=" + errors);
> 126:                 stderr.println(text);
> 127:             }

A mismatch should fail the test.

modules/javafx.base/src/test/java/test/util/ErrorLoggingUtility.java line 184:

> 182:             // catches lines starting with things like "Exception in thread "main" java.lang.RuntimeException:"
> 183:             "^" +
> 184:             "(?:" +

This might be easier to read and maintain with text blocks.

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

PR Review: https://git.openjdk.org/jfx/pull/1897#pullrequestreview-3231265800
PR Comment: https://git.openjdk.org/jfx/pull/1897#issuecomment-3300210311
PR Review Comment: https://git.openjdk.org/jfx/pull/1897#discussion_r2353364648
PR Review Comment: https://git.openjdk.org/jfx/pull/1897#discussion_r2353453548
PR Review Comment: https://git.openjdk.org/jfx/pull/1897#discussion_r2353554564