On Thu, 14 Dec 2023 21:14:33 GMT, Eirik Bjorsnos <d...@openjdk.org> wrote:
>> Archie Cobbs has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Address review comments.
>
> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 37:
>
>> 35: public static void main(String [] args) throws IOException {
>> 36:
>> 37: // Create gz data
>
> Perhaps expand the comment to explain that you are creating a concatenated
> stream?
Thanks - should be addressed in c7087e55319.
> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 54:
>
>> 52: try (GZIPInputStream in = new GZIPInputStream(new
>> ByteArrayInputStream(gz32))) {
>> 53: count1 = countBytes(in);
>> 54: }
>
> Consider rewriting countBytes to take the
> ByteArrayInputStream/ZeroAvailableInputStream as a parameter, so you could
> just do:
>
> ```suggestion
> long count1 = countBytes(new ByteArrayInputStream(gz32));
Thanks - should be addressed in c7087e55319.
> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 56:
>
>> 54: }
>> 55:
>> 56: // (a) Read it from a stream where available() always returns
>> zero
>
> Suggestion:
>
> // (b) Read it from a stream where available() always returns zero
Thanks - should be addressed in c7087e55319.
> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 64:
>
>> 62: // They should be the same
>> 63: if (count2 != count1)
>> 64: throw new AssertionError(count2 + " != " + count1);
>
> Consider converting the test to JUnit, this would allow:
> Suggestion:
>
> asssertEquals(count1, count2);
Thanks - should be addressed in c7087e55319.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1427507217
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1427507107
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1427507154
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1427507189
