RFR: 8341566: Adding factory for non-synchronized CharSequence Reader [v3]

Markus KARG duke at openjdk.org
Tue Oct 8 11:29:07 UTC 2024


On Sun, 6 Oct 2024 18:13:14 GMT, Bernd <duke at 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


More information about the core-libs-dev mailing list