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

Eirik Bjorsnos duke at openjdk.org
Mon Oct 30 15:28:51 UTC 2023


On Mon, 30 Oct 2023 14:10:25 GMT, Lance Andersen <lancea at openjdk.org> wrote:

>> 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:
>> 
>>  - Run CenSizeTooLarge automatically, since it no longer requires excessive memory or disk space
>>  - Use a SparseOutputStream to reduce required diskspace from 2GB to 4K. SparseOutputStream writes 'holes' instead of actual bytes until the final CEN is written.
>>  - TestTooManyEntries is renamed to CenSizeTooLarge
>>  - Merge branch 'master' into cen-size-too-large
>>  - MAX_EXTRA_FIELD_SIZE can be better expressed as 0xFFFF
>>  - Bring back '@requires sun.arch.data.model == 64' for now
>>  - Spell 'specified' correctly
>>  - Give test method a long, meaningful name
>>  - Remove blank line
>>  - Make CEN headers maximally large such that we need fewer of them and save memory
>>  - ... and 4 more: https://git.openjdk.org/jdk/compare/f5cb971b...9629b8d2
>
> test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java line 33:
> 
>> 31: import org.testng.annotations.BeforeTest;
>> 32: import org.testng.annotations.Test;
>> 33: 
> 
> As this will be a new test, could you please consider converting to junit.

- Added a comment explaining the chosen value for the constant, referencing APPNOTE.TXT 
- Converted the test to JUnit (even remembered to swap argument order of assertEquals!)

> test/jdk/java/util/zip/ZipFile/CenSizeTooLarge.java line 139:
> 
>> 137:     public void centralDirectoryTooLargeToFitInByteArray() {
>> 138:         ZipException ex = expectThrows(ZipException.class, () -> new ZipFile(hugeZipFile));
>> 139:         assertEquals(ex.getMessage(), "invalid END header (central directory size too large)");
> 
> Could be have the expected message a class constant so that if this message ever changes it is easier to find/update

Extracted the expected ZipException message to a class constant.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/12991#discussion_r1376407892
PR Review Comment: https://git.openjdk.org/jdk/pull/12991#discussion_r1376408869


More information about the core-libs-dev mailing list