RFR: 8304020: Speed up test/jdk/java/util/zip/ZipFile/TestTooManyEntries.java and clarify its purpose [v10]

[Eirik Bjorsnos]

On Wed, 15 Nov 2023 07:41:32 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

> Overall, this is a very good improvement to the test and looks good to me. I just a have a trivial comment about a typo in a code comment, which I've added inline.

Thanks for your review, Jaikiran! With these latest, comment-only changes, this PR is now looking for a sponsor.

> test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java line 78:
> 
>> 76:     public static final int NAME_LENGTH = 10;
>> 77: 
>> 78:     // Use a shared LocalDataTime on all entries to save processing time
> 
> Hello Eirik, there appears to be a typo in the comment here. It should have been `LocalDateTime`.

Fixed

> test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java line 204:
> 
>> 202:                 channel.position(position);
>> 203:                 // Check for last CEN record
>> 204:                 if (Arrays.equals(LAST_CEN_COMMENT_BYTES, 0, LAST_CEN_COMMENT_BYTES.length, b, off, len)) {
> 
> The way the instance of a `SparseOutputStream` is used in this test, it gets passed to the constructor of `ZipOutputStream`. That then means that there is no buffering involved when bytes are written out to the output stream internally by `ZipOutputStream`. The implementation of `ZipOutputStream` writes out the `ZipEntry`'s comment (if any) in one single `write(byte[])` call on the outputstream, so it's guaranteed that the `byte[] b` passed in here will actually have the entire comment (from `off` to `len`). So this `Arrays.equals(...)` check is then guaranteed to pass (when that specific entry does get written out). So this check looks good to me.

Thanks, this was a nice observation. I added a short comment to the SparseOutputStream noting the instances should be passed directly, without any buffering.

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

PR Comment: https://git.openjdk.org/jdk/pull/12991#issuecomment-1813123375
PR Review Comment: https://git.openjdk.org/jdk/pull/12991#discussion_r1394673748
PR Review Comment: https://git.openjdk.org/jdk/pull/12991#discussion_r1394674564