RFR: 8304014: Convert test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java to junit [v5]
Martin Buchholz
martin at openjdk.org
Fri Mar 31 18:41:24 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
I support rewriting tests like this into junit 5.
I wrote many jdk tests (like this) before a standard high quality test framework like junit 5 was available.
Zip file implementations are 90% corner cases, so expanding testing and improving testability of jdk classes would be valuable. For example, it would be nice to explicitly request use of ZIP64 extensions even when they are not needed, to avoid having to create multi-gigabyte test files.
test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java line 246:
> 244: /*
> 245: * Validate that a ZipException is thrown if the last CEN header is
> 246: * not immediatly followed by the start of the 'End of central directory' record
typo: immediatly
test/jdk/java/util/zip/ZipFile/CorruptedZipFiles.java line 300:
> 298:
> 299: /*
> 300: * Validate that a ZipException is thrown when a LOC header has an unexpected signature
I would leave off "Validate that"
-------------
PR Review: https://git.openjdk.org/jdk/pull/12563#pullrequestreview-1367398354
PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1154773604
PR Review Comment: https://git.openjdk.org/jdk/pull/12563#discussion_r1154779308
More information about the core-libs-dev
mailing list