RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

Mikhailo Seledtsov mikhailo.seledtsov at oracle.com
Tue Apr 4 00:01:47 UTC 2017


Looks good to me.

Misha

On 4/3/17, 4:54 PM, David Holmes wrote:
> On 4/04/2017 9:09 AM, Igor Ignatyev wrote:
>> yes, David you are right. it's so embarrassing ... besides uploading
>> webrev w/o all changes, I have also tested it w/o all changes, I should
>> have noticed that. Thank you for catching that.
>>
>> I have fixed that, tested and uploaded new webrev --
>> _http://cr.openjdk.java.net/~iignatyev/8177507/webrev.03_
>
> Looks fine now.
>
> Thanks,
> David
>
>> Thanks,
>> -- Igor
>>> On Apr 3, 2017, at 1:53 PM, David Holmes <david.holmes at oracle.com
>>> <mailto:david.holmes at oracle.com>> wrote:
>>>
>>> On 4/04/2017 2:46 AM, Igor Ignatyev wrote:
>>>> looks like I have uploaded the webrev before I saved files... 
>>>> uploaded a
>>>> new webrev w/ all mentioned changes:
>>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.02
>>>
>>> I don't see how the change to LambdaBreakpointTest can possibly work.
>>> The new test steps to each line in source code order, but that is not
>>> the execution order!
>>>
>>> David
>>> -----
>>>
>>>> -- Igor
>>>>
>>>>> On Apr 2, 2017, at 1:31 PM, David Holmes <david.holmes at oracle.com
>>>>> <mailto:david.holmes at oracle.com>
>>>>> <mailto:david.holmes at oracle.com>> wrote:
>>>>>
>>>>> Hi Igor,
>>>>>
>>>>> On 3/04/2017 3:09 AM, Igor Ignatyev wrote:
>>>>>> Hi,
>>>>>>
>>>>>> http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01 is the next
>>>>>> iteration w/ copyright years fixed,
>>>>>
>>>>> test/com/sun/jdi/BreakpointTest.java
>>>>>
>>>>> also needs fixing - says 2015.
>>>>>
>>>>>> LineNumberOnBraceTarg and
>>>>>
>>>>> test/com/sun/jdi/LineNumberOnBraceTest.java - Ok.
>>>>>
>>>>>> LambdaBreakpointTest made more unified w/ the rest.
>>>>>
>>>>> I don't see any change here.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>> -----
>>>>>
>>>>>> Thanks,
>>>>>> -- Igor
>>>>>>
>>>>>>> On Mar 29, 2017, at 12:57 PM, serguei.spitsyn at oracle.com
>>>>>>> <mailto:serguei.spitsyn at oracle.com>
>>>>>>> <mailto:serguei.spitsyn at oracle.com>
>>>>>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>>>>>
>>>>>>> This one also does not look unified:
>>>>>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html 
>>>>>>>
>>>>>>
>>>>>>> On 3/29/17 11:08, Mikhailo Seledtsov wrote:
>>>>>>>
>>>>>>> One style nit:
>>>>>>>
>>>>>>> LineNumberOnBraceTarg:
>>>>>>> All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE)
>>>>>>> for line number variables, but this test uses 'stopLine'.
>>>>>>> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.
>>>>>>
>>>>>>> On Mar 28, 2017, at 6:37 PM, David Holmes <david.holmes at oracle.com
>>>>>>> <mailto:david.holmes at oracle.com>
>>>>>>> <mailto:david.holmes at oracle.com>
>>>>>>> <mailto:david.holmes at oracle.com>> wrote:
>>>>>>>
>>>>>>> Two nits:
>>>>>>> - test/com/sun/jdi/FetchLocals.java
>>>>>>> - test/com/sun/jdi/LambdaBreakpointTest.java
>>>>>>>
>>>>>>> Second copyright year should be 2017.
>>>>>>
>>>>>>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev
>>>>>>> <igor.ignatyev at oracle.com <mailto:igor.ignatyev at oracle.com>
>>>>>>> <mailto:igor.ignatyev at oracle.com>
>>>>>>> <mailto:igor.ignatyev at oracle.com>> wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> could you please review this fix for 8177507?
>>>>>>>
>>>>>>> due to their nature, some of jdi tests are line number sensitive.
>>>>>>> unfortunately different tests indicate that differently, so it's 
>>>>>>> quite
>>>>>>> easy to overlook that and incidentally break tests, for example by
>>>>>>> changing module dependency declaration or license modification. 
>>>>>>> this
>>>>>>> fix unifies the way line number sensitivity is indicated and also
>>>>>>> improves readability/maintainability of some tests by using 
>>>>>>> constant
>>>>>>> fields instead of magic numbers.
>>>>>>>
>>>>>>> some of line number sensitive tests have been unexpectedly removed
>>>>>>> from execution because they had @test/nodynamiccopyright/ 
>>>>>>> instead of
>>>>>>> @test tag. this changeset fixes and returns them to regular 
>>>>>>> execution.
>>>>>>>
>>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>>>>>>> testing: test/com/sun/jdi
>>


More information about the serviceability-dev mailing list