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

[Filipp Zhinkin]

Evgeniya,

thanks! The change looks good (not a Reviewer).

Regards,
Filipp.

On 12/02/2014 07:10 PM, Evgeniya Stepanova 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 <mailto: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/

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20141202/fd13a40e/attachment-0001.html>