RFR: 7036144: GZIPInputStream readTrailer uses faulty available() test for end-of-stream [v3]

[Archie Cobbs]

On Fri, 15 Dec 2023 20:38:19 GMT, Eirik Bjorsnos <duke at openjdk.org> wrote:

> The test is shaping up nicely. Since it's a new test it should use JUnit 5.
> 
> Also included a couple suggestions, I'll stop now, promise! :)

No prob - they're all reasonable suggestions :)

> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 30:
> 
>> 28:  */
>> 29: 
>> 30: import org.junit.Test;
> 
> Let's use JUnit 5:
> Suggestion:
> 
> import org.junit.jupiter.api.Test;

Should be fixed in cf457eff38c.

> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 36:
> 
>> 34: import java.util.zip.*;
>> 35: 
>> 36: import static org.junit.Assert.assertArrayEquals;
> 
> Let's use JUnit 5:
> 
> Suggestion:
> 
> import static org.junit.jupiter.api.Assertions.assertArrayEquals;

Should be fixed in cf457eff38c.

> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 53:
> 
>> 51:         byte[] compressedN = repeat(compressed1, NUM_COPIES);
>> 52: 
>> 53:         // (a) Read back copied compressed data from a stream where available() is accurate and verify
> 
> Suggestion:
> 
>         // (a) Read back inflated data from a stream where available() is accurate and verify

Should be fixed in cf457eff38c.

> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 54:
> 
>> 52: 
>> 53:         // (a) Read back copied compressed data from a stream where available() is accurate and verify
>> 54:         byte[] readback1 = new GZIPInputStream(new ByteArrayInputStream(compressedN)).readAllBytes();
> 
> These readback lines got rather long. Perhaps consider extracting the GZIP reading into a method taking the source InputStream as a parameter?
> 
> Suggestion:
> 
>         byte[] readback1 = inflateFrom(new ByteArrayInputStream(compressedN));

Should be fixed in cf457eff38c.

> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 57:
> 
>> 55:         assertArrayEquals(uncompressedN, readback1);
>> 56: 
>> 57:         // (b) Read back copied compressed data from a stream where available() always returns zero and verify
> 
> Suggestion:
> 
>         // (b) Read back inflated data from a stream where available() always returns zero and verify

Should be fixed in cf457eff38c.

> test/jdk/java/util/zip/GZIP/GZIPInputStreamAvailable.java line 58:
> 
>> 56: 
>> 57:         // (b) Read back copied compressed data from a stream where available() always returns zero and verify
>> 58:         byte[] readback2 = new GZIPInputStream(new ZeroAvailableStream(new ByteArrayInputStream(compressedN))).readAllBytes();
> 
> Suggestion:
> 
>         byte[] readback2 = inflateFrom(new ZeroAvailableStream(new ByteArrayInputStream(compressedN)));

Should be fixed in cf457eff38c.

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

PR Comment: https://git.openjdk.org/jdk/pull/17113#issuecomment-1858489270
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1428486646
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1428487198
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1428486708
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1428487465
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1428486783
PR Review Comment: https://git.openjdk.org/jdk/pull/17113#discussion_r1428486924