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

[serguei.spitsyn at oracle.com]

Hi Misha,

The tests are nicely organized.

Just minor comments now:

The patterns must be defined only once as static final strings:
    static final String BeforePattern = "this-should-be-transformed ";
    static final String AfterPattern  = "this-has-been--transformed";

Not sure yet, where to define it better.
It seems, the TransformUtil class can be a good place.
Then the 'from' and 'to' parameters in the TransformUtil.replace() can 
be deleted.
It is if it is not important to keep this method universal.


http://cr.openjdk.java.net/~mseledtsov/8165805.00/test/runtime/SharedArchiveFile/serviceability/TransformUtil.java.html

   21         for (int i=0; i<max; ) {
   33         for (int x=0; x<f.length; x++) {
   38         for (int x=0; x<f.length; x++) {
   46         for (int i=0; i<b.length; i++) {

   Need spaces around the '=' and '<' signs.

   24                 numReplaced ++;

   Unneeded space.


http://cr.openjdk.java.net/~mseledtsov/8165805.00/test/runtime/SharedArchiveFile/serviceability/TransformerAgent.java.html

   12 // This is a test ulitilty class used to transform
   13 // specified classes via initial transformation (CFLH)
   14 // Names of classes to be transformed are supplied as arguments,

   Need a dot at the end of L13.


http://cr.openjdk.java.net/~mseledtsov/8165805.00/test/runtime/SharedArchiveFile/serviceability/transformRelatedClasses/TransformTestCommon.java.html

   24         if(entry.transformParent && entry.transformChild)
   26         if(entry.transformParent)
   28         if(entry.transformChild)
   53         if(entry.isParentExpectedShared)
   58         if(entry.isChildExpectedShared)

   Space is needed after 'if'.

   71         out.shouldContain("TransformerAgent: SimpleTransformer 
called for: " + child + "@1");
   72         out.shouldContain("TransformerAgent: SimpleTransformer 
called for: " + parent + "@1");
   73
   74         out.shouldNotContain("TransformerAgent: SimpleTransformer 
called for: " + child + "@2");
   75         out.shouldNotContain("TransformerAgent: SimpleTransformer 
called for: " + parent + "@2");

   Can be simplified:
          String patternBase = "TransformerAgent: SimpleTransformer 
called for: ";
          out.shouldContain(patternBase + child + "@1");
          out.shouldContain(patternBase + parent + "@1");
          out.shouldNotContain(patternBase + child + "@2");
          out.shouldNotContain(patternBase + parent + "@2");


   92         checkSharingByClass(entry,out, parent, child);

   Space is missed before 'out'.


Thanks,
Serguei



On 9/9/16 19:31, Mikhailo Seledtsov 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
>