[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8308033: The jcmd thread dump related tests should test virtual threads [v2]
From:       Jaikiran Pai <jpai () openjdk ! org>
Date:       2024-04-30 12:14:05
Message-ID: 7sc08xuI5H3EpIhuUKhZO4mycpBkzQjeELpS2boeaeY=.a7db8bb0-7284-4cd1-8127-1e3c47a19be4 () github ! com
[Download RAW message or body]

On Tue, 30 Apr 2024 12:08:21 GMT, Jaikiran Pai <jpai@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: 
> address additional problem listed tests in hotspot/jtreg/serviceability

I've opened this for review again. The additional changes were straightforward. The \
`test/hotspot/jtreg/serviceability/dcmd/thread/` tests which were problem listed were \
testing `Thread.print` command against the current thread. Those have now been \
updated to skip the tests when the current thread is a virtual thread, since \
`Thread.print` doesn't generate thread dumps for virtual threads.

The `test/hotspot/jtreg/serviceability/tmtools/jstack/DaemonThreadTest.java` has been \
updated to explicitly mark a (platform) thread launched from within the test as \
non-daemon to verify that the output of `jstack` correctly captures that state.

With these changes all these tests pass, both when jtreg uses platform thread and  \
virtual thread to launch the tests.

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

PR Comment: https://git.openjdk.org/jdk/pull/19016#issuecomment-2085163934


[prev in list] [next in list] [prev in thread] [next in thread]