On Tue, 2 Feb 2021 04:17:39 GMT, Jie Fu <[email protected]> wrote:

>> test/jdk/com/sun/jdi/JdbOptions.java line 99:
>> 
>>> 97:         test("-connect",
>>> 98:                 
>>> "com.sun.jdi.CommandLineLaunch:vmexec=java,options=\"-client\" 
>>> \"-XX:+PrintVMOptions\""
>>> 99:                 + " -XX:+IgnoreUnrecognizedVMOptions"
>> 
>> You need to be careful when using `-xx:+IgnoreUnrecognizedVMOptions`, 
>> because it can result in not detecting typos in the option names, which 
>> could happen with this test in the future if any other mods are made. Other 
>> options here are to skip this part of the test if JFR is not present, or 
>> better yet don't make the test rely on JFR. I don't think there is a reason 
>> that it needs to. It seems other options could have been used for the 
>> testing. Perhaps @alexmenkov can comment on why it was done this way.
>
> Thanks @plummercj for your review.
> 
> Let's wait for @alexmenkov 's comments.
> Thanks.

Special handling for options with commas, quotes and single quotes was added 
many years ago for JFR (as it uses commas inside). The logic there is a bit 
tricky and I didn't want to brake existing functionality when I made some 
changes there, so I used "live" examples of JRF options in the test.
I don't know other VM options which use commas inside.

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

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

Reply via email to