Re: RFR: 8306946: jdk/test/lib/process/ProcessToolsStartProcessTest.java fails with "wrong number of lines in OutputAnalyzer output" [v3]
On Thu, 27 Apr 2023 16:35:59 GMT, Leonid Mesnik wrote:
>> The ProcessTools.startProcess (...) has been updated to completely read
>> streams after process has been completed.
>> The test was updated to run 5 times with different number of lines and line
>> sizes.
>
> Leonid Mesnik has updated the pull request incrementally with one additional
> commit since the last revision:
>
> move buffers registration before pumping start point
Okay I _think_ I understand the changes now. I have one small suggestion below
but otherwise this seems okay.
Thanks.
test/lib/jdk/test/lib/process/ProcessTools.java line 190:
> 188: } catch (TimeoutException e) {
> 189: // continue execution, so wait() give a chance
> to write
> 190: } catch (InterruptedException | ExecutionException
> e) {
Probably also need to catch `CancellationException` here for good measure.
-
Marked as reviewed by dholmes (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/13683#pullrequestreview-1411942808
PR Review Comment: https://git.openjdk.org/jdk/pull/13683#discussion_r1184323176
Re: RFR: 8306946: jdk/test/lib/process/ProcessToolsStartProcessTest.java fails with "wrong number of lines in OutputAnalyzer output" [v3]
On Thu, 27 Apr 2023 16:35:59 GMT, Leonid Mesnik wrote: >> The ProcessTools.startProcess (...) has been updated to completely read >> streams after process has been completed. >> The test was updated to run 5 times with different number of lines and line >> sizes. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > move buffers registration before pumping start point The testing: - tier1 - tier5 to verify that nothing is brokens, - run jdk/test/lib/process/ProcessToolsStartProcessTest.java with 50 iterations instead of 5 on each platform 100 times - manually added some thread.sleep into startProcess and verify that not failures still appear - PR Comment: https://git.openjdk.org/jdk/pull/13683#issuecomment-1526061664
Re: RFR: 8306946: jdk/test/lib/process/ProcessToolsStartProcessTest.java fails with "wrong number of lines in OutputAnalyzer output" [v3]
On Thu, 27 Apr 2023 16:35:59 GMT, Leonid Mesnik wrote: >> The ProcessTools.startProcess (...) has been updated to completely read >> streams after process has been completed. >> The test was updated to run 5 times with different number of lines and line >> sizes. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > move buffers registration before pumping start point What Mach5 testing has been done with this fix? - PR Comment: https://git.openjdk.org/jdk/pull/13683#issuecomment-1526034233
Re: RFR: 8306946: jdk/test/lib/process/ProcessToolsStartProcessTest.java fails with "wrong number of lines in OutputAnalyzer output" [v3]
> The ProcessTools.startProcess (...) has been updated to completely read > streams after process has been completed. > The test was updated to run 5 times with different number of lines and line > sizes. Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision: move buffers registration before pumping start point - Changes: - all: https://git.openjdk.org/jdk/pull/13683/files - new: https://git.openjdk.org/jdk/pull/13683/files/10f1c5c2..d02b889a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=13683&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13683&range=01-02 Stats: 28 lines in 1 file changed: 10 ins; 9 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/13683.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13683/head:pull/13683 PR: https://git.openjdk.org/jdk/pull/13683
