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

Mikhailo Seledtsov mikhailo.seledtsov at oracle.com
Thu Feb 15 17:49:52 UTC 2018


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
>>>
>


More information about the hotspot-runtime-dev mailing list