On Wed, 15 Sep 2021 00:34:08 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
> 8273921: Refactor NSK/JDI tests to create thread using factory
I've gotten through about 1/3 of the test plus the new files, so thought I'd
pass along my comments so far. Overall it looks good though.
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().
test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequestManager/deleteEventRequest/delevtreq002a.java
line 141:
> 139: class Thread1delevtreq002a extends JDITask {
> 140:
> 141: String tName = null;
Another case where tName was not removed.
test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequestManager/stepRequests/stepreq002a.java
line 142:
> 140: class Thread1stepreq002a extends JDITask {
> 141:
> 142: String tName = null;
Another case where tName was not removed.
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.
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());`
-------------
PR: https://git.openjdk.java.net/jdk/pull/5515
