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