RFR: 8023978: [TEST_BUG] launcher tests must exclude platforms without server vm

[David Holmes]

Hi Kumar,

I don't quite see how this gets the "jre" part of a JDK:

!         JAVA_JRE_BIN = new File(JAVAHOME, "bin").getAbsolutePath();
!
!         File libDir = (isSDK)
!                 ? new File((new File(JAVAHOME)).getParentFile(), "lib")
!                 : new File(JAVAHOME, "lib");
!         JAVA_LIB = libDir.getAbsolutePath();
!         JAVA_JRE_LIB = new File(JAVAHOME, "lib").getAbsolutePath();

do you assume JAVAHOME points to the jre directory ??

BTW these tests being in tools/launcher are defined to require a JDK by 
definition, but jtreg should be passed a compilejdk when testing a JRE, 
and the test could also use that for the compiler if it needs one.

Thanks,
David

On 15/11/2013 12:54 PM, Kumar Srinivasan wrote:
>
> Hi Mandy,
>
> Thanks for reviewing this, comments inlined.
>
>> Hi Kumar,
>>
>> On 11/14/2013 2:57 PM, Kumar Srinivasan wrote:
>>> Hello,
>>>
>>> Please review fixes to the launcher tests, which allows for missing
>>> server vm variant.
>>>
>>> All SE platforms have server vm, however  Embedded SE systems do
>>> not contain server, most of them have client, thus these
>>> tests needed to be compensated for that.
>>>
>>> http://cr.openjdk.java.net/~ksrini/8023978/webrev.0/
>>>
>>
>> In ExecutionEnvironment.java, I understand the fix in line 251 and 257
>> that address the bug reported.  However, I don't understand why doExec
>> calls to launch javaCmd has to specify "-server" or "client".   The
>> jdk should have a default VM and should the test use that?
>
> I was being over-zealous, and trying not to rely on jvm.cfg, note that
> jvm.cfg aliases -client to -server on 64-bit systems, I removed these
> changes, as it does not add anything.
>
>>
>> TestHelper.java
>>
>>  144         JAVA_JRE_LIB = new File((new
>> File(JAVAHOME)).getParentFile(),
>>  145                         (isSDK) ? "jre/lib" :
>> "lib").getAbsolutePath();
>>
>> If JAVA_HOME is a JDK build (or JRE image), does this give you
>> an incorrect path?  I expect getParentFile() is not needed in that case.
>
> You are right this is an error, fixed.
>
>> This is copy-n-paste in many places.  Does the test require
>> to run on JDK image (not JDK build)?
>
> The test runs on both j2sdk-image and JDK build, I use both
> during development with the old infra/jdk7.
>
> With the new infra I  use only images, because it is much faster now
> to create images, also on Solaris images are mandatory, because
> solaris symlinks exists only in the the images and there is a test
> which checks for these in ExecutionEnvironment.java.
> Note: these tests cannot be run on j2re-image as jtreg and the test itself
> needs the compiler, downside of converting to java tests.
>
>>
>>  189     private static boolean haveVmVariant(String type) {
>>  190         if (isWindows) {
>>  191             File vmDir = new File(JAVA_JRE_BIN, type);
>>  192             File jvmFile = new File(vmDir, JVM_DLL);
>>
>>
>> jvm.dll should be under jre/bin/client or jre/bin/server as in other
>> arch, right?
>
> Right windows no arch directory,output below.
> % find . -name jvm.dll
> ./j2re-image/bin/server/jvm.dll
> ./j2sdk-image/jre/bin/server/jvm.dll
> ./j2sdk-server-image/jre/bin/server/jvm.dll
>
> *nixes do have arch directory (i386, amd64, ppc, etc).
>
> A minor refactoring done.
>
> New  full webrev:
> http://cr.openjdk.java.net/~ksrini/8023978/webrev.1/index.html
>
> Delta webrev wrt. webrev.0
> http://cr.openjdk.java.net/~ksrini/8023978/webrev.1/webrev.delta/index.html
>
> Kumar
>
>>
>> Mandy
>