On Thu, 20 Apr 2023 18:40:52 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> ProcessTools.startProcess() creates process and read it's output error
>> streams. So the any other using of corresponding Process.getInputStream()
>> and Process.getErrorStream() doesn't get process streams.
>>
>> This fix preserve process streams content and allow to read it after process
>> completion. The another possible solution would be to throw exception when
>> user tries to read already drained streams to fail tests earlier. However it
>> complicates usage of ProcessTools.startProcess() methods.
>>
>> The regression test has been provided with issue.
>
> Leonid Mesnik has updated the pull request incrementally with one additional
> commit since the last revision:
>
> JStackStressTest.java updated.
test/jdk/sun/tools/jhsdb/JStackStressTest.java line 86:
> 84: } catch (InterruptedException e) {
> 85: }
> 86: OutputAnalyzer jshellOutput = new
> OutputAnalyzer(jShellProcess);
It's not clear to me how moving this is fixing anything.
test/jdk/sun/tools/jstatd/JstatdTest.java line 357:
> 355: assertEquals(stdout.size(), 1, "Output should contain one line");
> 356: assertTrue(stdout.get(0).startsWith("jstatd started"), "List
> should start with 'jstatd started'");
> 357: assertNotEquals(output.getExitValue(), 0,
Before your fix, was the "jstatd started" line being missed because of this bug.
test/lib/jdk/test/lib/process/ProcessTools.java line 750:
> 748: public InputStream getInputStream() {
> 749: try {
> 750: waitFor();
With this added `waitFor()` the assumption now is that the caller doesn't
intent to do incremental reads of the output as the process generates it. For
example, if the test were to send some command to the process and then want to
read the resulting output, and do this repeatedly, it won't able to use the
InputStream to do that.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13560#discussion_r1173141642
PR Review Comment: https://git.openjdk.org/jdk/pull/13560#discussion_r1173139688
PR Review Comment: https://git.openjdk.org/jdk/pull/13560#discussion_r1173146951
