On Wed, 30 Sep 2020 16:52:01 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Hi all,
>>
>> could you please review the patch which removes `PropertyResolvingWrapper`
>> from `vmTestbase/nsk/jvmti` tests? as
>> `jtreg` doesn't support spaces in the arguments and doesn't handle `"` in
>> any special ways, the patch also:
>> - `s/"-javaOpts=/-javaOpts="/`
>> - makes `nsk.jvmti.scenarios.general_functions.GF08` to use 2nd arg as
>> `verboseType` and 3rd and the rest args
>> concatenated as `phrase` and updates the tests accordingly
>> - removes spaces and surrounding `"` from `nsk.jvmti.test.property*`
>> - removes `"` surrounding `-agentlib:`, replaces spaces in `-agentlib` with
>> `,` and updates `ArgumentHandler` to treat
>> `,` (as well as ` ` and `~`) as options delimiters, so it's consistent w/
>> `jvmti_tools.cpp`
>>
>> testing: ✅ `vmTestbase/nsk/jvmti` on {linux,windows,macos}-x64
>
> 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
-------------
PR: https://git.openjdk.java.net/jdk/pull/370
