Need reviewer: Makefile cleanup, tools testcase cleanup

Kelly O'Hair kelly.ohair at
Sun Jun 20 20:33:24 UTC 2010

On Jun 20, 2010, at 11:21 AM, Alan Bateman wrote:

> Kelly O'Hair wrote:
>> 6960853: Cleanup makefiles, remove unused vars etc.
>> 6962617: Testcase changes, cleanup of problem list for jdk_tools  
>> targets
>> Misc cleanup and matching openjdk6 where it can.
>> The jdk_tools2 target in jdk/test/Makefile is now run in jtreg - 
>> samevm mode and all
>> the jdk_tools* tests have been removed from the ProblemList.txt file.
>> Marked 3 tests with @ignore and a bugid.
>> -kto
> I've looked through the changes.  Mostly looks fine to me but I do  
> have a few comments.
> In test/sun/jvmstat/testlibrary/, the checkPort function is  
> using netstat without any parameters. I think this means it won't  
> list listener sockets and so freePort might return a port that is in  
> use (maybe use "netstat -a"?).

Good idea, will change to netstat -a

> The tests in test/sun/tools/jstatd seem to be using the pid of the  
> launched jstatd processes. Will this work with MKS? I think it  
> creates an intermediate shell so we have to do a bit of work to find  
> the actual pid.

The test works with JPRT, which is using MKS. I'm suspecting this was  
an old MKS issue maybe?
Anyway, the logic of scanning jps output for a Jstatd does not work in  
general, other tests
running make the test fail because it finds more than one PID.

> It's a shame to see the @ignore tag added to test/com/sun/tools/ 
> attach/ It's the main unit test for this API so we  
> loose test coverage if this test isn't run. Is it only Windows 2000  
> that this is failing? Could it be added to the problem list to  
> exclude for windows-5.0 only?

I'll change the testcase to detect windows 2000, and just skip it  
then, check out my new webrev.

> As you are editing the problem list, would you mind removing sun/ 
> management/jmxremote/bootstrap/ I forgot to  
> remove it when fixing 6950927 a few weeks ago. Another edit in this  
> area is to add javax/management/loading/LibraryLoader/ 
> (6959636).


> Minor nit is that the changes to indent by 2 spaces  
> instead of 4.



> -Alan.

More information about the build-dev mailing list