RFR (S) 8014905: Some hotspot tests should be updated to divide test jdk and compile jdk

[David Holmes]

On 1/06/2013 5:26 AM, Christian Tornqvist wrote:
> The problem here is how embedded runs these tests, they don't have a
> full JDK in test.jdk, instead they have the full JDK (same version
> apparently) as compile.jdk. Which means that these tests would not be
> able to run there if they were not using compile.jdk for finding
> anything other than "java".

This is not really related to Embedded per se, the problem is that 
someone has decided to use the existing test setup to test a JRE (in 
this case a Compact profile) instead of a JDK. A JRE can not be used for 
a lot of test suites.

> I think the assumption we have to do is that compile.jdk is the same
> version of JDK but with all the tools available while test.jdk may only
> have the "java" launcher available. In most use cases these will be
> pointing to the exact same place but for embedded testing they are not.

For a JRE they can not be the same.

I am unclear on how jtreg defines these things. Obviously test.jdk is 
the JDK under test. But what is compile.jdk meant to be, other than 
obviously a place to find javac. Is the expectation that this yields a 
stable javac from the JDK running jtreg, or is it acceptable to yield 
the javac from the JDK under test? You statement above indicates you 
think it was meant to be the JDK under test. I assumed it was meant to 
be a stable JDK. Perhaps we need a stable.jdk variable here?

> So for all uses of "java" we should use test.jdk and for all uses of any
> other tool we should use compile.jdk unless we have a very good reason
> for not doing so. Does this make sense?

Only if compile.jdk is really a stable JDK that is not the JDK under 
test. Using the new API's you've added a test can now ask for either a 
stable binary or a testing binary - which is good. Your change to the 
defaults seems okay now that everything has been changed consistently. 
But we still need to verify that tests are asking for the right kind of 
binary!

As I wrote below there is a problem for gc/TestVerifyDuringStartup.java:

    System.out.print("Testing:\n" + JDKToolFinder.getJDKTool("java"));
    ...
    ProcessBuilder pb =
       ProcessTools.createJavaProcessBuilder(vmOpts.toArray(new 
String[vmOpts.size()]));

as we will now report we are testing the compile.jdk/java but test 
test.jdk/java!

The other uses of JDKToolFinder are mainly for jcmd, and again I don't 
know for sure whether these expect to be running a stable jcmd or jcmd 
from test.jdk. If the latter then these changes break these tests!

Everything using createJavaProcess remains correct - java comes from 
test JDK.

There is only one usage I see of JDKToolLauncher:

./gc/metaspace/ClassMetaspaceSizeInJmapHeap.java:        JDKToolLauncher 
jmap = JDKToolLauncher.create("jmap")

but this will need to be corrected as it is supposed to be running jmap 
from the test.jdk.

David
-----

> Thanks,
> Christian
>
> On 2013-05-23 22:21, David Holmes wrote:
>> Hi Christian,
>>
>> On 24/05/2013 6:22 AM, Christian Tornqvist wrote:
>>> Hi everyone,
>>>
>>> Could I please have some reviews for this small test (library) fix. The
>>> following tests:
>>>
>>> gc\TestG1ZeroPGCTJcmdThreadPrint.java
>>>
>>> gc\TestVerifyDuringStartup.java
>>
>> This test will need modifying as it expects getJDKTool("java") to
>> return the JVM being tested.
>>
>> Everything else is running jcmd but I'm not clear if they simply use
>> jcmd as a mechanism in the test or whether they actually expect to be
>> testing jcmd from the VM under test?
>
>> Overall I'm not at all sure that your change is the best thing to do.
>> The utilities need to support both compile.jdk and test.jdk, but it
>> isn't clear to me that your selected defaults are right. For example
>> you changed ProcessTools.createJavaProcessBuilder to always use the
>> test.jdk, which is probably normally what is wanted, but sometimes you
>> might want the compile.jdk. Conversely you didn't change
>>
>> ./testlibrary/com/oracle/java/testlibrary/JDKToolLauncher.java
>>
>> so it always launches from the compile.jdk when some tests might want
>> to use it for the test.jdk.
>>
>> I think we need to support both forms in all the utilities. And I
>> think all the tests would ideally be specific about which they want to
>> use - but that is a lot of changes. But defaults need to be consistent
>> otherwise people will just be confused.
>>
>> Thanks,
>> David
>> -----
>>
>>> runtime\NMT\BaselineWithParameter.java
>>> runtime\NMT\JcmdScale.java
>>> runtime\NMT\JcmdWithNMTDisabled.java
>>>
>>> runtime\NMT\MallocTestType.java
>>>
>>> runtime\NMT\ShutdownTwice.java
>>>
>>> runtime\NMT\SummaryAfterShutdown.java
>>>
>>> runtime\NMT\SummarySanityCheck.java
>>>
>>> runtime\NMT\ThreadedMallocTestType.java
>>>
>>> runtime\NMT\ThreadedVirtualAllocTestType.java
>>>
>>> runtime\NMT\VirtualAllocTestType.java
>>>
>>> runtime\RedefineObject\TestRedefineObject.java
>>>
>>> use the JDKToolFinder.getJDKTool() method to get the path to different
>>> tools in JDK\bin. This only works if the Java used in the test is a full
>>> JDK build. If the Java used for testing is a JRE build the JDK tools
>>> will be available as compile.jdk instead of test.jdk.
>>>
>>> The solution was to simply change the getJDKTool() method to use the
>>> “compile.jdk” property to get the path to the tools (this will be set by
>>> jtreg to the same value as test.jdk if ‘-compilejdk’ is not specified on
>>> the command line). I’ve also added getTestJDKTool() and
>>> getCompileJDKTool() methods so that the tools can be explicitly grabbed
>>> from either location if necessary.
>>>
>>> Webrev can be found at:
>>> http://cr.openjdk.java.net/~ctornqvi/webrev/8014905/webrev.00/
>>>
>>> Bug (not yet visible at time of posting):
>>> http://bugs.sun.com/view_bug.do?bug_id=8014905
>>>
>>> Testing done was to run the above mentioned tests with jtreg using a JRE
>>> and –compilejdk pointing to a JDK and then again without –compilejdk but
>>> using a JDK to test.
>>>
>>> Thanks,
>>> Christian
>>>
>