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

Mikhail Yankelevich myankelevich at openjdk.org
Fri Mar 21 14:33:09 UTC 2025


On Fri, 21 Mar 2025 09:01:01 GMT, KIRIYAMA Takuya <duke at openjdk.org> wrote:

>> 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.

Thank you

>> 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.

Thank you

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2007710195
PR Review Comment: https://git.openjdk.org/jdk/pull/24050#discussion_r2007710564


More information about the core-libs-dev mailing list