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

[Lindenmaier, Goetz]

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