RFR(XL): 8157957 - ClassNotFoundException: jdk.test.lib.JDKToolFinder

[Christian Tornqvist]

Hi Dmitry,

>Do you have a plan to merge duplicate classes from different test
libraries[1]? It's another source of pain.
Yes, it's on the list of things to do. Merging the Hotspot test library and
fixing the @build issues was a first step, the JDK part would be the next
logical step, but in a future change :)

Thanks,
Christian

-----Original Message-----
From: Dmitry Samersoff [mailto:dmitry.samersoff at oracle.com] 
Sent: Tuesday, August 16, 2016 3:35 PM
To: Christian Tornqvist <christian.tornqvist at oracle.com>;
hotspot-dev at openjdk.java.net
Subject: Re: RFR(XL): 8157957 - ClassNotFoundException:
jdk.test.lib.JDKToolFinder

Christian,

Thank you for greet job.

I'm second to Coleen, - we should push these changes ASAP then fix remaining
problems one by one.

Do you have a plan to merge duplicate classes from different test
libraries[1]? It's another source of pain.

1.
chew:hs#cfind ProcessTools

./test/lib/share/classes/jdk/test/lib/process/ProcessTools.java
./jdk/test/lib/testlibrary/jdk/testlibrary/ProcessTools.java
./jaxp/test/javax/xml/jaxp/libs/jdk/testlibrary/ProcessTools.java
./hotspot/test/testlibrary/jdk/test/lib/ProcessTools.java

-Dmitry

On 2016-08-12 18:32, Christian Tornqvist wrote:
> Hi everyone,
> 
>  
> 
> Please review this fix for the CNFE issue we've had in the Hotspot 
> jtreg tests. The two big culprits for the intermittent CNFE are: 
> Duplicate classes on classpath with different content and @build tags 
> causing class files to be written in bad places.
> 
>  
> 
> There has been a lot of confusion around when there's a need to add 
> @build to a test. In general it turns out it has been overused, which 
> can lead to side effects. Here's an easy rule:
> 
>  
> 
> If you run only that test in a clean jtwork folder and it passes, then 
> there's no need for @build.
> 
>  
> 
> If it doesn't pass, then it might need an explicit @build, here are 
> some examples when it might be needed:
> 
>  
> 
> * If the class is used by ClassFileInstaller and this is invoked by 
> @run main ClassFileInstaller (sun.hotspot.Whitebox is an example of 
> this)
> 
> * If it's a class that that doesn't have reference from the Test 
> itself (if "javac Test" wouldn't compile it, you might need to 
> explicitly @build it)
> 
>  
> 
> The change includes:
> 
>  
> 
> * Removing all unnecessary @build tags, some of the CNFE was due to 
> use of it to compile classes in /test/lib or /testlibrary when having 
> different classpath (@library) set.
> 
> * Changed @build <testname> to @build sun.hotspot.Whitebox
> 
> * Moved /test/lib/share/classes/jdk/test/lib to /test/lib/jdk/test/lib
> 
> * Some of the classes in /hotspot/test/testlibrary was only used by 
> one or two tests, these classes shouldn't be part of the shared 
> testlibrary code and they were moved to the location of the test
> 
> * Merged/moved the classes in /hotspot/test/testlibrary to /test/lib
> 
> * Moved some of the /test/lib classes into packages
> 
> * Changed @library /testlibrary to @library /test/lib
> 
> * Changed imports from jdk.test.lib.* to explicit imports to help in 
> future refactoring
> 
>  
> 
> Almost all of the changes in here are in the jtreg @build and @library 
> tags, very little code has been touched. Copyright headers will be 
> updated before push.
> 
>  
> 
> Testing done: Ran the entire hotspot/test/ folder multiple times 
> locally and in RBT
> 
>  
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~ctornqvi/webrev/8157957/webrev.00/
> 
>  
> 
> A recently added test required me to fix that as well, generating and 
> uploading a new webrev of the Hotspot repo changes takes about 3h 
> though, so here's the diff for that file:
> 
>  
> 
> diff -r f37577c20a6b test/serviceability/sa/sadebugd/SADebugDTest.java
> 
> --- a/test/serviceability/sa/sadebugd/SADebugDTest.java Wed Aug 10 
> 21:02:14
> 2016 -0400
> 
> +++ b/test/serviceability/sa/sadebugd/SADebugDTest.java Fri Aug 12 
> +++ 11:25:54
> 2016 -0400
> 
> @@ -26,7 +26,7 @@
> 
>   * @summary Checks that the jshdb debugd utility sucessfully starts
> 
>   *          and tries to attach to a running process
> 
>   * @modules java.base/jdk.internal.misc
> 
> - * @library /test/lib/share/classes
> 
> + * @library /test/lib
> 
>   *
> 
>   * @run main/othervm SADebugDTest
> 
>   */
> 
>  
> 
> Bug:
> 
> https://bugs.openjdk.java.net/browse/JDK-8157957
> 
>  
> 
> Thanks,
> 
> Christian
> 
>  
> 
>  
> 


--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.