RFR(M): JDK-8165805 - [TESTBUG] CDS needs to support JVMTI CFLH - test development

[Mikhailo Seledtsov]

Hi Jiangli,

  Thank you for review. Please see my comments inline:

On 9/12/16, 11:09 AM, Jiangli Zhou wrote:
> Hi Misha,
>
> These look very clean and well organized.
>
> - The copyright headers for all source files are incorrect. Please fix them to use right copyright headers for open code.
I will fix it.
> - Could you please add descriptions in TransformInterfaceAndImplementor.java and TransformSuperAndSubClasses.java with details explaining what do they test and what are the expected behavior/result. It would help people understand the tests by listing all the test cases. It would also be helpful to explain what part is transformed in each class. I know TransformRelatedClasses already have comments with some explanations. If you prefer, you could add more details in TransoformRelatedClasses.java and add one-line comment in TransformInterfaceAndImplementor.java&  TransformSuperAndSubClasses.java referring to TransoformRelatedClasses.java.
Will do. I will add more details in TransformRelatedClasses.java, and 
then some brief comments inside TransformSuper*/TransformInterface* and 
reference to TransformRelatedClasses.java for more details.
> I agree with Ioi’s suggestion about moving TransformUtil, TranformerAgeent to hotspot/testlibrary/jvmti. That way they can be used by other tests (in different location).
Done.
> Thanks,
> Jiangli
Once I make the changes, I will post an updated webrev.

Thank you,
Misha
>> On Sep 9, 2016, at 7:31 PM, Mikhailo Seledtsov<mikhailo.seledtsov at oracle.com>  wrote:
>>
>>     Please review the tests that accompany the changes for
>>     "JDK-8078644 - CDS needs to support JVMTI CFLH"
>>
>>     The tests include several test cases that are produced by combination
>>     of transforming related classes: super/sub class and interface/implementor.
>>     The classes reside in CDS archive. Tests check to ensure that correct
>>     transformation is performed, classes are loaded from the archive only when appropriate,
>>     and that test process using the archive completes w/o errors/crashes.
>>
>>
>>     Details:
>>
>>     The main class is TransformRelatedClasses.java; start review with this class.
>>     Other classes are:
>>       - TransformSuperAndSubClasses.java, TransformInterfaceAndImplementor.java
>>         the "actual tests", but really just test headers calling into TransformRelatedClasses.java
>>
>>       - TransformTestCommon.java - common methods for transformation tests
>>
>>       - TestEntry.java - an entry into combinatorial test table representing a single test case
>>
>>       - CDSTestUtils.java, TransformUtil.java - utility classes that are common  CDS
>>         tests
>>
>>       - TransformerAgent.java - agent that performs transformation (comes with .mf file)
>>
>>
>>     JBS: https://bugs.openjdk.java.net/browse/JDK-8165805
>>     Webrev: http://cr.openjdk.java.net/~mseledtsov/8165805.00/
>>     Testing:
>>         Ran these tests on a standard set of platforms thru automated test system.
>>
>> Thank you,
>> Misha
>>