RFR (S): 8214086: [TESTBUG] Fix subgraph test cases in ReplaceCriticalClasses.java

[Jiangli Zhou]

Hi Ioi,

On 11/21/18 3:08 PM, Ioi Lam wrote:
> Hi Jiangli,
>
> For this test
>
> final String subgraphInit = "initialize_from_archived_subgraph " + 
> subgraphKlass;
> ...
> if (checkSubgraph) {
>      out.shouldNotContain(subgraphInit);
> }
>
> I think you should also test for the reverse condition -- if 
> java/lang/Long has not been redefined, you should see the subgraphInit 
> in the output.
>
> The reason for this is shouldNotContain() is very lenient. If you made 
> a mistake in formatting subgraphInit, shouldNotContain will not 
> complain, and you will end up having a bad test that doesn't test what 
> you're looking for.

I actually have the reverse check initially, but decided to remove it 
since we have other tests that explicitly check for archived Long cache 
and system module subgraphs. I can add one test case for the Long cache 
(for future proof in case the logging output is changed).

Thanks,
Jiangli

>
> Thanks
>
> - Ioi
>
>
>
> On 11/21/18 2:17 PM, Jiangli Zhou wrote:
>> Please review the following test fix in ReplaceCriticalClasses.java. 
>> I re-implemented the subgraph check to verify a subgraph is not used 
>> at runtime if one of it's klass (who's object is contained within the 
>> archived subgraph) is changed by the JVMTI agent.
>>
>> I added a test case for the newly archived Long cache subgraph. Since 
>> both ResolvedModule and Long classes are loaded during the primordial 
>> phase, '-early -notshared' are set in those test cases.
>>
>> As part of the change, I fixed 
>> HeapShared::initialize_from_archived_subgraph to only do the logging 
>> output after sbugraph(s) is successfully installed in the given Klass.
>>
>> webrev: http://cr.openjdk.java.net/~jiangli/8214086/webrev.00
>> bug: https://bugs.openjdk.java.net/browse/JDK-8214086
>>
>> Tested ReplaceCriticalClasses.java locally on linux-x64. Will run 
>> tier1-tier2.
>>
>> Thanks,
>> Jiangli