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

Reply via email to