On Fri, 17 Sep 2021 20:56:14 GMT, Chris Plummer <[email protected]> wrote:
>> Leonid Mesnik has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Renamed JDITask to NamedTask.
>
> test/hotspot/jtreg/vmTestbase/nsk/jdi/BreakpointRequest/addInstanceFilter/instancefilter004a.java
> line 154:
>
>> 152: static class Threadinstancefilter004a extends JDITask {
>> 153:
>> 154: String tName = null;
>
> Why did you keep tName? In previous similar tests you got rid of it and
> relied on getName().
Too late realized that it is redundant and forget to fix some tests.
Updated and checked that tName is used anymore.
> test/hotspot/jtreg/vmTestbase/nsk/share/jdi/JDITask.java line 26:
>
>> 24: package nsk.share.jdi;
>> 25:
>> 26: public abstract class JDITask implements Runnable {
>
> I was thinking maybe `NamedTask` or `NamedRunnable` might be a better and
> more generic name, unless you think you'll be enhancing this class, but
> `JDITask` is ok if you want to keep it.
Renamed to NamedTask.
> test/hotspot/jtreg/vmTestbase/nsk/share/jdi/JDIThreadFactory.java line 39:
>
>> 37: Thread t = threadFactory.newThread(task);
>> 38: t.setName(task.getName());
>> 39: return t;
>
> How about:
> `return newThread(task, task.getName());`
fixed
-------------
PR: https://git.openjdk.java.net/jdk/pull/5515