On Tue, 28 May 2024 16:56:18 GMT, Naoto Sato <na...@openjdk.org> wrote:

>> This test intends to verify the behavior of JdkConsole for the java.base 
>> module, wrt restoring the echo. The test assumes the internal methods that 
>> sets/gets the echo status of the platform.
>
> Naoto Sato has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 23 commits:
> 
>  - incorporated the same fix from 8332922
>  - Merge branch 'master' into JDK-8332161-restoreEcho-Test
>  - Merge branch 'master' into JDK-8332161-restoreEcho-Test
>  - Merge branch 'JDK-8332084-restoreEcho-readLock' into 
> JDK-8332161-restoreEcho-Test
>  - Added comment for restoreEchoLock
>  - fixing typo
>  - Merge branch 'JDK-8332084-restoreEcho-readLock' into 
> JDK-8332161-restoreEcho-Test
>  - Merge branch 'master' into JDK-8332084-restoreEcho-readLock
>  - leftover typo
>  - get/setEcho() -> echo()
>  - ... and 13 more: https://git.openjdk.org/jdk/compare/b8f2ec90...2477adf4

I might be mistaken, but this looks like a very well written unit test, which 
tests that an implementation flag retains its initial value. This does not look 
like an end-to-end test, which we usually write.

I wonder if we can use `expect` to run two end-to-end tests as follows. The 
first test would set up a console with echo on and then verify that after the 
spawned command completes the echo is still on. The second test would do the 
same for echo off.

The difference between the above approach and the test that this PR has 
proposed is that with the above approach we won't rely on an implementation 
detail, whose relevance to an objectively observed state is non-obvious or 
unknown. Instead, we'll use an OS utility, which makes for a relevant, 
straightforward, and maintainable test.

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

PR Comment: https://git.openjdk.org/jdk/pull/19315#issuecomment-2137515849

Reply via email to