> 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. > > `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 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 14 additional commits since the last revision: - Merge branch 'master' into readzip-junit - Merge the ReadAfterClose test into ReadZip, converting it to JUnit. - Improve comment explaining what happens when ZipEntry.setCompressedSize is calledExplicitly - Convert CopyZipFile.java to JUnit - Remove trailing whitespace - Merge GetDirEntry.java into ReadZip.readDirectoryEntries() - Retire redundant test ZipFile/CopyJar - Merge Available.java into ReadZip.java - Add @bug reference 4401122 on the Available.java test - Merge branch 'master' into readzip-junit - ... and 4 more: https://git.openjdk.org/jdk/compare/82804792...85500cd8 ------------- Changes: - all: https://git.openjdk.org/jdk/pull/17038/files - new: https://git.openjdk.org/jdk/pull/17038/files/27fe1131..85500cd8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17038&range=07 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17038&range=06-07 Stats: 3896 lines in 272 files changed: 2360 ins; 631 del; 905 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