RFR: 8356438: Update OutputAnalyzer to optionally print process output as it happens [v2]

Alice Pellegrini duke at openjdk.org
Tue Jun 10 07:52:29 UTC 2025


On Mon, 9 Jun 2025 04:03:28 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> test/lib/jdk/test/lib/process/OutputBuffer.java line 150:
>> 
>>> 148:       this.p = p;
>>> 149:       logProgress("Gathering output");
>>> 150:       boolean verbose = Boolean.getBoolean("outputanalyzer.verbose");
>> 
>> Putting a system property at this level of the code kind of hides the functionality.
>> 
>> An alternative solution would be to have `OutputAnalyzer` constructor(s) that takes a boolean argument. That boolean could be set as needed by individual tests using the `test.debug` property already used by a lot of tests.
>
> But isn't it the person running the tests that wants to set this, not an inherent property of a test itself? Or are you envisaging enabling it at the test-level so the person running the tests doesn't have to do so? But then how does that affect the overall jtreg log content. ??

To add on this, I should mention that I noticed a lot of tests are using OutputAnalyzer indirectly, returned as a result of a utility function, for example `ProcessTools.execute{Process,Command)`


git grep -E "ProcessTools.execute((Process)|(Command))" | wc -l
     351


Instead of calling one of OutputAnalyzer's constructors (700 invocations, but many of them are with the Eager, string based and not process based, constructor, which we don't care about)


I'm not sure adding additional parameters also to that code is an ideal solution, I'd much prefer adding the command line parameter to the docs of the OutputAnalyzer class, so that one knows to enable it when running the single test through jtreg

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25587#discussion_r2137161838


More information about the core-libs-dev mailing list