RFR(M): 8054892: Improve compiler's CLI tests error reporting

[Evgeniya Stepanova]

Hi Roland,
Thank you very much for the review!
On 05.12.2014 14:35, Roland Westrelin wrote:
> That looks good to me.
>
> Roland.
>
>> On Dec 5, 2014, at 9:16 AM, Evgeniya Stepanova <evgeniya.stepanova at oracle.com> wrote:
>>
>> Hi everyone,
>>
>> I need one more review. Could anyone do it for me?
>>
>> Thank you in advance,
>> Evgeniya Stepanova
>> On 03.12.2014 14:16, Evgeniya Stepanova wrote:
>>> Christian,
>>> Thank you for the review!
>>>
>>> On 03.12.2014 2:28, Christian Thalinger wrote:
>>>> Looks good.
>>>>
>>>>> On Dec 2, 2014, at 7:10 AM, Evgeniya Stepanova <evgeniya.stepanova at oracle.com> wrote:
>>>>>
>>>>> Hi Filipp,
>>>>> Thanks for the review!
>>>>> No problems, only changes in test/testlibrary/com/oracle/java/testlibrary/cli/CommandLineOptionTest.java were needed to do that.
>>>>> new webrev
>>>>> http://cr.openjdk.java.net/~eistepan/8054892/webrev.02/
>>>>>
>>>>> On 02.12.2014 18:20, Filipp Zhinkin wrote:
>>>>>> Evgeniya,
>>>>>>
>>>>>> thank you for taking care of it!
>>>>>>
>>>>>> Sorry that I'm asking you about that only now,
>>>>>> but could use the same order of arguments in
>>>>>> verifyJVMStartup and verifySameJVMStartup?
>>>>>>
>>>>>>      public static void verifyJVMStartup(String expectedMessages[],
>>>>>>              String unexpectedMessages[], ExitCode exitCode,
>>>>>>              String exitErrorMessage, String wrongWarningMessage,
>>>>>>              boolean addTestVMOptions, String... options)
>>>>>>
>>>>>>      public static void verifySameJVMStartup(String expectedMessages[],
>>>>>>              String unexpectedMessages[], String exitErrorMessage,
>>>>>>              String wrongWarningMessage, ExitCode exitCode, String... options)
>>>>>>
>>>>>> Could you place exitCode after *Message in both methods?
>>>>>>
>>>>>> Otherwise the change looks good.
>>>>>>
>>>>>> Thanks,
>>>>>> Filipp.
>>>>>>
>>>>>> On 12/02/2014 02:55 PM, Evgeniya Stepanova wrote:
>>>>>>> Hi Christian,
>>>>>>>
>>>>>>> Thank you very much for the review
>>>>>>>
>>>>>>> I've replaced message
>>>>>>> +                    "JVMStartup should have exit value '%d'.%n%s",
>>>>>>>
>>>>>>> with the
>>>>>>> +                    "JVM process should have exit value '%d'.%n%s",
>>>>>>>
>>>>>>> and
>>>>>>> +                    "JVMStartup should be successful with option '%s'.",
>>>>>>> with the
>>>>>>> +                    "JVM should start with option '%s' without errors.",
>>>>>>>
>>>>>>> new webrev is
>>>>>>> http://cr.openjdk.java.net/~eistepan/8054892/webrev.01/
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Evgeniya Stepanova
>>>>>>>
>>>>>>> On 02.12.2014 2:49, Christian Thalinger wrote:
>>>>>>>> I didn’t verify all new messages but this looks very useful.  The only thing I could complain about is:
>>>>>>>>
>>>>>>>> +                    "JVMStartup should have exit value '%d'.%n%s",
>>>>>>>>
>>>>>>>> JVMStartup probably refers to some internal method.  A more general message might be better.
>>>>>>>>
>>>>>>>> In any case, looks good to me.
>>>>>>>>
>>>>>>>>> On Nov 27, 2014, at 3:13 AM, Evgeniya Stepanova <evgeniya.stepanova at oracle.com> wrote:
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Could you please review fix for 8054892?
>>>>>>>>> Problem: CLI tests do not show what exactly went wrong when test failed. They show an error  behavior is not as expected.
>>>>>>>>> Solution:  Added explanation which behavior is expected and why it is so.
>>>>>>>>>
>>>>>>>>> Base class /testlibrary/com/oracle/java/testlibrary/cli/CommandLineOptionTest.java changed to use error string, submitted by tests classes.
>>>>>>>>>   -exitErrorMessage message to be shown if exit code of JVM process is not  as expected.
>>>>>>>>>   -wrongWarningMessage message to be shown if warning  messages in output are not as expected.
>>>>>>>>> -optionErrorString to be shown if option value is not as expected.
>>>>>>>>> Updated tests from test/compiler/rtm/cli/, test/compiler/rtm/cli/ and test/compiler/intrinsics/sha/cli folders.
>>>>>>>>>
>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8054892
>>>>>>>>> webrev: http://cr.openjdk.java.net/~eistepan/8054892/webrev.00/
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Evgeniya Stepanova
>>>>>>>>>
>>>>>>> -- 
>>>>>>> Evgeniya Stepanova
>>>>> -- 
>>>>> Evgeniya Stepanova
>>> -- 
>>> Evgeniya Stepanova
>> -- 
>> Evgeniya Stepanova

-- 
/Evgeniya Stepanova/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20141205/9fea1dd7/attachment.html>