RFR: 8321616: Retire binary test vectors in test/jdk/java/util/zip/ZipFile [v7]

[Lance Andersen]

On Thu, 14 Dec 2023 14:55:08 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> This PR suggests we retire the binary test vectors `ZipFile/input.zip`, `ZipFile/input.jar` and `ZipFile/crash.jar`
>> 
>> Binary test vectors are harder to analyze,  and sharing test vectors across unrelated tests increases maintenance burden. It would be better to have each test produce its own test vectors independently.
>> 
>> While visiting these dusty tests, we should take the opportunity to convert them to JUnit, add more comments and perform some mild modernization and cleanups where appropriate. We should also consider whether any test are duplicated and can be retired or moved into other test files as separate methods. See comments below.
>> 
>> To help reviewers, here are some comments on the updated tests:
>> 
>> `Available.java`
>> This test currently has no jtreg `@test` header, so isn't currently active. After discussion, we decided to merge this test into `ReadZip.java`. I added some checks to verify that reading from the stream reduces the number of available bytes accordingly, also checking the behavior when the stream is closed.
>> 
>> `CopyJar.java`
>> The concern of copying entries seems to now have better coverage in the test `zip/CopyZipFile`. We decided to retire this test rather than convert it and instead convert `CopyZipFile` to JUnit.
>> 
>> `EnumAfterClose.java`
>> To prevent confusion with Java Enums, I suggest we rename this test to `EnumerateAfterClose`.
>> 
>> `FinalizeInflater.java`  
>> The code verified by this test has been updated to use cleaners instead of finalizers. Still, this test code relies on finalizers. Not sure if this is an issue, but this test will need to be updated when finalizers are finally removed.
>> 
>> `GetDirEntry.java`
>> We decided to merge this test into `ReadZip.readDirectoryEntries` rather than keeping it as a separate test.
>> 
>> `ReadZip.java`
>> Nothing exciting here, the single main method was split into multiple JUnit methods, each focusing on a separate concern. A new test case `noentries()` was added, resolving [JDK-8322830](https://bugs.openjdk.org/browse/JDK-8322830)
>> 
>> `ReleaseInflater.java`
>> Nothing exciting, tried to add some comment to help understanding of what is tested.
>> 
>> `StreamZipEntriesTest.java`
>> This test was using TestNG so was converted to JUnit for consistency. Added some comments to help understanding.
>> 
>> `ReadAfterClose.java`
>> This uses the binary test vector `crash.jar`. The test is converted to JUnit and moved to `ReadZip.readAfterClose´. The test is expanded to exercise more ...
>
> Eirik Bjørsnøs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Merge the ReadAfterClose test into ReadZip, converting it to JUnit.

test/jdk/java/util/zip/ZipFile/ReadZip.java line 233:

> 231:         URI uri = URI.create("jar:" + zip.toUri());
> 232:         Map<String, Object> env = Map.of("create", "true", "forceZIP64End", "true");
> 233:         try (FileSystem fs = FileSystems.newFileSystem(uri, env)) {

No Need to use a URI here

test/jdk/java/util/zip/ZipFile/ReadZip.java line 243:

> 241:         }
> 242:         // Read using ZipFileSystem
> 243:         try (FileSystem fs = FileSystems.newFileSystem(uri, Map.of())) {

Same comment as above regarding URI

test/jdk/java/util/zip/ZipFile/ReadZip.java line 249:

> 247: 
> 248:     /**
> 249:      * Read a zip file created via "echo hello | zip dst.zip -",

This was created using info-zip so I would clarify in the comments

test/jdk/java/util/zip/ZipFile/ReadZip.java line 257:

> 255:     @Test
> 256:     public void readZip64EndZipProcess() throws IOException, InterruptedException {
> 257:         if (Files.notExists(Paths.get("/usr/bin/zip"))) {

We should address this as the test won't run on Windows.  It would be better to store the zip as a byte array so that it can be processed on all platforms and by removing ProcessBuilder, the test run will speed up a bit

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17038#discussion_r1430574058
PR Review Comment: https://git.openjdk.org/jdk/pull/17038#discussion_r1430574583
PR Review Comment: https://git.openjdk.org/jdk/pull/17038#discussion_r1430575196
PR Review Comment: https://git.openjdk.org/jdk/pull/17038#discussion_r1430578476