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

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Wed Jan 9 10:34:24 UTC 2019


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


More information about the hotspot-dev mailing list