RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v5]

Lance Andersen lancea at openjdk.org
Fri Mar 31 17:47:19 UTC 2023


On Fri, 31 Mar 2023 13:51:22 GMT, Eirik Bjorsnos <duke at openjdk.org> wrote:

>> CorruptedZipFiles could benefit from some spring cleaning and a conversion to junit:
>> 
>> - The actual tests are moved into their own `@Test` methods, given more meaningful names and a Javadoc comment explaining the constraint being verified
>> - The setup code is moved to a `@Before` method, slightly modernized and rewritten to take advantage of `assertEquals` 
>> - `checkZipExceptionImpl` is updated to take advantage of `assertThrows`
>> - A bunch of constants copied over from `ZipFile` can be deleted since JDK-6225935 has long been fixed
>
> Eirik Bjorsnos has updated the pull request incrementally with four additional commits since the last revision:
> 
>  - Document the structure of the 'good' ZIP
>  - Use the "Validate that.." comment style
>  - Document the ByteBuffer with a comment
>  - Use ByteBuffer when manipulating multi-byte fields

Thanks for your recent updates Eirik, I think this looks good.

A few minor comment suggestions

I will get a  Mach 5 run done as a sanity check

test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java line 56:

> 54:      * Byte array holding a valid template ZIP.
> 55:      *
> 56:      * This 'good' ZIP file has the following structure:

Change this -> the

test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java line 192:

> 190:     /*
> 191:      * Validate that a ZipException is thrown when the 'End of Central Directory'
> 192:      * (END) header has a CEN offset incoherent with the position calculated

Not sure _incoherent_ is the best term.

      perhaps something along the lines:

header  contains an invalid CEN Directory  starting offset.....

test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java line 203:

> 201:     /*
> 202:      * Validate that a ZipException is thrown when a A CEN header has an unexpected signature
> 203:      */

change  "a A" to "a"

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

PR Review: https://git.openjdk.org/jdk/pull/12563#pullrequestreview-1367326652
PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1154726957
PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1154735421
PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1154735787


More information about the core-libs-dev mailing list