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

Reply via email to