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

Christian Thalinger christian.thalinger at oracle.com
Tue Dec 2 22:28:58 UTC 2014


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/ <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/ <http://cr.openjdk.java.net/%7Eeistepan/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 <https://bugs.openjdk.java.net/browse/JDK-8054892>
>>>>> webrev: http://cr.openjdk.java.net/~eistepan/8054892/webrev.00/ <http://cr.openjdk.java.net/%7Eeistepan/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/e616da33/attachment.html>


More information about the hotspot-compiler-dev mailing list