On Mon, 27 Jan 2025 21:21:43 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one >> additional commit since the last revision: >> >> review: replace bug number in a couple of problem-list entries > > test/hotspot/jtreg/ProblemList-Virtual.txt line 41: > >> 39: >> 40: >> vmTestbase/nsk/jvmti/GetCurrentThreadCpuTime/curthrcputime001/TestDescription.java >> 8300708 generic-all >> 41: vmTestbase/nsk/jvmti/GetThreadCpuTime/thrcputime001/TestDescription.java >> 8300708 generic-all > > You still have these two unfixed tests in the problem list using the CR for > this PR. Either they will need a new CR, or you will need to use a different > CR for this PR. Thanks. I've created a new bug and replaced the bug number in these entries: [8348844](https://bugs.openjdk.org/browse/JDK-8348844): Remove remaining JVMTI tests from ProblemList-Virtual, use require instead > test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetLocalVariable/setlocal001.java > line 105: > >> 103: checkPoint(); >> 104: if (currThread.isVirtual()) { >> 105: out.println("meth01: skipping results check for virtual >> thread"); > > Should be "meth02". Similar issues below. Thanks. Fixed now. > test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetLocalVariable/setlocal001/setlocal001.cpp > line 78: > >> 76: err = jvmti_env->SetLocalLong(thr, 1, >> 77: table[i].slot, longVal); >> 78: if (err != JVMTI_ERROR_NONE && !(is_virtual && err == >> JVMTI_ERROR_OPAQUE_FRAME)) { > > Maybe add a new API called something `invalidError(err)`. Not necessary, but > might be a little better. Thanks. I was also thinking about this but was not sure it is worth it. Will update now as there are two of us now. :) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23314#discussion_r1932110589 PR Review Comment: https://git.openjdk.org/jdk/pull/23314#discussion_r1932113713 PR Review Comment: https://git.openjdk.org/jdk/pull/23314#discussion_r1932116105