Need reviewer: Makefile cleanup, tools testcase cleanup

[Kelly O'Hair]

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
>>
>> http://cr.openjdk.java.net/~ohair/openjdk7/test-changes/webrev/
>>
>> 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/util.sh, 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/BasicTests.sh. 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.

http://cr.openjdk.java.net/~ohair/openjdk7/test-changes/webrev/

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

Done.

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

Fixed.

-kto

>
> -Alan.