On Tue, 2 Jan 2024 13:31:01 GMT, Eirik Bjørsnøs <eir...@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: > > Add test verifying that ZipFile can open a ZIP file with zero entries Changes look fine. Probably need to change the copyright from 2023 -> 2024 ------------- Marked as reviewed by lancea (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17038#pullrequestreview-1787585369
