RFR: 8321616: Retire binary test vectors ZipFile/input.jar and ZipFile/input.zip

Eirik Bjorsnos duke at openjdk.org
Thu Dec 14 12:25:10 UTC 2023


On Thu, 14 Dec 2023 11:39:22 GMT, Lance Andersen <lancea at openjdk.org> wrote:

>> The scope of this PR has now expanded to removing uses of the `input.zip` and `input.jar` files, updating any test using them to produce their own test vectors, and convert affected tests to JUnit.
>> 
>> I'm marking the PR ready for review again. Before looking too closely at the code, it would be useful to discuss the following tests:
>> 
>> - `Available.java`: This test has no jtreg header. I've added one and converted the test. Is this worthwhile, or should we rather remove it?
>> - `CopyJar.java`: The concern tested seems to have superior coverage in the test `zip/CopyZipFile.java`. Should we retire `CopyJar.java` instead of coverting it?
>> - `DirEntry.java`: There is duplication between this test and `ReadZip.readDirectoryEntry()`. Should we retire one of these?
>
>> The scope of this PR has now expanded to removing uses of the `input.zip` and `input.jar` files, updating any test using them to produce their own test vectors, and convert affected tests to JUnit.
>> 
>> I'm marking the PR ready for review again. Before looking too closely at the code, it would be useful to discuss the following tests:
>> 
>> * `Available.java`: This test has no jtreg header. I've added one and converted the test. Is this worthwhile, or should we rather remove it?
> 
> This could be moved into ReadZip.  I do not believe we have a specific test and it is trivial
> 
>> * `CopyJar.java`: The concern tested seems to have superior coverage in the test `zip/CopyZipFile.java`. Should we retire `CopyJar.java` instead of coverting it?
> 
> Yes CopyZipFile already exercises Zipfile.ZipInputStream so it is safe to retire CopyJar (though CopyZipFile could use a junit conversion ;-) 
>> * `DirEntry.java`: There is duplication between this test and `ReadZip.readDirectoryEntry()`. Should we retire one of these?
> 
> I believe you meant GetDirEntry.java not DirEntry.java?
> 
> Having a test that specifically validates we can read META-INF is not a bad thing, but I suspect we have a test that already does that if not in the java/util/zip tests or java/util/jar tests.  If not we should keep it but merge it as you suggest

@LanceAndersen 

Thanks for your guidance! I moved `Available` into `ReadZip`, deleted `CopyJar` and merged `GetDirEntry` into ReadZip.readDirectoryEntries (adding a 'META-INF/' directory just in case)

-------------

PR Comment: https://git.openjdk.org/jdk/pull/17038#issuecomment-1855753641


More information about the core-libs-dev mailing list