On Sun, 6 Oct 2024 18:13:14 GMT, Bernd <d...@openjdk.org> wrote:
>> Markus KARG has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fixup! Reader.of(String)
>>
>> Dropping non-public JavaDocs
>
> test/jdk/java/io/Reader/Of.java line 63:
>
>> 61: @Test(dataProvider = "readers")
>> 62: public void testOpen(Reader reader) {
>> 63: assertNotNull(reader, "Reader.of(String) returned null");
>
> This does not look like a useful test and the assert is misleading (no String)
Removed this test.
> test/jdk/java/io/Reader/Of.java line 145:
>
>> 143: assertEquals(reader.transferTo(sw), 16, "transferTo() != 16");
>> 144: assertEquals(reader.transferTo(sw), 0,
>> 145: "transferTo() does not resect empty stream");
>
> Typo respect (I also wonder if “empty stream” (used multiple times) should be
> “end of stream” or “exhausted stream”. The test never get an empty stream to
> test.
"empty stream" is actually correct. The case "end of stream" is implied in the
line before (the test that expects result `16`, it starts with non-empty
stream, it stops at end-of-stream). From that point, the stream is now empty,
as `transferTo()` transferred *everything*. So what this test (the one
expecting result `0`) tests is what happens if `transferTo()` starts in an
already-empty situation.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21371#discussion_r1791504721
PR Review Comment: https://git.openjdk.org/jdk/pull/21371#discussion_r1791673725
