RFR: 8254129: IR Test Framework to support regex-based matching on the IR in JTreg compiler tests [v2]

[Igor Ignatyev]

On Mon, 19 Apr 2021 09:42:04 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> test/lib/jdk/test/lib/hotspot/ir_framework/TestFramework.java line 407:
>> 
>>> 405:                 start(null);
>>> 406:             } catch (TestVMException e) {
>>> 407:                 System.err.println("\n" + e.getExceptionInfo());
>> 
>> you shouldn't use "\n", as it might not be the right line-separator. you can either do:
>> Suggestion:
>> 
>>                 System.err.println();
>>                 System.err.println(e.getExceptionInfo());
>> 
>> or
>> Suggestion:
>> 
>>                 System.err.printf("%n%s%n", e.getExceptionInfo());
>
> I will address all `\n` in a separate commit as there are a lot of them. Could I also just use `System.lineSeparator()` anywhere where I'm currently using `\n` like  `System.err.println(System.lineSeparator() + e.getExceptionInfo()`)? Might be easier when I'm using `\n` with a `StringBuilder`, for example.

sure you case use `System.lineSeparator()`, it's just a matter of personal choice/style

>> test/lib/jdk/test/lib/hotspot/ir_framework/TestFramework.java line 804:
>> 
>>> 802:             System.err.println("--- OUTPUT TestFramework flag VM ---");
>>> 803:             System.err.println(flagVMOutput);
>>> 804:             throw new RuntimeException("\nTestFramework flag VM exited with " + exitCode);
>> 
>> what's the reason for `\n` in the begging of this exception message?
>
> I guess it's not necessary. Removed it.

�� . I noticed that you have leading `\n` in other exceptions, do you plan to remove it from there as well?

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

PR: https://git.openjdk.java.net/jdk/pull/3508