[foreign] RFR 8216284: Make sure test jars are deleted before/after tests

[Sundararajan Athijegannathan]

This change looks good.

PS. I guess the issue of deleting jar file-in-use is Windows specific. 
That's why it was never seen on other platforms.

-Sundar

On 08/01/19, 9:28 PM, Jorn Vernee wrote:
> Hi,
>
> I was seeing some errors while running the JextractToolProviderTest 
> test, but the test still passed. The errors were from the test jars 
> not being able to be deleted because some process was using the file. 
> It turns out this was due to JextractToolRunner::loadClass creating a 
> URLClassLoader to load classes from the jar, but the classloader never 
> being closed causing the file to still be in use when trying to delete 
> it. This was throwing IOExceptions which were being printed, but 
> otherwise ignored.
>
> With all the errors being thrown and ignored I wasn't sure whether I 
> could trust the outcome of the test. Different tests could also 
> possibly interfere with each other if they use the same jar file name 
> and the jar doesn't get deleted after each test.
>
> I've solved the problem by replacing the loadClass method with a 
> classLoader method which returns an AutoCloseable Loader through which 
> the classes can be loaded. The close() method on Loader closes the 
> class loader. I have also turned the errors I was seeing into hard 
> errors to make sure that the tests fail if an IOException occurs 
> unexpectedly. To make that possible I switched 
> JextractToolRunner::deleteFile to using Files.deleteIfExtists instead 
> of delete so that an error will not occur when it is called at the 
> start of a test but there's nothing to delete.
>
> JextractToolRunner::classLoader is called with a list of Paths to use 
> as class path entries e.g.:
>
>     Path helloJar = getOutputFilePath("hello.jar");
>     deleteFile(helloJar); // make sure jar does not exist at start of 
> test
>     Path helloH = getInputFilePath("hello.h");
>     run("-o", helloJar.toString(), helloH.toString()).checkSuccess(); 
> // create the jar
>     try(Loader loader = classLoader(helloJar)) {
>         // ... do tests
>     } finally { // class loader is closed after try-block
>         deleteFile(helloJar); // now succeeds to delete jar
>     }
>
> I've replaced calls to loadClass with calls to Loader::loadClass, also 
> adding a call to JextractToolRunner::classLoader to the enclosing 
> try-blocks. I've also made sure jars are being deleted before /after a 
> test in some places where that was not being done.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8216284
> Webrev: 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8216284/webrev.03/
>
> Thanks,
> Jorn