RFR: 8297875: jar should not compress the manifest directory entry [v2]

[Liam Miller-Cushon]

On Wed, 30 Nov 2022 22:15:14 GMT, Lance Andersen <lancea at openjdk.org> wrote:

>> Liam Miller-Cushon has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Use TestNG, and assert on the compression method
>
> test/jdk/tools/jar/ManifestDirectoryCompression.java line 52:
> 
>> 50: 
>> 51:     /** Remove dirs & files needed for test. */
>> 52:     private static void cleanup(Path dir) {
> 
> This can leverage the @afterMethod annotation if the code is reworked slightly to make it more TestNG friendly

Thanks, done

> test/jdk/tools/jar/ManifestDirectoryCompression.java line 60:
> 
>> 58:             }
>> 59:             Files.delete(dir);
>> 60:         } catch (IOException e) {
> 
> Have the method throw IOException and you do not need the catch block

Unfortunately it's recursing on `cleanup` in the lambda, so it can't throw checked exceptions without more refactoring. This is imitating the recursive deletion approach in another jar test, I'm happy to swap this out if you'd prefer a different approach

> test/jdk/tools/jar/ManifestDirectoryCompression.java line 72:
> 
>> 70:             doTest(topDir.resolve("test.jar"), entry);
>> 71:         } finally {
>> 72:             cleanup(topDir);
> 
> See comment above regarding cleanup()

Thanks, I moved the cleanup into a separate `@AfterMethod`

> test/jdk/tools/jar/ManifestDirectoryCompression.java line 83:
> 
>> 81:         try (JarFile jarFile = new JarFile(jar.toFile())) {
>> 82:             ZipEntry zipEntry = jarFile.getEntry("META-INF/");
>> 83:             assertEquals(zipEntry.getMethod(), ZipEntry.STORED);
> 
> Probably worth verifying zipEntry is not null so you do not get an NPE

Done

-------------

PR: https://git.openjdk.org/jdk/pull/11441