On Thu, 1 Dec 2022 11:44:11 GMT, Lance Andersen <lan...@openjdk.org> wrote:
>> Liam Miller-Cushon has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Improve test
>
> test/jdk/tools/jar/ManifestDirectoryCompression.java line 81:
>
>> 79:
>> 80: @Test
>> 81: public void run() throws Exception {
>
> Please rename `run() `to something like `TestDirectoryCompressionMethod()`.
> We are trying to make new tests have more meaningful names.
Done
> test/jdk/tools/jar/ManifestDirectoryCompression.java line 83:
>
>> 81: public void run() throws Exception {
>> 82: Path entryPath = Files.writeString(tempDir.resolve("test.txt"),
>> "Some text...");
>> 83: Path jar = tempDir.resolve("test.jar");
>
> Please see comment above regarding the cleanup method.
>
> One other thought you could consider given you only create a jar and file to
> add to the jar, is to simply add File.deleteIfExists() calls and not bother
> with a cleanup method given the test case is small and pretty straight
> forward. Your choice though :-)
Thanks - I left the cleanup in an `@After` method for now, but I'm happy to
change it.
One small reason to keep the `@After` method is that I think it ensures the
cleanup happens even if the test fails and exits early, without having to wrap
the test body in a `try`/`finally`.
-------------
PR: https://git.openjdk.org/jdk/pull/11441
