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

[David Holmes]

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/InheritedChannelTest.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.
>>>