[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR:8153989:Some SVC tests fail on compact2 due to an unnecessary test library dependency
From:       Mandy Chung <mandy.chung () oracle ! com>
Date:       2016-04-20 21:03:13
Message-ID: 524345D9-CA6F-425D-AB77-3C46ACDEF19F () oracle ! com
[Download RAW message or body]


> On Apr 20, 2016, at 9:06 AM, Alexander Kulyakhtin <alexander.kulyakhtin@oracle.com> \
> wrote: 
> Hi,
> 
> Could you, please, review this small tests-only fix:
> 
> CR: https://bugs.openjdk.java.net/browse/JDK-8153992 "Some SVC tests fail on \
>                 compact2 due to an unnecessary test library dependency"
> Webrev: http://cr.openjdk.java.net/~akulyakh/8153992/test/testlibrary/jdk/test/lib/ProcessTools.java.udiff.html
>  
> Before the fix the ProcessTools.getProcessId() used the \
> ManagementFactory.getRuntimeMXBean() API. The API is not available on compact2 and \
> below. Therefore the tests failed. 
> We are changing the ProcessTools.getProcessId() method to use the JDK 9 \
> Process.getPid(). This eliminates the unnecessary dependency making the tests pass \
> on compact2. 

This looks okay. But I see that getVmInputArguments calls \
ManagementFactory.getRuntimeMXBean.  So ProcessTools still has a dependency on \
java.management.

The jdk test library ProcessTools::getProcessId has been long ago to call \
Process::getPid and the method is changed to return long.  I thought that similar \
change would have been made in the hotspot test library at that time.

> I am not sure how acceptable it is to cast from long to int this change. If it is \
> not acceptable we can change the return type to long.  This however, will cause \
> massive changes throughout the hotspot tests which presently expect getProcessId() \
> to return int.

IMO it would be good to return long or have the callsite to call \
ProcessHandle.current().getPid().

Mandy


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic