[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR:8153978:New test to verify the modules info as returned by the JVMTI
From: Alexander Kulyakhtin <alexander.kulyakhtin () oracle ! com>
Date: 2016-05-05 11:25:27
Message-ID: c0ef6c27-b1ea-4feb-979c-b1a08d4d38c7 () default
[Download RAW message or body]
Sergey,
Thank you very much for the review.
I will be pushing the fix as soon as jtreg 4.2 is out, since 4.2 has the fix for \
CODETOOLS-7901662, required for this test.
Best regards,
Alexander
From: serguei.spitsyn@oracle.com
To: alexander.kulyakhtin@oracle.com
Cc: serviceability-dev@openjdk.java.net, aleksey.voytilov@oracle.com
Sent: Wednesday, May 4, 2016 11:31:07 PM GMT +03:00 Iraq
Subject: Re: RFR:8153978:New test to verify the modules info as returned by the JVMTI \
Hi Alexander,
It looks good.
Thank you for making the changes!
Thanks,
Serguei
On 5/4/16 05:17, Alexander Kulyakhtin wrote:
Hi Sergey,
Thank you very much for the review.
Please, find the updated webrev with your findings corrected at:
http://cr.openjdk.java.net/~akulyakh/8153978_02/index.html
Best regards,
Alexander
----- Original Message -----
From: serguei.spitsyn@oracle.com
To: alexander.kulyakhtin@oracle.com , serviceability-dev@openjdk.java.net
Cc: aleksey.voytilov@oracle.com
Sent: Tuesday, May 3, 2016 1:06:05 AM GMT +03:00 Iraq
Subject: Re: RFR:8153978:New test to verify the modules info as returned by the JVMTI \
Hi Alexander,
Could you, fix a couple of minor issues?
test/serviceability/jvmti/GetModulesInfo/JvmtiGetAllModulesTest.java
58 for(Module mod : my.modules()) {
59 if(!jvmtiModules.contains(mod)) {
A space is missed after the 'for' and 'if' keywords.
test/serviceability/jvmti/GetModulesInfo/ModulesInfo.java.
31 boolean compareExcludingUnnamed(ModulesInfo other) {
I'd suggest to call it compareNamed.
Otherwise, the new test looks great.
Thanks a lot for taking care about it!
Thanks,
Serguei
On 4/29/16 06:12, Alexander Kulyakhtin wrote:
Hi,
Could you, please, review these test-only changes (adding a new test).
CR: https://bugs.openjdk.java.net/browse/JDK-8153978 "New test to verify the modules \
info as returned by the JVMTI"
Webrev: http://cr.openjdk.java.net/~akulyakh/8153978_01/ The new test verifies that \
JVMTI returns the correct info about the modules loaded at the application startup. \
It also verifies that the returned info is consistent with the same info returned by \
the Java API. It then loads a new named module and checks the correctness of the \
JVMTI info again.
Due to a tools issue https://bugs.openjdk.java.net/browse/CODETOOLS-7901662 the test \
can only be pushed in when the updated jtreg is released. The test passes fine with \
the nightly jtreg build, containing the CODETOOLS-7901662 fix.
Best regards,
Alexander
[Attachment #3 (text/html)]
<html><head><style type='text/css'>p { margin: 0; }</style></head><body><div \
style='font-family: Times New Roman; font-size: 12pt; color: \
#000000'>Sergey,<br><br>Thank you very much for the review. <br>I will be pushing the \
fix as soon as jtreg 4.2 is out, since 4.2 has the fix for CODETOOLS-7901662, \
required for this test.<br><br>Best regards,<br>Alexander<br><br>From: \
serguei.spitsyn@oracle.com<br>To: alexander.kulyakhtin@oracle.com<br>Cc: \
serviceability-dev@openjdk.java.net, aleksey.voytilov@oracle.com<br>Sent: Wednesday, \
May 4, 2016 11:31:07 PM GMT +03:00 Iraq<br>Subject: Re: RFR:8153978:New test to \
verify the modules info as returned by the JVMTI<br><br>
<div>
<div class="moz-cite-prefix">Hi Alexander,<br>
<br>
It looks good.<br>
Thank you for making the changes!<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
<br>
On 5/4/16 05:17, Alexander Kulyakhtin wrote:<br>
</div>
<blockquote cite="mid:a1d1a716-1967-462b-8547-17493d7ce8d2@default">
<style>p { margin: 0; }</style>
<div style="font-family: Times New Roman; font-size: 12pt; color:
#000000">Hi Sergey,<br>
<br>
Thank you very much for the review.<br>
<br>
Please, find the updated webrev with your findings corrected at:
<br>
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~akulyakh/8153978_02/index.html" \
target="_blank">http://cr.openjdk.java.net/~akulyakh/8153978_02/index.html</a><br> \
<br> Best regards,<br>
Alexander<br>
<br>
----- Original Message -----<br>
From: <a class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com" \
target="_blank">serguei.spitsyn@oracle.com</a><br>
To: <a class="moz-txt-link-abbreviated" \
href="mailto:alexander.kulyakhtin@oracle.com" \
target="_blank">alexander.kulyakhtin@oracle.com</a>,
<a class="moz-txt-link-abbreviated" \
href="mailto:serviceability-dev@openjdk.java.net" \
target="_blank">serviceability-dev@openjdk.java.net</a><br>
Cc: <a class="moz-txt-link-abbreviated" \
href="mailto:aleksey.voytilov@oracle.com" \
target="_blank">aleksey.voytilov@oracle.com</a><br> Sent: Tuesday, May 3, 2016 \
1:06:05 AM GMT +03:00 Iraq<br> Subject: Re: RFR:8153978:New test to verify the \
modules info as returned by the JVMTI<br>
<br>
<div>
<div class="moz-cite-prefix">Hi Alexander,<br>
<br>
<br>
Could you, fix a couple of minor issues?<br>
<br>
test/serviceability/jvmti/GetModulesInfo/JvmtiGetAllModulesTest.java<br>
<pre> 58 for(Module mod : my.modules()) {
59 if(!jvmtiModules.contains(mod)) {
A space is missed after the 'for' and 'if' keywords.
</pre>
<br>
test/serviceability/jvmti/GetModulesInfo/ModulesInfo.java.<br>
<pre> 31 boolean compareExcludingUnnamed(ModulesInfo other) {
I'd suggest to call it compareNamed.
</pre>
<br>
Otherwise, the new test looks great.<br>
Thanks a lot for taking care about it!<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
<br>
On 4/29/16 06:12, Alexander Kulyakhtin wrote:<br>
</div>
<blockquote cite="mid:16ccbaaf-d909-4a1a-af87-5f456746fe20@default">
<pre>Hi,
Could you, please, review these test-only changes (adding a new test).
CR: <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8153978" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8153978</a> "New test to \
verify the modules info as returned by the JVMTI"
Webrev: <a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/%7Eakulyakh/8153978_01/" \
target="_blank">http://cr.openjdk.java.net/~akulyakh/8153978_01/</a>
The new test verifies that JVMTI returns the correct info about the modules loaded at \
the application startup. It also verifies that the returned info is consistent with \
the same info returned by the Java API. It then loads a new named module and checks \
the correctness of the JVMTI info again.
Due to a tools issue <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/CODETOOLS-7901662" \
target="_blank">https://bugs.openjdk.java.net/browse/CODETOOLS-7901662</a> the test \
can only be pushed in when the updated jtreg is released. The test passes fine with \
the nightly jtreg build, containing the CODETOOLS-7901662 fix.
Best regards,
Alexander
</pre>
</blockquote>
<br>
</div>
</div>
</blockquote>
<br>
</div>
</div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic