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

[Ioi Lam]

Looks good. Thanks!

- Ioi

On 11/21/18 4:34 PM, Jiangli Zhou wrote:
> Here is the updated webrev with a new test case suggested by Ioi:
>
> http://cr.openjdk.java.net/~jiangli/8214086/webrev.01/test/hotspot/jtreg/runtime/SharedArchiveFile/serviceability/ReplaceCriticalClasses.java.frames.html 
>
>
> Full webrev: http://cr.openjdk.java.net/~jiangli/8214086/webrev.01/
>
> Thanks,
> Jiangli
>
> On 11/21/18 3:23 PM, Jiangli Zhou wrote:
>> 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
>>
>