On Tue, 20 May 2025 11:29:42 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Reflects review comments
>
> test/jdk/java/io/Console/StdinEncodingTest.java line 46:
>
>> 44: * @run junit StdinEncodingTest
>> 45: */
>> 46: public class StdinEncodingTest {
>
> AFAICT, there is no similar test (e.g., one using a mock `CharsetProvider`)
> for `stdout.encoding`. Will it be addressed by another ticket? Shall we
> consider adding a similar `StdoutEncodingTest` too? (Not necessarily in this
> PR.)
`stdout.encoding` validity is tested through the public `charset()` mehtod,
which is in `CharsetTest.java`
> test/jdk/java/io/Console/csp/provider/MockCharsetProvider.java line 83:
>
>> 81: while (in.remaining() > 0) {
>> 82: char c = (char)in.get();
>> 83: if (c != '\n') {
>
> `Character.toUpperCase('\n') == '\n'`, not? If so, do we still need this
> `if`-branching?
Eliminating newlines was a hack to ignore them in the expect responses so that
it can check the combined output in one shot. Now I think it is better to check
the result separately, so modified as such.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2098947409
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2098947338
