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

[David Holmes]

Hi Goetz,

Overall this looks okay to me. A few nits below ...

On 7/01/2019 11:07 pm, Lindenmaier, Goetz wrote:
> Hi,
> 
> Different operating systems use different names for the environment variable
> that contains the search paths for native libraries.  This path is used in
> a row of tests. A switch over all OSes is needed to find out the proper variable
> name in each test using it.
> 
> This change introduces a central function
> Platform.sharedLibraryPathVariableName()
> that returns "LD_LIBRARY_PATH", "DYLD_LIBRARY_PATH", "PATH" or
> "LIBPATH" depending on the current OS.
> This change also adapts all usages of these variables in the tests to call
> this function.
> Because of the change to KDC.java I had to add @library /test/lib
> to much more tests than where I had to do the underlying change.

Ouch! that was unpleasant. :(

> The change also replaces local checking for path separators by
> File.pathSeparator in jdk/com/sun/jdi/PrivateTransportTest.java.
> 
> The change depends on "8215975: [testbug] Adapt nsk tests to
> the PPC, S390 and AIX platforms." which will be moved from jdk12
> to jdk soon.
> 
> Please review:
> http://cr.openjdk.java.net/~goetz/wr19/8216265-PathVar/01/

test/hotspot/jtreg/gtest/GTestWrapper.java

  75             env.put(pathVar, path + ":" + ldLibraryPath);

Shouldn't ":" be File.pathSeparator?

---

test/jdk/java/nio/channels/spi/SelectorProvider/inheritedChannel/InheritedChannelTest.java

Copyright year needs updating.

---

test/jdk/java/nio/channels/spi/SelectorProvider/inheritedChannel/InheritedChannelTest.java

70     private static final Path pathEnvVar

The variable isn't an env var, it's just a path - I suggest libraryPath.

  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.

---

test/jdk/tools/launcher/JliLaunchTest.java

  57             env.compute(pathEnvVar, (k, v) -> (v == null) ? libdir 
: libdir + ":" + v);

Shouldn't ":" be File.pathSeparator?

---

test/jdk/tools/launcher/Test7029048.java

  39 import jdk.test.lib.Platform;

Why do you need this?

---

test/jdk/vm/JniInvocationTest.java

This is a Mac only test so no changes needed.

---

  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.

Thanks,
David

> Best regards,
>    Goetz.
>