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

[Lindenmaier, Goetz]

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