RFR: 8273921: Refactor NSK/JDI tests to create thread using factory

Chris Plummer cjplummer at openjdk.java.net
Sat Sep 18 01:37:51 UTC 2021


On Wed, 15 Sep 2021 00:34:08 GMT, Leonid Mesnik <lmesnik at 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


More information about the serviceability-dev mailing list