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

[Roland Westrelin]

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