RFR: 8361613: System.console() should only be available for interactive terminal [v2]

[Naoto Sato]

On Mon, 14 Jul 2025 17:57:22 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> 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 <turbanoff at 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.

Good point. Modified to use `@BeforeAll`. For `ModuleSelectionTest`, one of the test is not using `expect`, so I left it as it is. In addition to that, I removed the `@requires` condition to allow that test to run on windows.

> 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.

Yeah, using `expect` command with junit's expect is confusing 🙂. Just renamed those methods using TTY/NonTTY, as"expect/noConsoleTest" reads somewhat odd as it includes the expected results in the test name.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26273#discussion_r2205749625
PR Review Comment: https://git.openjdk.org/jdk/pull/26273#discussion_r2205753058