RFR: 8252003: remove usage of PropertyResolvingWrapper in vmTestbase/nsk/jvmti

[Chris Plummer]

On Wed, 30 Sep 2020 17:10:25 GMT, Igor Ignatyev <iignatyev at openjdk.org> wrote:

>> test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/ArgumentHandler.java line 157:
>> 
>>> 155:             return;
>>> 156:
>>> 157:         StringTokenizer st = new StringTokenizer(optionString, " ~,");
>> 
>> I don't see jvmti_tools.cpp changes as part of this PR. Can you elaborate on why this change is needed now.
>
> `jvmti_tools.cpp` wasn't changed by this PR, `jvmti_tools.cpp` always accepted ` `, `~` and/or `,` as delimiters[[*]].
> as jtreg splits arguments by space symbol, we can't use ` ` in `-agentlib:`, so we had to change the delimiter used
> there to either `,` or `~`, `nsk/share/jvmti/ArgumentHandler` takes these options (via `getAgentOptionsString`) and
> then parse in `parseOptionString`. but `parseOptionString` was using `StringTokenizer` w/ default delimiters, ie `
> \t\n\r\f`, and as a result it wasn't splitting options correctly after we changed the delimiter used in tests'
> `-agentlib`. in other words, this change is just fixing an old bug which we didn't care before as we always used ` ` in
> this agent's options.   [*]:
> https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp#L224

I guess a better question would be is where is ~ being used in the arguments?

>> test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/general_functions/GF08/gf08t003/TestDescription.java line 62:
>> 
>>> 60:  *      nsk.jvmti.scenarios.general_functions.GF08.gf08t003
>>> 61:  *      jni
>>> 62:  *      Registering JNI native method
>> 
>> The motivation for this change isn't obvious to me, and seems less readable than the original code.
>
> the reason I had to this change is that jtreg doesn't handle `"` in any special way, so `gf08t003
> nsk.jvmti.scenarios.general_functions.GF08.gf08t003 "Registering JNI native method" jni` is passed to `gf08t` as
> [`gf08t003`, `nsk.jvmti.scenarios.general_functions.GF08.gf08t003`, `"Registering`, `JNI`, `native`, `method"`, `jni`].
> the way I fixed that was by changing the order of `phrase`, `verboseType`, so `verboseType` is now 2nd arg, and
> `phrase` 3rd and the rest of args joined.  the alternative way would be to teach `gf08t` about `"` and make it join
> arguments until it gets an even number of `"` and remove them, this, from my PoV, would be a bit more confusing. if
> you, however, are of strong opinion here, I can go w/ that alternative (or any other if you have more suggestions)

So what code used to handle passing quoted arguments? Is this something that PropertyResolvingWrapper used to do?

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

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