Need reviewer: Makefile cleanup, tools testcase cleanup
kelly.ohair at oracle.com
Sun Jun 20 13:33:24 PDT 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
>> 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.
> 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.
> 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).
> Minor nit is that the changes to Metainf.java indent by 2 spaces
> instead of 4.
More information about the build-dev