On Wed, 30 Sep 2020 16:46:56 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/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 string opinion here, I can go
w/ that alternative (or any other if you have more suggestions)

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

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

Reply via email to