On Thu, 28 Sep 2023 16:47:09 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:

>> The main.wrapper was the first name for jtreg test thread factory plugin. 
>> However, during integration of this feature in jtreg it was decided to use 
>> test.thread.factory name. So this fix just renames "main.wrapper" property 
>> to  "test.thread.factory" so it is more compliant with jtreg naming. Also, 
>> it makes more sense for tests when it is used to create other then main 
>> threads in test.
>> Testing: tier1-5.
>> Verified that "main.wrapper" is not used in test sources anymore.
>> 
>> I haven't rename DebugeeWrapperd and MainWrapper classes in JDI test 
>> frameworks because they are actually more main wrappers than thread 
>> factories.
>
> Leonid Mesnik has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fix

Marked as reviewed by amenkov (Reviewer).

test/hotspot/jtreg/vmTestbase/nsk/share/jdi/Binder.java line 711:

> 709:         String cmdline = classToExecute + " " + 
> ArgumentHandler.joinArguments(rawArgs, quote);
> 710: 
> 711:         if(System.getProperty("test.thread.factory") != null) {

Please add space after `if`

test/hotspot/jtreg/vmTestbase/nsk/share/jpda/DebugeeBinder.java line 353:

> 351:         args.add(jdwpArgs);
> 352: 
> 353:         if(System.getProperty("test.thread.factory") != null) {

Please add space after `if`

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

PR Review: https://git.openjdk.org/jdk/pull/15950#pullrequestreview-1649853506
PR Review Comment: https://git.openjdk.org/jdk/pull/15950#discussion_r1340758858
PR Review Comment: https://git.openjdk.org/jdk/pull/15950#discussion_r1340759866

Reply via email to