RFR: 8321616: Retire binary test vectors ZipFile/input.jar and ZipFile/input.zip [v2]
Eirik Bjorsnos
duke at openjdk.org
Thu Dec 14 10:18:03 UTC 2023
> 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
More information about the core-libs-dev
mailing list