RFR: JDK-8322141: SequenceInputStream.transferTo should not return as soon as Long.MAX_VALUE bytes have been transferred [v4]

Markus KARG duke at openjdk.org
Sun Dec 17 14:05:41 UTC 2023


On Sat, 16 Dec 2023 19:09:05 GMT, Vladimir Sitnikov <vsitnikov at openjdk.org> wrote:

>> Markus KARG has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Test was using Integer but must use Long
>
> test/jdk/java/io/SequenceInputStream/TransferTo.java line 139:
> 
>> 137:         InputStream is1 = repeat(0, Long.MAX_VALUE);
>> 138:         InputStream is2 = repeat(0, 1);
>> 139:         assertNotEquals(is1.available(), 0);
> 
> Wdyt of asserting the expected value with assertEquals or removing the assertions before .transferTo altogether?
> Suggestion:
> 
>         assertEquals(is1.available(), Integer.MAX_VALUE, "The stream should have Long.MAX_VALUE bytes .available() before .transferTo (.available returns int)");

The reader shall understand what the test is good for and what it works like. There is no benefit in checking the *actual* value (there is no need to explicitly fail if `available` returns something *else*, as long as it is *not zero*), so this would confuse the reader more than it helps. I kept the before-assertations solely to proof that the before-value of `available()` is *not the default value* of `InputStream`, hence to proof that the source code of this test is actually able to detect a *change*. We can remove this, it is not needed, but it makes it easier *to understand* what this test actually expects *to change*.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17119#discussion_r1429169709


More information about the core-libs-dev mailing list