On Thu, 7 Mar 2024 16:29:12 GMT, Elif Aslan <d...@openjdk.org> wrote:

>> This change is intended to address the segmentation fault issue that occurs 
>> when jspawnhelper is called without arguments,.
>> There is a new test added  to verify the behavior in such cases.
>> 
>> `[ec2-user@ip-172-16-0-10 jdk]$ make CONF=linux-x86_64-server-fastdebug test 
>> TEST=test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java`
>> 
>> 
>> 
>> ==============================
>> Test summary
>> ==============================
>>    TEST                                              TOTAL  PASS  FAIL ERROR
>>    jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java
>>                                                          1     1     0     0
>> ==============================
>> TEST SUCCESS
>
> Elif Aslan has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Use outputanalyzer

test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java line 238:

> 236:         }
> 237:     }
> 238: }

This looks like a leftover, please revert.

test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java line 46:

> 44: 
> 45:     private static void jspawnhelperWithNArgs(int nArgs) throws Exception 
> {
> 46:         System.out.println("Running jspawnhelper with "+nArgs+" args");

Style: spaces around `+` operator.

test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java line 47:

> 45:     private static void jspawnhelperWithNArgs(int nArgs) throws Exception 
> {
> 46:         System.out.println("Running jspawnhelper with "+nArgs+" args");
> 47:         String[] args = new String[nArgs +1];

Style: spaces around `+` operator.

test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java line 54:

> 52:         if (!p.waitFor(TIMEOUT, TimeUnit.SECONDS)) {
> 53:             throw new RuntimeException("jspawnhelper timed out after " + 
> TIMEOUT + " seconds");
> 54:         }

I don't think we need to wait here, the `shouldHaveExitValue` waits for us.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1516472478
PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1516472833
PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1516477172
PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1516476881

Reply via email to