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