> 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 
> read methods.

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:
  - all: https://git.openjdk.org/jdk/pull/17038/files
  - new: https://git.openjdk.org/jdk/pull/17038/files/85500cd8..669b8da1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=17038&range=08
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17038&range=07-08

  Stats: 30 lines in 1 file changed: 28 ins; 0 del; 2 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

Reply via email to