On Thu, 2 May 2024 03:57:25 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Can I please get a review of these test-only changes which proposes to >> remove the `test/jdk/sun/tools/jcmd/JcmdOutputEncodingTest.java` and >> `test/jdk/sun/tools/jstack/BasicJStackTest.java` tests from >> `ProblemList-Virtual.txt`? >> >> When jtreg was enhanced to allow running the tests from within a virtual >> (main) thread, some tests were problem listed since they either were failing >> at that time or the test code would require additional work to make it work >> when the current thread is a virtual thread. The >> `test/jdk/sun/tools/jcmd/JcmdOutputEncodingTest.java` and >> `test/jdk/sun/tools/jstack/BasicJStackTest.java` are 2 such threads which >> require special handling when the current thread is a virtual thread. >> >> `test/jdk/sun/tools/jcmd/JcmdOutputEncodingTest.java` has been updated to >> use a different command to dump stacktraces when the test runs in a virtual >> thread. I went back and looked at the original issue for which this test was >> introduced and based on that, using a different command to dump the >> stacktraces shouldn't impact what the test was originally intended to test. >> >> `test/jdk/sun/tools/jstack/BasicJStackTest.java` has been updated to be >> skipped when the current thread is the virtual thread. This is because the >> test exercises the `jstack` tool which doesn't print stacktraces of virtual >> threads and thus the test isn't applicable for virtual threads. >> >> I've run these tests with this change, both with platform threads and >> virtual threads in our CI and the tests complete without failures. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > Chris' review - DaemonThreadTest.java, no need for checking if thread is > virtual This looks good. ------------- Marked as reviewed by sspitsyn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19016#pullrequestreview-2044949004
