On Fri, 11 Jul 2025 22:07:01 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> In prior JDK releases, `System.console()` could return a `Console` instance
>> even when the JVM was not attached to an interactive terminal. This could
>> lead to confusion, particularly when input was not from a keyboard or output
>> was redirected, such as to or from a file or pipe, especially when using
>> methods like `readPassword()`. Starting with JDK 25, the default behavior
>> has changed: `System.console()` now returns `null` if standard input and/or
>> output is redirected. However, if a JLine-based Console implementation is
>> explicitly specified via the system property
>> `-Djdk.console=jdk.internal.le`, the previous behavior may still occur.
>> This PR aims to align the behavior of the JLine-based `Console`
>> implementation with the default `System.console()` behavior. The actual code
>> change is a one-liner in `JdkConsoleProviderImpl.java`; the rest of the
>> changes are adjustments to test cases to reflect the updated behavior. A
>> corresponding CSR has also been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update test/jdk/java/io/Console/LocaleTest.java
>
> Co-authored-by: Andrey Turbanov <turban...@gmail.com>
test/jdk/java/io/Console/DefaultCharsetTest.java line 52:
> 50: void testDefaultCharset(String stdoutEncoding) throws Exception {
> 51: // check "expect" command availability
> 52: var expect = Paths.get("/usr/bin/expect");
We only need to check "expect" availability once, so we should move this check
to a `@BeforeAll` static method. It's also more clear that this check is a
precondition, and not part of the actual test. Applies to the other locations,
but primarily the other parameterized tests.
test/jdk/java/io/Console/ModuleSelectionTest.java line 32:
> 30: * @library /test/lib
> 31: * @build jdk.test.lib.Utils
> 32: * jdk.test.lib.JDKToolFinder
Is `jdk.test.lib.JDKToolFinder` needed?
test/jdk/java/io/Console/ModuleSelectionTest.java line 65:
> 63: @ParameterizedTest
> 64: @MethodSource("options")
> 65: void testWithoutExpect(String opts, String expected) throws Exception
> {
`expected` param looks unused.
test/jdk/java/io/Console/ModuleSelectionTest.java line 75:
> 73: @ParameterizedTest
> 74: @MethodSource("options")
> 75: void testWithExpect(String opts, String expected) throws Exception {
I think it would be more clear if these tests were renamed to
`expectConsoleTest` and `noConsoleTest` or something along those lines.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26273#discussion_r2205500449
PR Review Comment: https://git.openjdk.org/jdk/pull/26273#discussion_r2205576522
PR Review Comment: https://git.openjdk.org/jdk/pull/26273#discussion_r2205557781
PR Review Comment: https://git.openjdk.org/jdk/pull/26273#discussion_r2205573038
