On Thu, 29 Aug 2024 16:21:36 GMT, Lance Andersen <lan...@openjdk.org> wrote:

>> Eirik Bjørsnøs has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Collapse the ZipFile-related tests into a single, parameterized method
>
> test/jdk/java/util/zip/Available.java line 147:
> 
>> 145:         }
>> 146:     }
>> 147: 
> 
> Could we collapse the two above tests  via
> 
> 
> @ParameterizedTest
> @ValueSource(strings = { "stored.txt", "delated.txt" })
> void testAvailbleRemainingBytes(iString zipEntry) {
>     try (ZipFile zfile = new ZipFile(zip.toFile())) {
>             assertRemainingUncompressedBytes(zfile, zipEntry);
>         }
> }

I like it!

I collapsed `ZipFile` related tests into the method 
`testZipFileStreamsRemainingBytes` and inlined the 
`assertRemainingUncompressedBytes` method into that.

A few minor improvements to the inlined `assertRemainingUncompressedBytes` code 
body:

* Added a comment saying that the `InputStream` could be `ZipFileInputStream` 
or `ZipFileInflaterInputStream`
* Moved the "decrement by one" assertion logic into the loop, asserting 
decrement on each iteration 
* Cleaned up some spacing issues in the for loop

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20744#discussion_r1736708432

Reply via email to