On Thu, 14 Dec 2023 14:55:08 GMT, Eirik Bjorsnos <d...@openjdk.org> wrote:
>> This PR suggests we retire the binary test vectors `ZipFile/input.zip` and >> `ZipFile/input.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, 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. >> >> `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 >> read methods. > > Eirik Bjorsnos has updated the pull request incrementally with one additional > commit since the last revision: > > Merge the ReadAfterClose test into ReadZip, converting it to JUnit. Seeing that `ReadAfterClose.java` uses a binary test vector `crash.jar`, I think it makes sense to include it in this PR, convert it to JUnit and move it into `ReadZip.readAfterClose`. This removes the last remaining binary test vector ZIP in the `ZipFile/` directory. ------------- PR Comment: https://git.openjdk.org/jdk/pull/17038#issuecomment-1856008970
