> 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. I > added a jtreg header with `@bug 4401122`. 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. An alternative action > would be to remove this test. It seems to validate the current implementation > more than the specification. > > `CopyJar.java` > The 1999 bug description JDK-4239446 is short and somewhat confusing. It > seems at some point that the CRC of `ZipEntry` read by `ZipFile.getEntry` > would be read from a LOC header instead of the CEN header, which means it > could be zero for streaming entries with data descriptors. (However, the bug > description also mentions calling `getNextEntry`, which is a `ZipInputStream` > method?) In any case, this concern seems to now have better coverage in the > test `zip/CopyZipFile` from 2020, so it would perhaps be desirable to simply > remove `CopyJar.java` to reduce duplication? > > `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` > This test seems to duplicate the test `ReadZip.readDirectoryEntry`. It would > perhaps be better to remove `GetDirEntry.java` to reduce duplication? > > `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 us...
Eirik Bjorsnos has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision: - Add @bug reference 4401122 on the Available.java test - Merge branch 'master' into readzip-junit - The input.jar test vector included a META-INF/ directory before the manifest, lets keep it that way - Remove the use of binary test vector ZIP/JAR files input.zip and input.jar, converting affected tests to JUnit - Update copyright year - Convert ReadZip to JUnit ------------- Changes: - all: https://git.openjdk.org/jdk/pull/17038/files - new: https://git.openjdk.org/jdk/pull/17038/files/f6a00f0c..56f55d89 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17038&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17038&range=00-01 Stats: 5608 lines in 92 files changed: 3340 ins; 1807 del; 461 mod Patch: https://git.openjdk.org/jdk/pull/17038.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17038/head:pull/17038 PR: https://git.openjdk.org/jdk/pull/17038
