RFR: 8352016: Improve java/lang/RuntimeTests/RuntimeExitLogTest.java [v2]

[KIRIYAMA Takuya]

On Tue, 18 Mar 2025 12:34:37 GMT, Mikhail Yankelevich <myankelevich at openjdk.org> wrote:

>> KIRIYAMA Takuya has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8352016: Improve java/lang/RuntimeTests/RuntimeExitLogTest.java
>
> test/jdk/java/lang/RuntimeTests/RuntimeExitLogTest.java line 52:
> 
>> 50:     private static final String TEST_JDK = System.getProperty("test.jdk");
>> 51:     private static final String TEST_SRC = System.getProperty("test.src");
>> 52:     private static final String NL = System.getProperty("line.separator");
> 
> Wouldn't it be easier to just use `System.lineSeparator();` here, what do you think? 
> 
> Also I'd personally rename it to `NEW_LINE` or `SEPARATOR`, as it took me a few read throughs to understand what does NL stand for, but thats minor.

I changed it as you said for readability.

> test/jdk/java/lang/RuntimeTests/RuntimeExitLogTest.java line 53:
> 
>> 51:     private static final String TEST_SRC = System.getProperty("test.src");
>> 52:     private static final String NL = System.getProperty("line.separator");
>> 53:     private static Object HOLD_LOGGER;
> 
> Nitpick: This is not used, could you please remove if there will be another revision.

The "HOLD_LOGGER" variable is not directly used but is necessary for test stability. It ensures that the ThrowingHandler remains attached to the "java.lang.Runtime" logger and is not garbage collected.

> test/jdk/java/lang/RuntimeTests/RuntimeExitLogTest.java line 174:
> 
>> 172:                     System.err.println(expectMessage.replaceAll("\\n", NL));
>> 173:                     System.err.println("---- Actual output begin");
>> 174:                     lines.forEach(l -> System.err.println(l));
> 
> Nitpick: `lines.forEach(System.err::println);` would be easier to read in my opinion, but it's fine as it is.

I see. I changed it as you said for readability.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2007132409
PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2007121689
PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2007134108