RFR(M): 8216265: [testbug] Introduce Platform.sharedLibraryPathVariableName() and adapt all tests.

[David Holmes]

Hi Goetz,

We are seeing two test failures after this push:

Test: sun/security/krb5/auto/LoginProc.java

C:\\ade\\mesos\\work_dir\\jib-master\\install\\jdk13-jdk.134\\src.full\\open\\test\\jdk\\sun\\security\\krb5\\auto\\KDC.java:24: 
error: package jdk.test.lib does not exist
import jdk.test.lib.Platform;
                    ^
Test: 
java/nio/channels/spi/SelectorProvider/inheritedChannel/InheritedChannelTest.java

/scratch/opt/mach5/mesos/work_dir/jib-master/install/jdk13-jdk.134/src.full/open/test/jdk/java/nio/channels/spi/SelectorProvider/inheritedChannel/InheritedChannelTest.java:101: 
error: cannot find symbol
         String pathVar = Platform.sharedLibraryPathVariableName();
                          ^
   symbol:   variable Platform
   location: class InheritedChannelTest

---

https://bugs.openjdk.java.net/browse/JDK-8217017

If you can't fix promptly I will do so in next couple of hours.

Thanks,
David



On 15/01/2019 1:24 am, Lindenmaier, Goetz wrote:
> Hi Martin,
> 
> thanks for reveiwing.
> 
> I'll fix the indentation to 4 before pushing.
> 
> Best regards,
>    Goetz.
> 
>> -----Original Message-----
>> From: Doerr, Martin
>> Sent: Montag, 14. Januar 2019 16:13
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; David Holmes
>> <david.holmes at oracle.com>; 'hotspot-dev at openjdk.java.net' <hotspot-
>> dev at openjdk.java.net>; gary.adams at oracle.com
>> Subject: RE: RFR(M): 8216265: [testbug] Introduce
>> Platform.sharedLibraryPathVariableName() and adapt all tests.
>>
>> Hi Götz,
>>
>> do we have an indentation rule for " .toAbsolutePath().toString();" in
>> JliLaunchTest.java?
>>
>> Nice cleanup! Looks good.
>>
>> Best regards,
>> Martin
>>
>>
>> -----Original Message-----
>> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of
>> Lindenmaier, Goetz
>> Sent: Mittwoch, 9. Januar 2019 13:52
>> To: David Holmes <david.holmes at oracle.com>; 'hotspot-
>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>;
>> gary.adams at oracle.com
>> Subject: RE: RFR(M): 8216265: [testbug] Introduce
>> Platform.sharedLibraryPathVariableName() and adapt all tests.
>>
>> Hi David,
>>
>> I fixed these locally.
>>
>> Best regards,
>>    Goetz.
>>
>>
>>
>>> -----Original Message-----
>>> From: David Holmes <david.holmes at oracle.com>
>>> Sent: Mittwoch, 9. Januar 2019 13:28
>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; 'hotspot-
>>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>;
>>> gary.adams at oracle.com
>>> Subject: Re: RFR(M): 8216265: [testbug] Introduce
>>> Platform.sharedLibraryPathVariableName() and adapt all tests.
>>>
>>> Hi Goetz,
>>>
>>> On 9/01/2019 8:34 pm, Lindenmaier, Goetz wrote:
>>>> Hi David,
>>>>
>>>> thanks for looking at my change.
>>>> It was asked for by Gary when he reviewed
>>> https://bugs.openjdk.java.net/browse/JDK-8215975
>>>>
>>>> New webrev:
>>>> http://cr.openjdk.java.net/~goetz/wr19/8216265-PathVar/02-
>> incremental/
>>>
>>> Looks good. Two further pre-existing nits spotted:
>>>
>>> test/hotspot/jtreg/gtest/GTestWrapper.java
>>>
>>> !  * Copyright (c) 2016, 2019 Oracle
>>>
>>> Need a comma after 2019.
>>>
>>> Ditto for:
>>>
>>>
>> test/jdk/java/nio/channels/spi/SelectorProvider/inheritedChannel/Inherited
>>> ChannelTest.java
>>>
>>> Actually I now see quite a number of files missing the comma <sigh> so
>>> I'll file a general bug to fix that.
>>>
>>> Thanks,
>>> David
>>>
>>>
>>>> http://cr.openjdk.java.net/~goetz/wr19/8216265-PathVar/02/
>>>>
>>>> See my comments inline below.
>>>>
>>>> Best regards,
>>>>     Goetz.
>>>>
>>>>> test/hotspot/jtreg/gtest/GTestWrapper.java
>>>>>
>>>>>     75             env.put(pathVar, path + ":" + ldLibraryPath);
>>>>>
>>>>> Shouldn't ":" be File.pathSeparator?
>>>> Fixed.
>>>>
>>>>>
>>>
>> test/jdk/java/nio/channels/spi/SelectorProvider/inheritedChannel/Inherited
>>>>> ChannelTest.java
>>>>>
>>>>> Copyright year needs updating.
>>>> Done.
>>>>
>>>>>
>>>
>> test/jdk/java/nio/channels/spi/SelectorProvider/inheritedChannel/Inherited
>>>>> ChannelTest.java
>>>>>
>>>>> 70     private static final Path pathEnvVar
>>>>>
>>>>> The variable isn't an env var, it's just a path - I suggest libraryPath.
>>>> A cleanup not directly related.  But makes sense, done.
>>>>
>>>>>     101
>>>>> System.out.println(Platform.sharedLibraryPathVariableName() + "=" +
>>>>> pathEnvVar);
>>>>>     ...
>>>>>     114         env.put(Platform.sharedLibraryPathVariableName(),
>>>>> pathEnvVar.toString());
>>>>>
>>>>> I suggest storing the name in a local to avoid the second call.
>>>> Done.
>>>>
>>>>> test/jdk/tools/launcher/JliLaunchTest.java
>>>>>
>>>>>     57             env.compute(pathEnvVar, (k, v) -> (v == null) ? libdir
>>>>> : libdir + ":" + v);
>>>>>
>>>>> Shouldn't ":" be File.pathSeparator?
>>>> This is because there is anyways a switch about the OS.
>>>> Did some more cleaning up.
>>>>
>>>>> test/jdk/tools/launcher/Test7029048.java
>>>>>
>>>>>     39 import jdk.test.lib.Platform;
>>>>>
>>>>> Why do you need this?
>>>> Removed.
>>>>
>>>>> test/jdk/vm/JniInvocationTest.java
>>>>>
>>>>> This is a Mac only test so no changes needed.
>>>> I would like to change this anyways. I think this makes
>>>> it look more consistent.
>>>>
>>>>>     test/lib/jdk/test/lib/Platform.java
>>>>>
>>>>> The javadoc comments is unnecessary as we don't generate javadoc
>> here.
>>> I
>>>>> see you copied the preceding sharedLibraryExt() style. The @return is
>>>>> superfluous.
>>>> Changed. Better?
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Best regards,
>>>>>>      Goetz.
>>>>>>