On Mon, 4 Mar 2024 23:23:04 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> This change is intended to address the segmentation fault issue that occurs
>> when jspawnhelper is called without arguments, and it includes an updated
>> test to verify the behavior in such cases.
>>
>> Existing tests passes since it does not check behavior without args.
>> After test update the test fails without
>>
>> if (argc != 2) {
>> shutItDown();
>> }
>>
>> code block
>>
>>> Test results: failed: 1
>>> Report written to
>>> /home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/test-results/jtreg_test_jdk_java_lang_ProcessBuilder_JspawnhelperProtocol_java/html/report.html
>>> Results written to
>>> /home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/test-support/jtreg_test_jdk_java_lang_ProcessBuilder_JspawnhelperProtocol_java
>>> Error: Some tests failed or other problems occurred.
>>> Finished running test
>>> 'jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java'
>>> Test report is stored in
>>> build/linux-x86_64-server-fastdebug/test-results/jtreg_test_jdk_java_lang_ProcessBuilder_JspawnhelperProtocol_java
>>>
>>> ==============================
>>> Test summary
>>> ==============================
>>> TEST TOTAL PASS FAIL
>>> ERROR
>>> jtreg:test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java
>>> >> 1 0 1
>>> >> 0 <<
>>> ==============================
>>> TEST FAILURE
>>
>>
>>
>> after added the code block back
>>
>> if (argc != 2) {
>> shutItDown();
>> }
>>
>>
>> updated test passes
>>
>>
>> lang/ProcessBuilder/JspawnhelperProtocol.d:/home/ec2-user/moe/ws/openjdk/jdk/test/jdk/java/lang/ProcessBuilder:/home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/test-support/jtreg_test_jdk_java_lang_ProcessBuilder_JspawnhelperProtocol_java/classes/0/test/lib
>> \\
>> -Xmx768m \\
>> -XX:MaxRAMPercentage=1.04167 \\
>> -Dtest.boot.jdk=/home/ec2-user/moe/soft/jdk/jdk-21 \\
>>
>> -Djava.io.tmpdir=/home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/test-support/jtreg_test_jdk_java_lang_ProcessBuilder_JspawnhelperProtocol_java/tmp
>> \\
>> -ea \\
>> -esa \\
>>
>> -Djava.library.path=/home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/images/test/jdk/jtreg/native
>> \\
>> com.sun.javatest.regtest.agent.MainWrapper
>> /home/ec2-user/moe/ws/openjdk/jdk/build/linux-x86_64-server-fastdebug/test-support/jtreg_test_jdk_java_lang...
>
> test/jdk/java/lang/ProcessBuilder/JspawnhelperProtocol.java line 111:
>
>> 109: if (p.exitValue() != 1)
>> 110: System.exit(ERROR + 2);
>> 111: System.exit(0);
>
> Its bad form to System.exit from not at the top level. Is that necessary.
This is in line with other tests in this file , e.g. see `parentCode()`.
I agree that code would be cleaner if the test is refactored to return error
code from the test methods and system.exit() in the main instead, but this
might be a separate pr done before or after this one.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18112#discussion_r1511953089
