> 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

Reply via email to