On Sun, 20 Sep 2026 12:55:48 GMT, Jaikiran Pai <[email protected]> wrote:
>> test/jdk/java/io/Reader/ReadAll.java line 283: >> >>> 281: } >>> 282: >>> 283: // InputStreamReader implementation: Internal decoder, empty >>> stream but decoder has bytes >> >> Here and a few other places in this updated test, I think it would be good >> to replace the use of "Internal decoder" with something like "decoder >> belongs to the Charset instance". The "empty stream but decoder has bytes" >> part here is a bit confusing, since the stream isn't empty. For this >> specific comment maybe something like this would be appropriate? >> >> decoder belongs to the Charset instance, read all bytes from the stream >> using read(), then call readAllAsString() > > Same comment for one other place in this test which says "empty stream but > decoder has bytes". In fact the code comment is pretty correct, and I do not see what is confusing. The intention of all these tests is to proof that `readAllAsString()` works correctly *in specific situations*. The comment describes the situation at which that exact method is invoked (which is *not* the situation when the *test* starts): The input stream will be empty *when `readAllAsString()` will be invoked*, as the sole byte which was found in that stream when the *test* started is already consumed and now stuck in the decoder. The sense of the comment is *not* to repeat the obvious, but to make clear the use case. Don't know how to make this any clearer. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32264#discussion_r4116193226
