RFR (M) 8179249 Improve process output analysis in CDS tests

[Ioi Lam]

Thanks Misha.

Test results are good. I'll push the changes now.

- Ioi


On 2/15/18 9:49 AM, Mikhailo Seledtsov wrote:
> Looks good.
>
> Misha
>
> On 2/14/18, 11:36 AM, Ioi Lam wrote:
>> Hi Misha,
>>
>> Thanks for the review. I have updated the webrev at
>>
>> http://cr.openjdk.java.net/~iklam/jdk11/8179249-improve-cds-tests-process-output-analysis.v02/ 
>>
>>
>> The only changes from the last webrev is I fixed two additional tests:
>>
>> jvmti/transformRelatedClasses/TransformRelatedClassesAppCDS.java
>>     jigsaw/classpathtests/DummyClassesInBootClassPath.java
>>
>> Please take another look. If things look OK I will push.
>>
>> More comments in-line:
>>
>> On 1/26/18 3:30 PM, Mikhailo Seledtsov wrote:
>>> Hi Ioi,
>>>
>>>     The design looks good to me, and I like how the use of Lambda 
>>> simplifies the code.
>>>     Also, thank you for documenting the usage in such a detail.
>>>
>>>     One question: now that you introduced TestCommon.run(), can we 
>>> remove TestCommon.exec*() ?
>>>
>> Yes, eventually we should remove all the different forms of 
>> TestCommon.exec*(). I plan to do that in a future RFE. That would 
>> take a lot of effort, though.
>>
>> The main purpose of the current RFE is to introduce the new 
>> TestCommon.run() and CDSTestUtils.Result API, so that all new tests 
>> will be written using this new and more robust API.
>>
>> Thanks
>> - Ioi
>>
>>> Thank you,
>>> Misha
>>>
>>> On 1/25/18, 1:16 PM, Ioi Lam wrote:
>>>> Hi,
>>>>
>>>> This is a preliminary RFR. I still need to do some cleanup and add 
>>>> more JavaDocs. However, I just wanted to show the design and see if 
>>>> people are OK with it:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8179249
>>>> http://cr.openjdk.java.net/~iklam/jdk11/8179249-improve-cds-tests-process-output-analysis.v01/ 
>>>>
>>>>
>>>> The goal of this RFE is to make the CDS tests more maintainable. 
>>>> You can read the background in the RFE description. Here are some 
>>>> examples of the new style:
>>>>
>>>>      * 1. For simple substring matching:
>>>>      *
>>>>      *      CCDSTestUtils.run(args).assertNormalExit("Hi");
>>>>      *      CCDSTestUtils.run(args).assertNormalExit("a", "b", "x");
>>>>      * CCDSTestUtils.run(args).assertAbnormalExit("failure 1", 
>>>> "failure2");
>>>>      *
>>>>      * 2. For more complex output matching: using Lambda expressions
>>>>      *
>>>>      *      CCDSTestUtils.run(args)
>>>>      *         .assertNormalExit(output -> 
>>>> output.shouldNotContain("this should not be printed");
>>>>      *      CCDSTestUtils.run(args)
>>>>      *         .assertAbnormalExit(output -> {
>>>>      *             output.shouldNotContain("this should not be 
>>>> printed");
>>>>      *             output.shouldHaveExitValue(123);
>>>>      *           });
>>>>      *
>>>>      * 3. Chaining several checks:
>>>>      *
>>>>      *      CCDSTestUtils.run(args)
>>>>      *         .assertNormalExit(output -> 
>>>> output.shouldNotContain("this should not be printed")
>>>>      *         .assertNormalExit("should have this", "should have 
>>>> that");
>>>>
>>>> More detailed description can be found in CDSTestUtils.java
>>>>
>>>> In this push, I am limiting the changes only to those tests that 
>>>> called TestCommon.execCommon(), which had the more unreadable and 
>>>> error-prone code. I may clean up the other tests in a future RFE.
>>>>
>>>> I removed the TestCommon.execCommon calls from all the appcds test 
>>>> cases, except for the following tests
>>>>
>>>> runtime/appcds/ClassLoaderTest.java
>>>>
>>>>     This test has a bug (JDK-8196121), so it should be fixed 
>>>> separately.
>>>>
>>>> runtime/appcds/jigsaw/classpathtests/DummyClassesInBootClassPath.java
>>>>
>>>>     This test has a bug (JDK-8196124), so it should be fixed 
>>>> separately.
>>>>
>>>> runtime/appcds/jvmti/transformRelatedClasses/TransformRelatedClassesAppCDS.java 
>>>>
>>>>
>>>>      This test is related to 
>>>> runtime/SharedArchiveFile/serviceability/transformRelatedClasses I 
>>>> plan to fix all the tests in runtime/SharedArchiveFile in a 
>>>> separate RFE.
>>>>
>>>> Thanks
>>>> - Ioi
>>>>
>>